From ecebdb87c90840173844d98e4343672df64cb861 Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 23 Jul 2026 20:44:03 -0400 Subject: [PATCH 01/20] feat(flags): add rules engine boundary --- .../__tests__/__utils__/rulesTestUtils.ts | 82 +++ .../configuration/__tests__/rules.test.ts | 175 +++++ .../configuration/__tests__/wire.test.ts | 59 ++ .../core/src/flags/configuration/rules.ts | 614 ++++++++++++++++++ packages/core/src/flags/configuration/wire.ts | 94 ++- 5 files changed, 1021 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts create mode 100644 packages/core/src/flags/configuration/__tests__/rules.test.ts create mode 100644 packages/core/src/flags/configuration/rules.ts diff --git a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts new file mode 100644 index 000000000..4eb2aef52 --- /dev/null +++ b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts @@ -0,0 +1,82 @@ +/* + * 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://github.com/DataDog). + * Copyright 2016-Present Datadog, Inc. + */ + +import { OperatorType } from '@datadog/flagging-core'; +import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; + +import type { + RulesEngine, + RulesEvaluationDetails, + RulesEvaluationRequest, + RulesValueType +} from '../../rules'; + +export const buildRulesConfiguration = (): UniversalFlagConfigurationV1 => ({ + createdAt: '2026-07-23T12:00:00.000Z', + format: 'SERVER', + environment: { name: 'test' }, + flags: { + 'dynamic-flag': { + key: 'dynamic-flag', + enabled: true, + variationType: 'BOOLEAN', + variations: { + enabled: { key: 'enabled', value: true }, + disabled: { key: 'disabled', value: false } + }, + allocations: [ + { + key: 'allocation-1', + rules: [ + { + conditions: [ + { + operator: OperatorType.ONE_OF, + attribute: 'country', + value: ['US'] + } + ] + } + ], + splits: [ + { + variationKey: 'enabled', + serialId: 7, + extraLogging: { experiment: 'checkout' }, + shards: [ + { + salt: 'test-salt', + ranges: [{ start: 0, end: 100 }], + totalShards: 100 + } + ] + } + ], + doLog: false + } + ] + } + } +}); + +type FakeRulesEvaluation = RulesEvaluationDetails; + +export interface FakeRulesEngine extends RulesEngine { + evaluate: jest.Mock< + FakeRulesEvaluation, + [RulesEvaluationRequest] + >; +} + +// TODO(FFL-2837): Remove this fake after the upstream rules wire and engine +// contract are published and the state-matrix tests can use canonical vectors. +export const createFakeRulesEngine = ( + result: FakeRulesEvaluation +): FakeRulesEngine => { + return { + evaluate: jest.fn(() => result) + } as FakeRulesEngine; +}; diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts new file mode 100644 index 000000000..932851cdd --- /dev/null +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -0,0 +1,175 @@ +/* + * 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://github.com/DataDog). + * Copyright 2016-Present Datadog, Inc. + */ + +import { + flaggingCoreRulesEngine, + getNoopRulesLogger, + prepareRulesConfiguration, + toRulesEvaluationContext +} from '../rules'; + +import { + buildRulesConfiguration, + createFakeRulesEngine +} from './__utils__/rulesTestUtils'; + +describe('rules configuration', () => { + it('converts an SDK context to a flat rules context and reserves identifiers', () => { + expect( + toRulesEvaluationContext({ + targetingKey: 'user-1', + attributes: { + country: 'US', + id: 'customer-id', + targetingKey: 'attribute-key', + enabled: true + } + }) + ).toEqual({ + targetingKey: 'user-1', + country: 'US', + enabled: true + }); + }); + + it('clones and freezes a valid rules configuration', () => { + const source = buildRulesConfiguration(); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + + source.flags['dynamic-flag'].enabled = false; + + expect(prepared.configuration.flags['dynamic-flag'].enabled).toBe(true); + expect(Object.isFrozen(prepared.configuration)).toBe(true); + expect( + Object.isFrozen( + prepared.configuration.flags['dynamic-flag'].allocations[0] + ) + ).toBe(true); + }); + + it('rejects an unsupported operator', () => { + const source = buildRulesConfiguration(); + const condition = + source.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + + if (!condition) { + throw new Error('The fixture has no condition.'); + } + (condition as { operator: string }).operator = 'ONE_OF_SHA256'; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: + 'The rules configuration uses the unsupported operator "ONE_OF_SHA256".' + }); + }); + + it('rejects an invalid regular expression', () => { + const source = buildRulesConfiguration(); + const conditions = + source.flags['dynamic-flag'].allocations[0].rules?.[0].conditions; + if (!conditions) { + throw new Error('The fixture has no conditions.'); + } + conditions[0] = { + operator: 'MATCHES', + attribute: 'country', + value: '[' + } as typeof conditions[number]; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: 'A regular expression condition is not valid.' + }); + }); + + it('rejects a split that points to an absent variation', () => { + const source = buildRulesConfiguration(); + source.flags['dynamic-flag'].allocations[0].splits[0].variationKey = + 'absent'; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: 'A split has an invalid variation key.' + }); + }); + + it('normalizes a real flagging-core evaluation', () => { + const configuration = buildRulesConfiguration(); + + const result = flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { + targetingKey: 'user-1', + country: 'US' + }, + logger: getNoopRulesLogger() + }); + + expect(result).toMatchObject({ + value: true, + variant: 'enabled', + reason: 'TARGETING_MATCH', + metadata: { + allocationKey: 'allocation-1', + variationType: 'boolean', + doLog: false, + extraLogging: { experiment: 'checkout' }, + splitSerialId: 7 + } + }); + expect(result.metadata.evaluationTimestampMs).toEqual( + expect.any(Number) + ); + }); + + it('checks own properties before it calls flagging-core', () => { + const result = flaggingCoreRulesEngine.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey: 'toString', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }); + + expect(result).toEqual({ + value: false, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }); + }); + + it('provides a deterministic fake engine for client tests', () => { + const fake = createFakeRulesEngine({ + value: true, + variant: 'fake', + reason: 'TARGETING_MATCH', + metadata: {} + }); + + expect( + fake.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: true, variant: 'fake' }); + }); +}); diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index 23f1d359b..e464a701b 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -7,6 +7,8 @@ import type { ParsedFlagsConfiguration } from '../types'; import { configurationFromString, configurationToString } from '../wire'; +import { buildRulesConfiguration } from './__utils__/rulesTestUtils'; + const buildResponse = () => ({ data: { id: '2', @@ -125,3 +127,60 @@ describe('configurationToString round-trip', () => { ); }); }); + +describe('rules configuration wire compatibility', () => { + it('parses and serializes a rules configuration', () => { + const rulesBased = { + response: buildRulesConfiguration(), + fetchedAt: 123, + etag: 'rules-etag' + }; + const wire = JSON.stringify({ + version: 1, + rulesBased: { + ...rulesBased, + response: JSON.stringify(rulesBased.response) + } + }); + + const parsed = configurationFromString(wire) as { + rulesBased?: typeof rulesBased; + }; + + expect(parsed.rulesBased).toEqual(rulesBased); + expect( + configurationFromString( + configurationToString( + (parsed as unknown) as ParsedFlagsConfiguration + ) + ) + ).toEqual(parsed); + }); + + it('keeps both branches in a mixed configuration', () => { + const mixedWire = buildWire({ + rulesBased: { + response: JSON.stringify(buildRulesConfiguration()) + } + }); + + const parsed = configurationFromString(mixedWire) as { + precomputed?: unknown; + rulesBased?: unknown; + }; + + expect(parsed.precomputed).toBeDefined(); + expect(parsed.rulesBased).toBeDefined(); + }); + + it('returns an empty configuration for malformed rules JSON', () => { + expect( + configurationFromString( + JSON.stringify({ + version: 1, + rulesBased: { response: '{' } + }) + ) + ).toEqual({}); + }); +}); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts new file mode 100644 index 000000000..592f4216d --- /dev/null +++ b/packages/core/src/flags/configuration/rules.ts @@ -0,0 +1,614 @@ +/* + * 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 { + evaluateRulesBasedConfiguration, + OperatorType +} from '@datadog/flagging-core'; +import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; + +import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; + +export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; + +type RulesValueByType = { + boolean: boolean; + string: string; + number: number; + object: JsonValue; +}; + +export interface RulesLogger { + debug: (message: string, ...args: unknown[]) => void; + info: (message: string, ...args: unknown[]) => void; + warn: (message: string, ...args: unknown[]) => void; + error: (message: string, ...args: unknown[]) => void; +} + +export interface RulesEvaluationContext { + targetingKey?: string; + [key: string]: PrimitiveValue | undefined; +} + +export interface RulesEvaluationMetadata { + allocationKey?: string; + variationType?: RulesValueType; + doLog?: boolean; + extraLogging?: Record; + splitSerialId?: number; + evaluationTimestampMs?: number; +} + +export interface RulesEvaluationDetails { + value: T; + reason?: string; + variant?: string; + errorCode?: string; + errorMessage?: string; + metadata: RulesEvaluationMetadata; +} + +export interface RulesEvaluationRequest { + configuration: UniversalFlagConfigurationV1; + type: T; + flagKey: string; + defaultValue: RulesValueByType[T]; + context: RulesEvaluationContext; + logger: RulesLogger; +} + +export interface RulesEngine { + evaluate( + request: RulesEvaluationRequest + ): RulesEvaluationDetails; +} + +type RawEvaluationDetails = { + value: T; + reason?: string; + variant?: string; + errorCode?: string; + errorMessage?: string; + flagMetadata?: Record; +}; + +type EvaluateRules = ( + configuration: UniversalFlagConfigurationV1, + type: T, + flagKey: string, + defaultValue: RulesValueByType[T], + context: RulesEvaluationContext, + logger: RulesLogger +) => RawEvaluationDetails; + +const evaluateRules = evaluateRulesBasedConfiguration as EvaluateRules; + +const NOOP_LOGGER: RulesLogger = { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {} +}; + +export const getNoopRulesLogger = (): RulesLogger => NOOP_LOGGER; + +/** + * Convert the SDK context to the flat context that flagging-core uses. + * + * `id` and `targetingKey` are reserved. The adapter always derives them from + * `EvaluationContext.targetingKey`. + */ +export const toRulesEvaluationContext = ( + context: EvaluationContext +): RulesEvaluationContext => { + const attributes = new Map(); + + for (const [key, value] of Object.entries(context.attributes ?? {})) { + if (key === 'id' || key === 'targetingKey' || value === undefined) { + continue; + } + attributes.set(key, value); + } + + return { + ...Object.fromEntries(attributes), + targetingKey: context.targetingKey + }; +}; + +const hasOwn = (value: object, key: PropertyKey): boolean => + Object.prototype.hasOwnProperty.call(value, key); + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const isStringRecord = (value: unknown): value is Record => { + if (!isRecord(value)) { + return false; + } + + return Object.values(value).every(item => typeof item === 'string'); +}; + +const isJsonValue = (value: unknown): value is JsonValue => { + if ( + value === null || + typeof value === 'string' || + typeof value === 'boolean' + ) { + return true; + } + if (typeof value === 'number') { + return Number.isFinite(value); + } + if (Array.isArray(value)) { + return value.every(isJsonValue); + } + if (isRecord(value)) { + return Object.values(value).every(isJsonValue); + } + return false; +}; + +const variationValueIsValid = ( + variationType: string, + value: unknown +): boolean => { + switch (variationType) { + case 'BOOLEAN': + return typeof value === 'boolean'; + case 'STRING': + return typeof value === 'string'; + case 'INTEGER': + case 'NUMERIC': + return typeof value === 'number' && Number.isFinite(value); + case 'JSON': + return isJsonValue(value); + default: + return false; + } +}; + +const SUPPORTED_OPERATORS: ReadonlySet = new Set( + Object.values(OperatorType) +); + +const validateCondition = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.attribute !== 'string' || + typeof value.operator !== 'string' + ) { + return 'A rule condition has an invalid shape.'; + } + + if (!SUPPORTED_OPERATORS.has(value.operator)) { + return `The rules configuration uses the unsupported operator "${value.operator}".`; + } + + switch (value.operator) { + case OperatorType.MATCHES: + case OperatorType.NOT_MATCHES: + if (typeof value.value !== 'string') { + return 'A regular expression condition must contain a string.'; + } + try { + // TODO(FFL-2837): Replace this compile-only check with the upstream + // safe-regex policy before dynamic offline rules leave draft state. + RegExp(value.value); // dd-iac-scan ignore-line + } catch { + return 'A regular expression condition is not valid.'; + } + return undefined; + case OperatorType.ONE_OF: + case OperatorType.NOT_ONE_OF: + return Array.isArray(value.value) && + value.value.every(item => typeof item === 'string') + ? undefined + : 'A membership condition must contain a string array.'; + case OperatorType.GTE: + case OperatorType.GT: + case OperatorType.LTE: + case OperatorType.LT: + return typeof value.value === 'number' && + Number.isFinite(value.value) + ? undefined + : 'A numeric condition must contain a finite number.'; + case OperatorType.IS_NULL: + return typeof value.value === 'boolean' + ? undefined + : 'A null condition must contain a boolean.'; + default: + return 'The rules configuration uses an unsupported operator.'; + } +}; + +const validateRules = (value: unknown): string | undefined => { + if (value === undefined) { + return undefined; + } + if (!Array.isArray(value)) { + return 'An allocation rules field must be an array.'; + } + + for (const rule of value) { + if (!isRecord(rule) || !Array.isArray(rule.conditions)) { + return 'A rule has an invalid shape.'; + } + for (const condition of rule.conditions) { + const error = validateCondition(condition); + if (error) { + return error; + } + } + } + + return undefined; +}; + +const validateShards = (value: unknown): string | undefined => { + if (!Array.isArray(value)) { + return 'A split shards field must be an array.'; + } + + for (const shard of value) { + if ( + !isRecord(shard) || + typeof shard.salt !== 'string' || + !Number.isInteger(shard.totalShards) || + (shard.totalShards as number) <= 0 || + !Array.isArray(shard.ranges) + ) { + return 'A shard has an invalid shape.'; + } + + for (const range of shard.ranges) { + if ( + !isRecord(range) || + !Number.isInteger(range.start) || + !Number.isInteger(range.end) || + (range.start as number) < 0 || + (range.end as number) <= (range.start as number) || + (range.end as number) > (shard.totalShards as number) + ) { + return 'A shard range is not valid.'; + } + } + } + + return undefined; +}; + +const isValidDate = (value: unknown): boolean => + value instanceof Date + ? !Number.isNaN(value.getTime()) + : typeof value === 'string' && !Number.isNaN(Date.parse(value)); + +const validateAllocation = ( + value: unknown, + variations: Record +): string | undefined => { + if ( + !isRecord(value) || + typeof value.key !== 'string' || + !Array.isArray(value.splits) + ) { + return 'An allocation has an invalid shape.'; + } + + if (value.startAt !== undefined && !isValidDate(value.startAt)) { + return 'An allocation start time is not valid.'; + } + if (value.endAt !== undefined && !isValidDate(value.endAt)) { + return 'An allocation end time is not valid.'; + } + + const rulesError = validateRules(value.rules); + if (rulesError) { + return rulesError; + } + + for (const split of value.splits) { + if ( + !isRecord(split) || + typeof split.variationKey !== 'string' || + !hasOwn(variations, split.variationKey) + ) { + return 'A split has an invalid variation key.'; + } + if (split.serialId !== undefined && !Number.isInteger(split.serialId)) { + return 'A split serial ID is not valid.'; + } + if ( + split.extraLogging !== undefined && + !isStringRecord(split.extraLogging) + ) { + return 'A split extraLogging field is not valid.'; + } + + const shardsError = validateShards(split.shards); + if (shardsError) { + return shardsError; + } + } + + return undefined; +}; + +const validateFlag = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.key !== 'string' || + typeof value.enabled !== 'boolean' || + typeof value.variationType !== 'string' || + !isRecord(value.variations) || + !Array.isArray(value.allocations) + ) { + return 'A flag has an invalid shape.'; + } + + if ( + !['BOOLEAN', 'INTEGER', 'NUMERIC', 'STRING', 'JSON'].includes( + value.variationType + ) + ) { + return `A flag uses the unsupported variation type "${value.variationType}".`; + } + + for (const variation of Object.values(value.variations)) { + if ( + !isRecord(variation) || + typeof variation.key !== 'string' || + !variationValueIsValid(value.variationType, variation.value) + ) { + return 'A variation has an invalid shape or value.'; + } + } + + for (const allocation of value.allocations) { + const error = validateAllocation(allocation, value.variations); + if (error) { + return error; + } + } + + return undefined; +}; + +const validateRulesConfiguration = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.createdAt !== 'string' || + typeof value.format !== 'string' || + !isRecord(value.environment) || + typeof value.environment.name !== 'string' || + !isRecord(value.flags) + ) { + return 'The rules configuration has an invalid envelope.'; + } + + for (const flag of Object.values(value.flags)) { + const error = validateFlag(flag); + if (error) { + return error; + } + } + + return undefined; +}; + +const cloneValue = (value: unknown): unknown => { + if (value instanceof Date) { + return new Date(value.getTime()); + } + if (Array.isArray(value)) { + return value.map(cloneValue); + } + if (isRecord(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, item]) => [key, cloneValue(item)]) + ); + } + return value; +}; + +const freezeValue = (value: unknown): void => { + if (typeof value !== 'object' || value === null || Object.isFrozen(value)) { + return; + } + + Object.freeze(value); + for (const item of Object.values(value)) { + freezeValue(item); + } +}; + +export type PreparedRulesConfiguration = + | { + status: 'ready'; + configuration: UniversalFlagConfigurationV1; + } + | { + status: 'error'; + errorMessage: string; + }; + +/** + * Clone and validate untrusted rules before `FlagsClient` stores them. + */ +export const prepareRulesConfiguration = ( + value: unknown +): PreparedRulesConfiguration => { + const clone = cloneValue(value); + + // TODO(FFL-2837): Replace the temporary SDK validator with the published + // flagging-core validation API. Keep the branch-level result contract. + const errorMessage = validateRulesConfiguration(clone); + if (errorMessage) { + return { status: 'error', errorMessage }; + } + + freezeValue(clone); + return { + status: 'ready', + configuration: clone as UniversalFlagConfigurationV1 + }; +}; + +const normalizeVariationType = ( + variationType: unknown +): RulesValueType | undefined => { + switch (variationType) { + case 'boolean': + case 'string': + case 'number': + case 'object': + return variationType; + case 'BOOLEAN': + return 'boolean'; + case 'STRING': + return 'string'; + case 'INTEGER': + case 'NUMERIC': + return 'number'; + case 'JSON': + return 'object'; + default: + return undefined; + } +}; + +const recoverSplitMetadata = ( + configuration: UniversalFlagConfigurationV1, + flagKey: string, + variant: string | undefined, + allocationKey: string | undefined, + splitSerialId: number | undefined +): Pick => { + const flags = configuration.flags as Record; + if (!hasOwn(flags, flagKey)) { + return {}; + } + + const flag = flags[flagKey]; + if (!isRecord(flag)) { + return {}; + } + + const variationType = normalizeVariationType(flag.variationType); + if (!Array.isArray(flag.allocations)) { + return { variationType }; + } + + const variations = isRecord(flag.variations) ? flag.variations : {}; + const variationEntry = Object.entries(variations).find(([, value]) => { + return isRecord(value) && value.key === variant; + }); + const variationKey = variationEntry?.[0]; + + for (const allocation of flag.allocations) { + if ( + !isRecord(allocation) || + allocation.key !== allocationKey || + !Array.isArray(allocation.splits) + ) { + continue; + } + const split = allocation.splits.find(candidate => { + if (!isRecord(candidate)) { + return false; + } + if ( + splitSerialId !== undefined && + candidate.serialId === splitSerialId + ) { + return true; + } + return ( + splitSerialId === undefined && + variationKey !== undefined && + candidate.variationKey === variationKey + ); + }); + if (isRecord(split) && isStringRecord(split.extraLogging)) { + return { extraLogging: split.extraLogging, variationType }; + } + } + + return { variationType }; +}; + +export const flaggingCoreRulesEngine: RulesEngine = { + evaluate( + request: RulesEvaluationRequest + ): RulesEvaluationDetails { + const flags = request.configuration.flags as Record; + if (!hasOwn(flags, request.flagKey)) { + return { + value: request.defaultValue, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }; + } + + const result = evaluateRules( + request.configuration, + request.type, + request.flagKey, + request.defaultValue, + request.context, + request.logger + ); + const rawMetadata = result.flagMetadata ?? {}; + const allocationKey = + typeof rawMetadata.allocationKey === 'string' + ? rawMetadata.allocationKey + : typeof rawMetadata.__dd_allocation_key === 'string' + ? rawMetadata.__dd_allocation_key + : undefined; + const splitSerialId = + typeof rawMetadata.__dd_split_serial_id === 'number' + ? rawMetadata.__dd_split_serial_id + : undefined; + + // TODO(FFL-2837): Remove this metadata lookup when flagging-core + // returns extraLogging and the original UFC variation type. + const recoveredMetadata = recoverSplitMetadata( + request.configuration, + request.flagKey, + result.variant, + allocationKey, + splitSerialId + ); + + return { + value: result.value, + reason: result.reason, + variant: result.variant, + errorCode: result.errorCode, + errorMessage: result.errorMessage, + metadata: { + allocationKey, + variationType: + normalizeVariationType(rawMetadata.variationType) ?? + recoveredMetadata.variationType, + doLog: + typeof rawMetadata.doLog === 'boolean' + ? rawMetadata.doLog + : typeof rawMetadata.__dd_do_log === 'boolean' + ? rawMetadata.__dd_do_log + : undefined, + extraLogging: recoveredMetadata.extraLogging, + splitSerialId, + evaluationTimestampMs: + typeof rawMetadata.__dd_eval_timestamp_ms === 'number' + ? rawMetadata.__dd_eval_timestamp_ms + : undefined + } + }; + } +}; diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 952636fd0..f47a6fb84 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -9,7 +9,95 @@ // it returns an empty configuration (`{}`) for malformed input or an unsupported wire // version rather than throwing. `configurationToString` is the inverse (its fix from // https://github.com/DataDog/openfeature-js-client/pull/331 shipped in flagging-core 2.0.0). -export { - configurationFromString, - configurationToString +import { + configurationFromString as coreConfigurationFromString, + configurationToString as coreConfigurationToString } from '@datadog/flagging-core'; +import type { + FlagsConfiguration, + UniversalFlagConfigurationV1 +} from '@datadog/flagging-core'; + +type PendingRulesConfiguration = FlagsConfiguration & { + rulesBased?: { + response: UniversalFlagConfigurationV1; + fetchedAt?: number; + etag?: string; + }; +}; + +type PendingRulesWire = { + version: 1; + rulesBased?: { + response: string; + fetchedAt?: number; + etag?: string; + }; +}; + +const readPendingRulesWire = ( + source: string +): PendingRulesWire['rulesBased'] | undefined => { + try { + const wire = JSON.parse(source) as PendingRulesWire; + if ( + wire.version !== 1 || + !wire.rulesBased || + typeof wire.rulesBased.response !== 'string' + ) { + return undefined; + } + + return wire.rulesBased; + } catch { + return undefined; + } +}; + +/** + * Use flagging-core to parse a configuration wire. + */ +export const configurationFromString = (source: string): FlagsConfiguration => { + const configuration = coreConfigurationFromString( + source + ) as PendingRulesConfiguration; + + // TODO(FFL-2837): Delete this JSON compatibility shim after + // DataDog/openfeature-js-client#336 is published by flagging-core. + const pendingRules = readPendingRulesWire(source); + if (pendingRules) { + try { + configuration.rulesBased = { + ...pendingRules, + response: JSON.parse(pendingRules.response) + }; + } catch { + return {}; + } + } + + return configuration; +}; + +/** + * Use flagging-core to serialize a parsed configuration. + */ +export const configurationToString = ( + configuration: FlagsConfiguration +): string => { + const serialized = coreConfigurationToString(configuration); + const pendingConfiguration = configuration as PendingRulesConfiguration; + if (!pendingConfiguration.rulesBased) { + return serialized; + } + + // TODO(FFL-2837): Delete this JSON compatibility shim after + // DataDog/openfeature-js-client#336 is published by flagging-core. + const wire = JSON.parse(serialized) as PendingRulesWire; + wire.rulesBased = { + fetchedAt: pendingConfiguration.rulesBased.fetchedAt, + etag: pendingConfiguration.rulesBased.etag, + response: JSON.stringify(pendingConfiguration.rulesBased.response) + }; + return JSON.stringify(wire); +}; From faa43dcca324d3da1812438af46f6321b4902793 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 27 Jul 2026 16:10:42 -0400 Subject: [PATCH 02/20] fix(flags): align rules boundary with upstream --- .../__tests__/__utils__/rulesTestUtils.ts | 5 +- .../configuration/__tests__/rules.test.ts | 122 +++++++++++------ .../configuration/__tests__/wire.test.ts | 46 ++++--- .../core/src/flags/configuration/rules.ts | 128 ++++-------------- packages/core/src/flags/configuration/wire.ts | 23 ++-- 5 files changed, 148 insertions(+), 176 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts index 4eb2aef52..81190c964 100644 --- a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts +++ b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts @@ -45,7 +45,6 @@ export const buildRulesConfiguration = (): UniversalFlagConfigurationV1 => ({ { variationKey: 'enabled', serialId: 7, - extraLogging: { experiment: 'checkout' }, shards: [ { salt: 'test-salt', @@ -71,8 +70,8 @@ export interface FakeRulesEngine extends RulesEngine { >; } -// TODO(FFL-2837): Remove this fake after the upstream rules wire and engine -// contract are published and the state-matrix tests can use canonical vectors. +// Client tests use this fake to control evaluation independently of the +// flagging-core implementation and its canonical integration vectors. export const createFakeRulesEngine = ( result: FakeRulesEvaluation ): FakeRulesEngine => { diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 932851cdd..aeb3abdbf 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -35,6 +35,17 @@ describe('rules configuration', () => { }); }); + it('preserves the difference between a missing and empty targeting key', () => { + expect(toRulesEvaluationContext({})).toHaveProperty( + 'targetingKey', + undefined + ); + expect(toRulesEvaluationContext({ targetingKey: '' })).toHaveProperty( + 'targetingKey', + '' + ); + }); + it('clones and freezes a valid rules configuration', () => { const source = buildRulesConfiguration(); const prepared = prepareRulesConfiguration(source); @@ -55,7 +66,7 @@ describe('rules configuration', () => { ).toBe(true); }); - it('rejects an unsupported operator', () => { + it('omits a flag that uses an unsupported operator', () => { const source = buildRulesConfiguration(); const condition = source.flags['dynamic-flag'].allocations[0].rules?.[0] @@ -64,16 +75,18 @@ describe('rules configuration', () => { if (!condition) { throw new Error('The fixture has no condition.'); } - (condition as { operator: string }).operator = 'ONE_OF_SHA256'; + (condition as { operator: string }).operator = 'FUTURE_OPERATOR'; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: - 'The rules configuration uses the unsupported operator "ONE_OF_SHA256".' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); }); - it('rejects an invalid regular expression', () => { + it('omits a flag that contains an invalid regular expression', () => { const source = buildRulesConfiguration(); const conditions = source.flags['dynamic-flag'].allocations[0].rules?.[0].conditions; @@ -86,21 +99,52 @@ describe('rules configuration', () => { value: '[' } as typeof conditions[number]; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: 'A regular expression condition is not valid.' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); }); - it('rejects a split that points to an absent variation', () => { + it('omits a flag whose split points to an absent variation', () => { const source = buildRulesConfiguration(); source.flags['dynamic-flag'].allocations[0].splits[0].variationKey = 'absent'; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: 'A split has an invalid variation key.' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); + }); + + it('keeps valid flags when it omits an invalid flag', () => { + const source = buildRulesConfiguration(); + const validFlag = buildRulesConfiguration().flags['dynamic-flag']; + validFlag.key = 'valid-flag'; + source.flags['valid-flag'] = validFlag; + + const condition = + source.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + if (!condition) { + throw new Error('The fixture has no condition.'); + } + (condition as { operator: string }).operator = 'FUTURE_OPERATOR'; + + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(Object.keys(prepared.configuration.flags)).toEqual([ + 'valid-flag' + ]); }); it('normalizes a real flagging-core evaluation', () => { @@ -125,33 +169,31 @@ describe('rules configuration', () => { metadata: { allocationKey: 'allocation-1', variationType: 'boolean', - doLog: false, - extraLogging: { experiment: 'checkout' }, - splitSerialId: 7 + doLog: false } }); - expect(result.metadata.evaluationTimestampMs).toEqual( - expect.any(Number) - ); }); - it('checks own properties before it calls flagging-core', () => { - const result = flaggingCoreRulesEngine.evaluate({ - configuration: buildRulesConfiguration(), - type: 'boolean', - flagKey: 'toString', - defaultValue: false, - context: { targetingKey: 'user-1' }, - logger: getNoopRulesLogger() - }); - - expect(result).toEqual({ - value: false, - reason: 'ERROR', - errorCode: 'FLAG_NOT_FOUND', - metadata: {} - }); - }); + it.each(['toString', 'constructor', '__proto__'])( + 'checks own properties before it evaluates %s', + flagKey => { + const result = flaggingCoreRulesEngine.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey, + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }); + + expect(result).toEqual({ + value: false, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }); + } + ); it('provides a deterministic fake engine for client tests', () => { const fake = createFakeRulesEngine({ diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index e464a701b..9be51f2b6 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -128,8 +128,8 @@ describe('configurationToString round-trip', () => { }); }); -describe('rules configuration wire compatibility', () => { - it('parses and serializes a rules configuration', () => { +describe('temporary rules configuration wire compatibility', () => { + it('parses a legacy rules configuration', () => { const rulesBased = { response: buildRulesConfiguration(), fetchedAt: 123, @@ -148,13 +148,22 @@ describe('rules configuration wire compatibility', () => { }; expect(parsed.rulesBased).toEqual(rulesBased); - expect( - configurationFromString( - configurationToString( - (parsed as unknown) as ParsedFlagsConfiguration - ) + }); + + it('does not serialize a rules configuration', () => { + const configuration = { + rulesBased: { + response: buildRulesConfiguration() + } + }; + + expect(() => + configurationToString( + (configuration as unknown) as ParsedFlagsConfiguration ) - ).toEqual(parsed); + ).toThrow( + 'Rules configurations cannot be serialized to the wire format' + ); }); it('keeps both branches in a mixed configuration', () => { @@ -173,14 +182,17 @@ describe('rules configuration wire compatibility', () => { expect(parsed.rulesBased).toBeDefined(); }); - it('returns an empty configuration for malformed rules JSON', () => { - expect( - configurationFromString( - JSON.stringify({ - version: 1, - rulesBased: { response: '{' } - }) - ) - ).toEqual({}); + it('keeps a valid precomputed branch when rules JSON is malformed', () => { + const parsed = configurationFromString( + buildWire({ + rulesBased: { response: '{' } + }) + ) as { + precomputed?: unknown; + rulesBased?: unknown; + }; + + expect(parsed.precomputed).toBeDefined(); + expect(parsed.rulesBased).toBeUndefined(); }); }); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 592f4216d..0ee103dd5 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -37,9 +37,6 @@ export interface RulesEvaluationMetadata { allocationKey?: string; variationType?: RulesValueType; doLog?: boolean; - extraLogging?: Record; - splitSerialId?: number; - evaluationTimestampMs?: number; } export interface RulesEvaluationDetails { @@ -125,14 +122,6 @@ const hasOwn = (value: object, key: PropertyKey): boolean => const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); -const isStringRecord = (value: unknown): value is Record => { - if (!isRecord(value)) { - return false; - } - - return Object.values(value).every(item => typeof item === 'string'); -}; - const isJsonValue = (value: unknown): value is JsonValue => { if ( value === null || @@ -196,8 +185,9 @@ const validateCondition = (value: unknown): string | undefined => { return 'A regular expression condition must contain a string.'; } try { - // TODO(FFL-2837): Replace this compile-only check with the upstream - // safe-regex policy before dynamic offline rules leave draft state. + // TODO(FFL-2837): Define a bounded regular expression policy before + // dynamic offline rules leave draft state. Upstream PR #344 validates + // regular expression syntax, but it does not limit expensive patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -322,12 +312,6 @@ const validateAllocation = ( if (split.serialId !== undefined && !Number.isInteger(split.serialId)) { return 'A split serial ID is not valid.'; } - if ( - split.extraLogging !== undefined && - !isStringRecord(split.extraLogging) - ) { - return 'A split extraLogging field is not valid.'; - } const shardsError = validateShards(split.shards); if (shardsError) { @@ -378,7 +362,9 @@ const validateFlag = (value: unknown): string | undefined => { return undefined; }; -const validateRulesConfiguration = (value: unknown): string | undefined => { +const validateRulesConfigurationEnvelope = ( + value: unknown +): string | undefined => { if ( !isRecord(value) || typeof value.createdAt !== 'string' || @@ -390,13 +376,6 @@ const validateRulesConfiguration = (value: unknown): string | undefined => { return 'The rules configuration has an invalid envelope.'; } - for (const flag of Object.values(value.flags)) { - const error = validateFlag(flag); - if (error) { - return error; - } - } - return undefined; }; @@ -444,13 +423,21 @@ export const prepareRulesConfiguration = ( ): PreparedRulesConfiguration => { const clone = cloneValue(value); - // TODO(FFL-2837): Replace the temporary SDK validator with the published - // flagging-core validation API. Keep the branch-level result contract. - const errorMessage = validateRulesConfiguration(clone); + // TODO(FFL-2837): Delete this legacy JSON clone and validator after a + // flagging-core release contains upstream PR #344. That implementation + // decodes the protobuf response and omits unsupported or invalid flags. + const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; } + const flags = (clone as UniversalFlagConfigurationV1).flags; + for (const [flagKey, flag] of Object.entries(flags)) { + if (validateFlag(flag)) { + delete flags[flagKey]; + } + } + freezeValue(clone); return { status: 'ready', @@ -481,64 +468,21 @@ const normalizeVariationType = ( } }; -const recoverSplitMetadata = ( +const recoverVariationType = ( configuration: UniversalFlagConfigurationV1, - flagKey: string, - variant: string | undefined, - allocationKey: string | undefined, - splitSerialId: number | undefined -): Pick => { + flagKey: string +): RulesValueType | undefined => { const flags = configuration.flags as Record; if (!hasOwn(flags, flagKey)) { - return {}; + return undefined; } const flag = flags[flagKey]; if (!isRecord(flag)) { - return {}; - } - - const variationType = normalizeVariationType(flag.variationType); - if (!Array.isArray(flag.allocations)) { - return { variationType }; - } - - const variations = isRecord(flag.variations) ? flag.variations : {}; - const variationEntry = Object.entries(variations).find(([, value]) => { - return isRecord(value) && value.key === variant; - }); - const variationKey = variationEntry?.[0]; - - for (const allocation of flag.allocations) { - if ( - !isRecord(allocation) || - allocation.key !== allocationKey || - !Array.isArray(allocation.splits) - ) { - continue; - } - const split = allocation.splits.find(candidate => { - if (!isRecord(candidate)) { - return false; - } - if ( - splitSerialId !== undefined && - candidate.serialId === splitSerialId - ) { - return true; - } - return ( - splitSerialId === undefined && - variationKey !== undefined && - candidate.variationKey === variationKey - ); - }); - if (isRecord(split) && isStringRecord(split.extraLogging)) { - return { extraLogging: split.extraLogging, variationType }; - } + return undefined; } - return { variationType }; + return normalizeVariationType(flag.variationType); }; export const flaggingCoreRulesEngine: RulesEngine = { @@ -570,21 +514,6 @@ export const flaggingCoreRulesEngine: RulesEngine = { : typeof rawMetadata.__dd_allocation_key === 'string' ? rawMetadata.__dd_allocation_key : undefined; - const splitSerialId = - typeof rawMetadata.__dd_split_serial_id === 'number' - ? rawMetadata.__dd_split_serial_id - : undefined; - - // TODO(FFL-2837): Remove this metadata lookup when flagging-core - // returns extraLogging and the original UFC variation type. - const recoveredMetadata = recoverSplitMetadata( - request.configuration, - request.flagKey, - result.variant, - allocationKey, - splitSerialId - ); - return { value: result.value, reason: result.reason, @@ -595,18 +524,15 @@ export const flaggingCoreRulesEngine: RulesEngine = { allocationKey, variationType: normalizeVariationType(rawMetadata.variationType) ?? - recoveredMetadata.variationType, + recoverVariationType( + request.configuration, + request.flagKey + ), doLog: typeof rawMetadata.doLog === 'boolean' ? rawMetadata.doLog : typeof rawMetadata.__dd_do_log === 'boolean' ? rawMetadata.__dd_do_log - : undefined, - extraLogging: recoveredMetadata.extraLogging, - splitSerialId, - evaluationTimestampMs: - typeof rawMetadata.__dd_eval_timestamp_ms === 'number' - ? rawMetadata.__dd_eval_timestamp_ms : undefined } }; diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index f47a6fb84..681f68b94 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -62,8 +62,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { source ) as PendingRulesConfiguration; - // TODO(FFL-2837): Delete this JSON compatibility shim after - // DataDog/openfeature-js-client#336 is published by flagging-core. + // TODO(FFL-2837): Delete this legacy JSON compatibility shim after a + // flagging-core release contains DataDog/openfeature-js-client#344. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -72,7 +72,7 @@ export const configurationFromString = (source: string): FlagsConfiguration => { response: JSON.parse(pendingRules.response) }; } catch { - return {}; + return configuration; } } @@ -85,19 +85,12 @@ export const configurationFromString = (source: string): FlagsConfiguration => { export const configurationToString = ( configuration: FlagsConfiguration ): string => { - const serialized = coreConfigurationToString(configuration); const pendingConfiguration = configuration as PendingRulesConfiguration; - if (!pendingConfiguration.rulesBased) { - return serialized; + if (pendingConfiguration.rulesBased) { + throw new Error( + 'Rules configurations cannot be serialized to the wire format' + ); } - // TODO(FFL-2837): Delete this JSON compatibility shim after - // DataDog/openfeature-js-client#336 is published by flagging-core. - const wire = JSON.parse(serialized) as PendingRulesWire; - wire.rulesBased = { - fetchedAt: pendingConfiguration.rulesBased.fetchedAt, - etag: pendingConfiguration.rulesBased.etag, - response: JSON.stringify(pendingConfiguration.rulesBased.response) - }; - return JSON.stringify(wire); + return coreConfigurationToString(configuration); }; From a6264ea85ab514808f78c05c696a37933197f901 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 28 Jul 2026 14:55:17 -0400 Subject: [PATCH 03/20] fix(flags): refresh upstream compatibility TODOs --- .../configuration/__tests__/rules.test.ts | 33 +++++++++++++++++++ .../core/src/flags/configuration/rules.ts | 30 ++++++++++++----- packages/core/src/flags/configuration/wire.ts | 11 +++++-- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index aeb3abdbf..58488478f 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -174,6 +174,39 @@ describe('rules configuration', () => { }); }); + it.each([ + ['INTEGER', 42], + ['NUMERIC', 1.5] + ] as const)( + 'normalizes %s variation metadata to number', + (variationType, variationValue) => { + const configuration = buildRulesConfiguration(); + const flag = configuration.flags['dynamic-flag']; + flag.variationType = variationType; + flag.variations.enabled.value = variationValue; + flag.variations.disabled.value = 0; + + const result = flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'number', + flagKey: 'dynamic-flag', + defaultValue: 0, + context: { + targetingKey: 'user-1', + country: 'US' + }, + logger: getNoopRulesLogger() + }); + + expect(result).toMatchObject({ + value: variationValue, + metadata: { + variationType: 'number' + } + }); + } + ); + it.each(['toString', 'constructor', '__proto__'])( 'checks own properties before it evaluates %s', flagKey => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 0ee103dd5..024f2060b 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -12,6 +12,11 @@ import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; +// TODO(FFL-2837): Replace this legacy UFC v1 alias with +// `NonNullable['response']` after a flagging-core +// release contains DataDog/openfeature-js-client#344. +type RulesConfigurationResponse = UniversalFlagConfigurationV1; + export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; type RulesValueByType = { @@ -49,7 +54,7 @@ export interface RulesEvaluationDetails { } export interface RulesEvaluationRequest { - configuration: UniversalFlagConfigurationV1; + configuration: RulesConfigurationResponse; type: T; flagKey: string; defaultValue: RulesValueByType[T]; @@ -73,7 +78,7 @@ type RawEvaluationDetails = { }; type EvaluateRules = ( - configuration: UniversalFlagConfigurationV1, + configuration: RulesConfigurationResponse, type: T, flagKey: string, defaultValue: RulesValueByType[T], @@ -187,7 +192,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 validates - // regular expression syntax, but it does not limit expensive patterns. + // the protobuf indexes, but it does not limit expensive patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -408,7 +413,7 @@ const freezeValue = (value: unknown): void => { export type PreparedRulesConfiguration = | { status: 'ready'; - configuration: UniversalFlagConfigurationV1; + configuration: RulesConfigurationResponse; } | { status: 'error'; @@ -425,13 +430,14 @@ export const prepareRulesConfiguration = ( // TODO(FFL-2837): Delete this legacy JSON clone and validator after a // flagging-core release contains upstream PR #344. That implementation - // decodes the protobuf response and omits unsupported or invalid flags. + // decodes a generated Protobuf-ES response and omits unsupported or invalid + // flags. Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; } - const flags = (clone as UniversalFlagConfigurationV1).flags; + const flags = (clone as RulesConfigurationResponse).flags; for (const [flagKey, flag] of Object.entries(flags)) { if (validateFlag(flag)) { delete flags[flagKey]; @@ -441,7 +447,7 @@ export const prepareRulesConfiguration = ( freezeValue(clone); return { status: 'ready', - configuration: clone as UniversalFlagConfigurationV1 + configuration: clone as RulesConfigurationResponse }; }; @@ -468,8 +474,12 @@ const normalizeVariationType = ( } }; +// TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the +// flagging-core dependency contains DataDog/openfeature-js-client#344. +// The protobuf evaluator supplies `variationType` and maps integer and numeric +// variations to the OpenFeature type `number`. const recoverVariationType = ( - configuration: UniversalFlagConfigurationV1, + configuration: RulesConfigurationResponse, flagKey: string ): RulesValueType | undefined => { const flags = configuration.flags as Record; @@ -490,6 +500,10 @@ export const flaggingCoreRulesEngine: RulesEngine = { request: RulesEvaluationRequest ): RulesEvaluationDetails { const flags = request.configuration.flags as Record; + + // TODO(FFL-2837): Delete this local compatibility guard after the + // flagging-core dependency contains DataDog/openfeature-js-client#344. + // Keep the reserved-name contract tests for the upstream implementation. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 681f68b94..7a3fe7cd1 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -18,6 +18,9 @@ import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; +// TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers +// after a flagging-core release contains DataDog/openfeature-js-client#344. +// Re-export the upstream functions and use `FlagsConfiguration.rules`. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -62,8 +65,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { source ) as PendingRulesConfiguration; - // TODO(FFL-2837): Delete this legacy JSON compatibility shim after a - // flagging-core release contains DataDog/openfeature-js-client#344. + // TODO(FFL-2837): Delete this legacy JSON compatibility shim with the + // pending types above. The upstream parser decodes `rules.response` as a + // generated Protobuf-ES message. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -86,6 +90,9 @@ export const configurationToString = ( configuration: FlagsConfiguration ): string => { const pendingConfiguration = configuration as PendingRulesConfiguration; + + // TODO(FFL-2837): Delete this local serialization guard with the pending + // types above. PR #344 makes the upstream serializer reject `rules`. if (pendingConfiguration.rulesBased) { throw new Error( 'Rules configurations cannot be serialized to the wire format' From 7c40f7c6d621c20b3f84be90dba34175a788fb57 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 28 Jul 2026 18:37:17 -0400 Subject: [PATCH 04/20] test(flags): enforce portable wire boundary --- .../configuration/__tests__/wire.test.ts | 14 ++++++++++++++ packages/core/src/flags/configuration/wire.ts | 19 +++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index 9be51f2b6..dc81e8a4b 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -92,6 +92,20 @@ describe('configurationFromString', () => { expect(configurationFromString('not json')).toEqual({}); }); + it('does not treat a raw protobuf response as a portable wire', () => { + // A service or distribution layer must put one base64 encoding of + // these bytes in a version 1 `rules.response` JSON envelope. + const rawProtobufAsBase64 = 'CgR0ZXN0'; + + expect(configurationFromString(rawProtobufAsBase64)).toEqual({}); + }); + + it('does not treat the legacy UFC JSON response as a portable wire', () => { + const legacyServiceResponse = JSON.stringify(buildRulesConfiguration()); + + expect(configurationFromString(legacyServiceResponse)).toEqual({}); + }); + it('returns an empty config when the inner response is invalid JSON', () => { const wire = JSON.stringify({ version: 1, diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 7a3fe7cd1..9ebc5d79b 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -5,9 +5,11 @@ */ // Wire (de)serialization is reused from `@datadog/flagging-core` (the canonical -// implementation) rather than reimplemented here. `configurationFromString` is lenient: -// it returns an empty configuration (`{}`) for malformed input or an unsupported wire -// version rather than throwing. `configurationToString` is the inverse (its fix from +// implementation) rather than reimplemented here. The input is the complete portable JSON +// envelope. It is not the raw protobuf or legacy JSON response from the UFC service. +// `configurationFromString` is lenient: it returns an empty configuration (`{}`) for +// malformed input or an unsupported wire version rather than throwing. +// `configurationToString` is the inverse (its fix from // https://github.com/DataDog/openfeature-js-client/pull/331 shipped in flagging-core 2.0.0). import { configurationFromString as coreConfigurationFromString, @@ -20,7 +22,10 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344. -// Re-export the upstream functions and use `FlagsConfiguration.rules`. +// Re-export the upstream functions and use `FlagsConfiguration.rules`. The +// distribution layer must put one base64 encoding of the raw dd-source#34959 +// protobuf response in the version 1 `rules.response` field. Do not add that +// service transport or envelope construction here. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -67,7 +72,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // TODO(FFL-2837): Delete this legacy JSON compatibility shim with the // pending types above. The upstream parser decodes `rules.response` as a - // generated Protobuf-ES message. + // generated Protobuf-ES message. Do not adapt this shim to decode a raw + // service response or to add a base64 layer. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -92,7 +98,8 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this local serialization guard with the pending - // types above. PR #344 makes the upstream serializer reject `rules`. + // types above. PR #344 makes the upstream serializer reject `rules`. The + // parsed protobuf does not contain the original portable-wire bytes. if (pendingConfiguration.rulesBased) { throw new Error( 'Rules configurations cannot be serialized to the wire format' From fb3020d14563c19788ddeb1bfb70868f79f89c13 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 28 Jul 2026 20:24:45 -0400 Subject: [PATCH 05/20] docs(flags): align parser migration TODOs --- packages/core/src/flags/configuration/rules.ts | 3 ++- packages/core/src/flags/configuration/wire.ts | 18 +++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 024f2060b..76ebf3770 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,7 +14,8 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344. +// release contains DataDog/openfeature-js-client#344. Keep the +// `FlagsConfiguration` type import on the flagging-core package root. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 9ebc5d79b..0d31d43a8 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -4,8 +4,9 @@ * Copyright 2016-Present Datadog, Inc. */ -// Wire (de)serialization is reused from `@datadog/flagging-core` (the canonical -// implementation) rather than reimplemented here. The input is the complete portable JSON +// Published flagging-core 2.0.2 exports wire conversion from its package root. PR #344 moves +// that conversion to the opt-in `@datadog/flagging-core/configuration` entry point so the default +// entry point does not load Protobuf-ES. In both versions, the input is the complete portable JSON // envelope. It is not the raw protobuf or legacy JSON response from the UFC service. // `configurationFromString` is lenient: it returns an empty configuration (`{}`) for // malformed input or an unsupported wire version rather than throwing. @@ -22,10 +23,12 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344. -// Re-export the upstream functions and use `FlagsConfiguration.rules`. The -// distribution layer must put one base64 encoding of the raw dd-source#34959 -// protobuf response in the version 1 `rules.response` field. Do not add that -// service transport or envelope construction here. +// Import and re-export the wire functions and `FlagsConfigurationWire` type from +// `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules +// evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution +// layer must put one base64 encoding of the raw dd-source#34959 protobuf response +// in the version 1 `rules.response` field. Do not add that service transport or +// envelope construction here. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -73,7 +76,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // TODO(FFL-2837): Delete this legacy JSON compatibility shim with the // pending types above. The upstream parser decodes `rules.response` as a // generated Protobuf-ES message. Do not adapt this shim to decode a raw - // service response or to add a base64 layer. + // service response or to add a base64 layer. Do not copy the strict base64 + // validator that PR #344 removed in favor of the Protobuf-ES decoder. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { From e66a244bb508fbd525695ea3dfa9301cdb9483bf Mon Sep 17 00:00:00 2001 From: Blake Date: Wed, 29 Jul 2026 10:20:21 -0400 Subject: [PATCH 06/20] fix(flags): preserve invalid rules errors --- .../configuration/__tests__/rules.test.ts | 91 +++++++++++++++++-- .../core/src/flags/configuration/rules.ts | 46 ++++++++-- packages/core/src/flags/configuration/wire.ts | 6 +- 3 files changed, 127 insertions(+), 16 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 58488478f..496d7c42a 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -66,7 +66,7 @@ describe('rules configuration', () => { ).toBe(true); }); - it('omits a flag that uses an unsupported operator', () => { + it('preserves a flag with an unsupported operator and reports PARSE_ERROR', () => { const source = buildRulesConfiguration(); const condition = source.flags['dynamic-flag'].allocations[0].rules?.[0] @@ -83,10 +83,25 @@ describe('rules configuration', () => { if (prepared.status !== 'ready') { throw new Error(prepared.errorMessage); } - expect(prepared.configuration.flags).toEqual({}); + expect(prepared.configuration.flags).toHaveProperty('dynamic-flag'); + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'PARSE_ERROR', + errorMessage: expect.stringContaining('FUTURE_OPERATOR') + }); }); - it('omits a flag that contains an invalid regular expression', () => { + it('reports PARSE_ERROR for a flag with an invalid regular expression', () => { const source = buildRulesConfiguration(); const conditions = source.flags['dynamic-flag'].allocations[0].rules?.[0].conditions; @@ -105,10 +120,22 @@ describe('rules configuration', () => { if (prepared.status !== 'ready') { throw new Error(prepared.errorMessage); } - expect(prepared.configuration.flags).toEqual({}); + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ + errorCode: 'PARSE_ERROR', + errorMessage: 'A regular expression condition is not valid.' + }); }); - it('omits a flag whose split points to an absent variation', () => { + it('reports PARSE_ERROR when a split points to an absent variation', () => { const source = buildRulesConfiguration(); source.flags['dynamic-flag'].allocations[0].splits[0].variationKey = 'absent'; @@ -119,10 +146,22 @@ describe('rules configuration', () => { if (prepared.status !== 'ready') { throw new Error(prepared.errorMessage); } - expect(prepared.configuration.flags).toEqual({}); + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ + errorCode: 'PARSE_ERROR', + errorMessage: 'A split has an invalid variation key.' + }); }); - it('keeps valid flags when it omits an invalid flag', () => { + it('keeps valid flags usable when another flag has a parse error', () => { const source = buildRulesConfiguration(); const validFlag = buildRulesConfiguration().flags['dynamic-flag']; validFlag.key = 'valid-flag'; @@ -143,8 +182,46 @@ describe('rules configuration', () => { throw new Error(prepared.errorMessage); } expect(Object.keys(prepared.configuration.flags)).toEqual([ + 'dynamic-flag', 'valid-flag' ]); + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'valid-flag', + defaultValue: false, + context: { targetingKey: 'user-1', country: 'US' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: true, errorCode: undefined }); + }); + + // TODO(FFL-2837): Replace this legacy JSON compatibility test with a + // generated protobuf fixture after a flagging-core release contains + // DataDog/openfeature-js-client#344 at or after `be0d886`. + it('keeps supported known data when an unknown field is present', () => { + const source = buildRulesConfiguration(); + (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { + futureField: string; + }).futureField = 'ignored'; + + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1', country: 'US' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: true, errorCode: undefined }); }); it('normalizes a real flagging-core evaluation', () => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 76ebf3770..8e5d7a528 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -15,7 +15,8 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core // release contains DataDog/openfeature-js-client#344. Keep the -// `FlagsConfiguration` type import on the flagging-core package root. +// `FlagsConfiguration` type import on the flagging-core package root. PR #344 +// now preserves invalid flags and reports their stored errors during evaluation. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -125,6 +126,15 @@ export const toRulesEvaluationContext = ( const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); +// TODO(FFL-2837): Delete this compatibility error store after a flagging-core +// release contains DataDog/openfeature-js-client#344 at or after `ba1dbaf`. +// The generated protobuf parser uses the same per-configuration error model, +// and its evaluator returns `PARSE_ERROR` with the stored validation message. +const errorsByConfiguration = new WeakMap< + RulesConfigurationResponse, + ReadonlyMap +>(); + const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); @@ -431,24 +441,31 @@ export const prepareRulesConfiguration = ( // TODO(FFL-2837): Delete this legacy JSON clone and validator after a // flagging-core release contains upstream PR #344. That implementation - // decodes a generated Protobuf-ES response and omits unsupported or invalid - // flags. Do not adapt this validator to the generated response type. + // decodes a generated Protobuf-ES response, preserves invalid flags, and + // records per-flag errors for evaluation. Do not adapt this validator to + // the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; } - const flags = (clone as RulesConfigurationResponse).flags; + const configuration = clone as RulesConfigurationResponse; + const flags = configuration.flags; + const errors = new Map(); for (const [flagKey, flag] of Object.entries(flags)) { - if (validateFlag(flag)) { - delete flags[flagKey]; + const flagError = validateFlag(flag); + if (flagError) { + errors.set(flagKey, flagError); } } freezeValue(clone); + if (errors.size > 0) { + errorsByConfiguration.set(configuration, errors); + } return { status: 'ready', - configuration: clone as RulesConfigurationResponse + configuration }; }; @@ -514,6 +531,21 @@ export const flaggingCoreRulesEngine: RulesEngine = { }; } + // TODO(FFL-2837): Delete this compatibility check with the local error + // store after the published PR #344 evaluator reports parser errors. + const configurationError = errorsByConfiguration + .get(request.configuration) + ?.get(request.flagKey); + if (configurationError) { + return { + value: request.defaultValue, + reason: 'ERROR', + errorCode: 'PARSE_ERROR', + errorMessage: configurationError, + metadata: {} + }; + } + const result = evaluateRules( request.configuration, request.type, diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 0d31d43a8..cfbef5abc 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -28,7 +28,8 @@ import type { // evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution // layer must put one base64 encoding of the raw dd-source#34959 protobuf response // in the version 1 `rules.response` field. Do not add that service transport or -// envelope construction here. +// envelope construction here. PR #344 preserves invalid protobuf flags and +// reports their validation errors when the flag is evaluated. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -77,7 +78,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // pending types above. The upstream parser decodes `rules.response` as a // generated Protobuf-ES message. Do not adapt this shim to decode a raw // service response or to add a base64 layer. Do not copy the strict base64 - // validator that PR #344 removed in favor of the Protobuf-ES decoder. + // validator that PR #344 removed in favor of the Protobuf-ES decoder. The + // published parser must also include PR #344's unknown-field tolerance. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { From 521a0cf4372b752c8dfdae06addd5b5b9ca51e0d Mon Sep 17 00:00:00 2001 From: Blake Date: Wed, 29 Jul 2026 12:22:48 -0400 Subject: [PATCH 07/20] fix(flags): reject unsafe rules integers --- .../configuration/__tests__/rules.test.ts | 71 ++++++++++++++++++- .../core/src/flags/configuration/rules.ts | 52 +++++++++----- packages/core/src/flags/configuration/wire.ts | 11 +-- 3 files changed, 113 insertions(+), 21 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 496d7c42a..634e3133c 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -199,7 +199,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 at or after `be0d886`. + // DataDog/openfeature-js-client#344 at or after `4f6f40c`. it('keeps supported known data when an unknown field is present', () => { const source = buildRulesConfiguration(); (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { @@ -224,6 +224,75 @@ describe('rules configuration', () => { ).toMatchObject({ value: true, errorCode: undefined }); }); + // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range + // protobuf `int64` fixture after flagging-core contains PR #344 at or after + // `4f6f40c`. The generated parser must preserve the source value as `bigint`. + it('returns PARSE_ERROR instead of serving an unsafe integer', () => { + const source = buildRulesConfiguration(); + const flag = source.flags['dynamic-flag']; + flag.variationType = 'INTEGER'; + flag.variations.enabled.value = Number.MAX_SAFE_INTEGER + 1; + flag.variations.disabled.value = 0; + + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect( + prepared.configuration.flags['dynamic-flag'].variations.enabled + .value + ).toBe(Number.MAX_SAFE_INTEGER + 1); + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'number', + flagKey: 'dynamic-flag', + defaultValue: 0, + context: { targetingKey: 'user-1', country: 'US' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ + value: 0, + reason: 'ERROR', + errorCode: 'PARSE_ERROR', + errorMessage: + 'Integer variation value cannot be represented safely as a JavaScript number' + }); + }); + + it('returns PARSE_ERROR for an unsafe shard integer', () => { + const source = buildRulesConfiguration(); + source.flags[ + 'dynamic-flag' + ].allocations[0].splits[0].shards[0].totalShards = + Number.MAX_SAFE_INTEGER + 1; + + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect( + flaggingCoreRulesEngine.evaluate({ + configuration: prepared.configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1', country: 'US' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'PARSE_ERROR', + errorMessage: + 'Protobuf uint64 cannot be represented safely as a JavaScript number' + }); + }); + it('normalizes a real flagging-core evaluation', () => { const configuration = buildRulesConfiguration(); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 8e5d7a528..f0bc143f6 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,9 +14,10 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344. Keep the +// release contains DataDog/openfeature-js-client#344 through `4f6f40c`. Keep the // `FlagsConfiguration` type import on the flagging-core package root. PR #344 -// now preserves invalid flags and reports their stored errors during evaluation. +// preserves protobuf integers as `bigint`, and it reports unsafe conversions as +// stored per-flag errors during evaluation. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -127,9 +128,9 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 at or after `ba1dbaf`. +// release contains DataDog/openfeature-js-client#344 at or after `4f6f40c`. // The generated protobuf parser uses the same per-configuration error model, -// and its evaluator returns `PARSE_ERROR` with the stored validation message. +// including `PARSE_ERROR` for an integer that is not a safe JavaScript number. const errorsByConfiguration = new WeakMap< RulesConfigurationResponse, ReadonlyMap @@ -168,6 +169,7 @@ const variationValueIsValid = ( case 'STRING': return typeof value === 'string'; case 'INTEGER': + return typeof value === 'number' && Number.isSafeInteger(value); case 'NUMERIC': return typeof value === 'number' && Number.isFinite(value); case 'JSON': @@ -202,8 +204,8 @@ const validateCondition = (value: unknown): string | undefined => { } try { // TODO(FFL-2837): Define a bounded regular expression policy before - // dynamic offline rules leave draft state. Upstream PR #344 validates - // the protobuf indexes, but it does not limit expensive patterns. + // dynamic offline rules leave draft state. Upstream PR #344 through + // `4f6f40c` validates protobuf data, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -270,6 +272,9 @@ const validateShards = (value: unknown): string | undefined => { ) { return 'A shard has an invalid shape.'; } + if (!Number.isSafeInteger(shard.totalShards)) { + return 'Protobuf uint64 cannot be represented safely as a JavaScript number'; + } for (const range of shard.ranges) { if ( @@ -282,6 +287,12 @@ const validateShards = (value: unknown): string | undefined => { ) { return 'A shard range is not valid.'; } + if ( + !Number.isSafeInteger(range.start) || + !Number.isSafeInteger(range.end) + ) { + return 'Protobuf uint64 cannot be represented safely as a JavaScript number'; + } } } @@ -359,6 +370,14 @@ const validateFlag = (value: unknown): string | undefined => { } for (const variation of Object.values(value.variations)) { + if ( + value.variationType === 'INTEGER' && + isRecord(variation) && + typeof variation.value === 'number' && + !Number.isSafeInteger(variation.value) + ) { + return 'Integer variation value cannot be represented safely as a JavaScript number'; + } if ( !isRecord(variation) || typeof variation.key !== 'string' || @@ -440,10 +459,10 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344. That implementation - // decodes a generated Protobuf-ES response, preserves invalid flags, and - // records per-flag errors for evaluation. Do not adapt this validator to - // the generated response type. + // flagging-core release contains upstream PR #344 through `4f6f40c`. That + // implementation preserves protobuf integers as `bigint` and records a + // per-flag error when evaluation cannot produce a safe JavaScript number. + // Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; @@ -493,9 +512,9 @@ const normalizeVariationType = ( }; // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the -// flagging-core dependency contains DataDog/openfeature-js-client#344. -// The protobuf evaluator supplies `variationType` and maps integer and numeric -// variations to the OpenFeature type `number`. +// flagging-core dependency contains DataDog/openfeature-js-client#344 through +// `4f6f40c`. The protobuf evaluator maps only safely represented integer +// variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, flagKey: string @@ -520,8 +539,8 @@ export const flaggingCoreRulesEngine: RulesEngine = { const flags = request.configuration.flags as Record; // TODO(FFL-2837): Delete this local compatibility guard after the - // flagging-core dependency contains DataDog/openfeature-js-client#344. - // Keep the reserved-name contract tests for the upstream implementation. + // flagging-core dependency contains DataDog/openfeature-js-client#344 + // through `4f6f40c`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -532,7 +551,8 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator reports parser errors. + // store after the published PR #344 evaluator at or after `4f6f40c` + // reports parser errors, including unsafe integer conversions. const configurationError = errorsByConfiguration .get(request.configuration) ?.get(request.flagKey); diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index cfbef5abc..33f4e1586 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -22,14 +22,15 @@ import type { } from '@datadog/flagging-core'; // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers -// after a flagging-core release contains DataDog/openfeature-js-client#344. +// after a flagging-core release contains DataDog/openfeature-js-client#344 +// through `4f6f40c`. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution // layer must put one base64 encoding of the raw dd-source#34959 protobuf response // in the version 1 `rules.response` field. Do not add that service transport or // envelope construction here. PR #344 preserves invalid protobuf flags and -// reports their validation errors when the flag is evaluated. +// protobuf integers, and reports unsafe integer conversion when evaluated. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -79,7 +80,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // generated Protobuf-ES message. Do not adapt this shim to decode a raw // service response or to add a base64 layer. Do not copy the strict base64 // validator that PR #344 removed in favor of the Protobuf-ES decoder. The - // published parser must also include PR #344's unknown-field tolerance. + // published parser must also include PR #344's unknown-field tolerance and + // lossless integer parsing through `4f6f40c`. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -105,7 +107,8 @@ export const configurationToString = ( // TODO(FFL-2837): Delete this local serialization guard with the pending // types above. PR #344 makes the upstream serializer reject `rules`. The - // parsed protobuf does not contain the original portable-wire bytes. + // parsed protobuf does not contain the original portable-wire bytes and can + // contain `bigint` values after `4f6f40c`. if (pendingConfiguration.rulesBased) { throw new Error( 'Rules configurations cannot be serialized to the wire format' From ddc087676337baa3cd874141c4ac601af28040c0 Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 30 Jul 2026 08:35:16 -0400 Subject: [PATCH 08/20] fix(flags): align compatibility with upstream rules --- .../configuration/__tests__/rules.test.ts | 7 ++-- .../configuration/__tests__/wire.test.ts | 20 ++++++----- .../core/src/flags/configuration/rules.ts | 33 +++++++++++-------- packages/core/src/flags/configuration/wire.ts | 28 +++++++++------- 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 634e3133c..f026b9619 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -97,7 +97,8 @@ describe('rules configuration', () => { value: false, reason: 'ERROR', errorCode: 'PARSE_ERROR', - errorMessage: expect.stringContaining('FUTURE_OPERATOR') + errorMessage: + 'The rules configuration uses an unsupported operator.' }); }); @@ -199,7 +200,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 at or after `4f6f40c`. + // DataDog/openfeature-js-client#344 through `41dff20`. it('keeps supported known data when an unknown field is present', () => { const source = buildRulesConfiguration(); (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { @@ -226,7 +227,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `4f6f40c`. The generated parser must preserve the source value as `bigint`. + // `41dff20`. The generated parser must preserve the source value as `bigint`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { const source = buildRulesConfiguration(); const flag = source.flags['dynamic-flag']; diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index dc81e8a4b..984bea29c 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -164,20 +164,24 @@ describe('temporary rules configuration wire compatibility', () => { expect(parsed.rulesBased).toEqual(rulesBased); }); - it('does not serialize a rules configuration', () => { - const configuration = { + it('round-trips a legacy rules configuration', () => { + const original = { rulesBased: { - response: buildRulesConfiguration() + response: buildRulesConfiguration(), + fetchedAt: 123, + etag: 'rules-etag' } }; - expect(() => + const restored = configurationFromString( configurationToString( - (configuration as unknown) as ParsedFlagsConfiguration + (original as unknown) as ParsedFlagsConfiguration ) - ).toThrow( - 'Rules configurations cannot be serialized to the wire format' - ); + ) as { + rulesBased?: typeof original.rulesBased; + }; + + expect(restored.rulesBased).toEqual(original.rulesBased); }); it('keeps both branches in a mixed configuration', () => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index f0bc143f6..40f642921 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,10 +14,10 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `4f6f40c`. Keep the +// release contains DataDog/openfeature-js-client#344 through `41dff20`. Keep the // `FlagsConfiguration` type import on the flagging-core package root. PR #344 -// preserves protobuf integers as `bigint`, and it reports unsafe conversions as -// stored per-flag errors during evaluation. +// preserves protobuf integers as `bigint`, and its evaluator reports unsafe +// conversions as deterministic per-flag `PARSE_ERROR` results. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -128,9 +128,11 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 at or after `4f6f40c`. -// The generated protobuf parser uses the same per-configuration error model, -// including `PARSE_ERROR` for an integer that is not a safe JavaScript number. +// release contains DataDog/openfeature-js-client#344 through `41dff20`. +// The generated protobuf evaluator validates the requested flag and the data +// that evaluation reaches. It does not build this error map during parsing. +// It returns deterministic `PARSE_ERROR` results, including for an integer that +// is not a safe JavaScript number. const errorsByConfiguration = new WeakMap< RulesConfigurationResponse, ReadonlyMap @@ -193,7 +195,7 @@ const validateCondition = (value: unknown): string | undefined => { } if (!SUPPORTED_OPERATORS.has(value.operator)) { - return `The rules configuration uses the unsupported operator "${value.operator}".`; + return 'The rules configuration uses an unsupported operator.'; } switch (value.operator) { @@ -205,7 +207,8 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `4f6f40c` validates protobuf data, but it does not limit patterns. + // `41dff20` compiles protobuf regular expressions lazily and caches + // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -459,8 +462,9 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `4f6f40c`. That - // implementation preserves protobuf integers as `bigint` and records a + // flagging-core release contains upstream PR #344 through `41dff20`. That + // implementation preserves protobuf integers as `bigint` and validates only + // the requested flag data that evaluation reaches. It returns a deterministic // per-flag error when evaluation cannot produce a safe JavaScript number. // Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); @@ -513,7 +517,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `4f6f40c`. The protobuf evaluator maps only safely represented integer +// `41dff20`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -540,7 +544,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `4f6f40c`. Keep the reserved-name contract tests. + // through `41dff20`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -551,8 +555,9 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator at or after `4f6f40c` - // reports parser errors, including unsafe integer conversions. + // store after the published PR #344 evaluator through `41dff20` validates + // reached flag data and reports deterministic errors, including unsafe + // integer conversions. const configurationError = errorsByConfiguration .get(request.configuration) ?.get(request.flagKey); diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 33f4e1586..01426a543 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -23,14 +23,15 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `4f6f40c`. +// through `41dff20`. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution // layer must put one base64 encoding of the raw dd-source#34959 protobuf response // in the version 1 `rules.response` field. Do not add that service transport or -// envelope construction here. PR #344 preserves invalid protobuf flags and -// protobuf integers, and reports unsafe integer conversion when evaluated. +// envelope construction here. PR #344 preserves decoded protobuf flags and +// protobuf integers. Its evaluator reports invalid reached data and unsafe +// integer conversion as deterministic `PARSE_ERROR` results. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -81,7 +82,7 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // service response or to add a base64 layer. Do not copy the strict base64 // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance and - // lossless integer parsing through `4f6f40c`. + // lossless integer parsing through `41dff20`. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -105,14 +106,19 @@ export const configurationToString = ( ): string => { const pendingConfiguration = configuration as PendingRulesConfiguration; - // TODO(FFL-2837): Delete this local serialization guard with the pending - // types above. PR #344 makes the upstream serializer reject `rules`. The - // parsed protobuf does not contain the original portable-wire bytes and can - // contain `bigint` values after `4f6f40c`. + // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending + // types above after the dependency contains PR #344 through `41dff20`. + // The upstream serializer encodes generated protobuf rules back to base64. + // This temporary UFC v1 shim serializes its legacy JSON response instead. if (pendingConfiguration.rulesBased) { - throw new Error( - 'Rules configurations cannot be serialized to the wire format' - ); + const serialized = JSON.parse( + coreConfigurationToString(configuration) + ) as PendingRulesWire; + serialized.rulesBased = { + ...pendingConfiguration.rulesBased, + response: JSON.stringify(pendingConfiguration.rulesBased.response) + }; + return JSON.stringify(serialized); } return coreConfigurationToString(configuration); From 55f897b812245bc98fd7cd373bbe6756adb5a8eb Mon Sep 17 00:00:00 2001 From: Blake Date: Fri, 31 Jul 2026 09:51:47 -0400 Subject: [PATCH 09/20] test(flags): preserve unknown rules fields --- .../configuration/__tests__/rules.test.ts | 7 ++++-- .../configuration/__tests__/wire.test.ts | 8 ++++++- .../core/src/flags/configuration/rules.ts | 22 +++++++++++-------- packages/core/src/flags/configuration/wire.ts | 12 +++++----- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index f026b9619..8eeba39f5 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -200,7 +200,8 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `41dff20`. + // DataDog/openfeature-js-client#344 through `41dff20`. Round-trip the + // generated fixture and confirm that serialization preserves the unknown field. it('keeps supported known data when an unknown field is present', () => { const source = buildRulesConfiguration(); (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { @@ -227,7 +228,9 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `41dff20`. The generated parser must preserve the source value as `bigint`. + // `41dff20` plus the no-`BigInt` follow-up. The generated parser must preserve + // the source value as `bigint` where supported. Run the same evaluation with + // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { const source = buildRulesConfiguration(); const flag = source.flags['dynamic-flag']; diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index 984bea29c..0e1cdc0b7 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -165,9 +165,15 @@ describe('temporary rules configuration wire compatibility', () => { }); it('round-trips a legacy rules configuration', () => { + const response = buildRulesConfiguration() as ReturnType< + typeof buildRulesConfiguration + > & { + futureField?: { value: number }; + }; + response.futureField = { value: 7 }; const original = { rulesBased: { - response: buildRulesConfiguration(), + response, fetchedAt: 123, etag: 'rules-etag' } diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 40f642921..f0f663b9f 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,10 +14,12 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `41dff20`. Keep the -// `FlagsConfiguration` type import on the flagging-core package root. PR #344 -// preserves protobuf integers as `bigint`, and its evaluator reports unsafe -// conversions as deterministic per-flag `PARSE_ERROR` results. +// release contains DataDog/openfeature-js-client#344 through `41dff20`, restores +// 32-byte SHA digest validation, and defines or fixes integer evaluation without +// global `BigInt`. Keep the `FlagsConfiguration` type import on the flagging-core +// package root. PR #344 preserves protobuf integers as `bigint`, and its evaluator +// reports unsafe conversions as deterministic per-flag `PARSE_ERROR` results when +// `BigInt` is available. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -128,7 +130,8 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `41dff20`. +// release contains DataDog/openfeature-js-client#344 through `41dff20` and fixes +// or explicitly excludes integer and shard evaluation without global `BigInt`. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. // It returns deterministic `PARSE_ERROR` results, including for an integer that @@ -462,10 +465,11 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `41dff20`. That + // flagging-core release contains upstream PR #344 through `41dff20` and the + // no-`BigInt` integer contract is fixed or declared unsupported. That // implementation preserves protobuf integers as `bigint` and validates only - // the requested flag data that evaluation reaches. It returns a deterministic - // per-flag error when evaluation cannot produce a safe JavaScript number. + // the requested flag data that evaluation reaches. With `BigInt`, it returns a + // deterministic per-flag error when evaluation cannot produce a safe number. // Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { @@ -557,7 +561,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this compatibility check with the local error // store after the published PR #344 evaluator through `41dff20` validates // reached flag data and reports deterministic errors, including unsafe - // integer conversions. + // integer conversions with and without global `BigInt` when supported. const configurationError = errorsByConfiguration .get(request.configuration) ?.get(request.flagKey); diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 01426a543..984a52021 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -23,7 +23,7 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `41dff20`. +// through `41dff20` plus the required SHA digest and no-`BigInt` follow-ups. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution @@ -81,8 +81,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // generated Protobuf-ES message. Do not adapt this shim to decode a raw // service response or to add a base64 layer. Do not copy the strict base64 // validator that PR #344 removed in favor of the Protobuf-ES decoder. The - // published parser must also include PR #344's unknown-field tolerance and - // lossless integer parsing through `41dff20`. + // published parser must also include PR #344's unknown-field tolerance, + // unknown-field serialization, and lossless integer parsing through + // `41dff20`, plus the final no-`BigInt` runtime decision. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -107,8 +108,9 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `41dff20`. - // The upstream serializer encodes generated protobuf rules back to base64. + // types above after the dependency contains PR #344 through `41dff20` and + // its required follow-ups. The upstream serializer encodes generated protobuf + // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. if (pendingConfiguration.rulesBased) { const serialized = JSON.parse( From 6171275951680f2a97c24a886ceb64a308d00d38 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 3 Aug 2026 09:37:18 -0400 Subject: [PATCH 10/20] docs(flags): refresh capability migration TODOs --- .../configuration/__tests__/rules.test.ts | 6 +++-- .../core/src/flags/configuration/rules.ts | 19 ++++++++-------- packages/core/src/flags/configuration/wire.ts | 22 +++++++++++-------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 8eeba39f5..2d8b6a431 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -200,8 +200,10 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `41dff20`. Round-trip the + // DataDog/openfeature-js-client#344 through `9f794c7`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. + // Add a fixture with an unsupported minimum feature level and require a + // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. it('keeps supported known data when an unknown field is present', () => { const source = buildRulesConfiguration(); (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { @@ -228,7 +230,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `41dff20` plus the no-`BigInt` follow-up. The generated parser must preserve + // `9f794c7` plus the no-`BigInt` follow-up. The generated parser must preserve // the source value as `bigint` where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index f0f663b9f..6949a9e96 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,7 +14,7 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `41dff20`, restores +// release contains DataDog/openfeature-js-client#344 through `9f794c7`, restores // 32-byte SHA digest validation, and defines or fixes integer evaluation without // global `BigInt`. Keep the `FlagsConfiguration` type import on the flagging-core // package root. PR #344 preserves protobuf integers as `bigint`, and its evaluator @@ -130,7 +130,7 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `41dff20` and fixes +// release contains DataDog/openfeature-js-client#344 through `9f794c7` and fixes // or explicitly excludes integer and shard evaluation without global `BigInt`. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. @@ -210,7 +210,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `41dff20` compiles protobuf regular expressions lazily and caches + // `9f794c7` compiles protobuf regular expressions lazily and caches // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { @@ -465,7 +465,7 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `41dff20` and the + // flagging-core release contains upstream PR #344 through `9f794c7` and the // no-`BigInt` integer contract is fixed or declared unsupported. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. With `BigInt`, it returns a @@ -521,7 +521,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `41dff20`. The protobuf evaluator maps only safely represented integer +// `9f794c7`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -548,7 +548,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `41dff20`. Keep the reserved-name contract tests. + // through `9f794c7`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -559,9 +559,10 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `41dff20` validates - // reached flag data and reports deterministic errors, including unsafe - // integer conversions with and without global `BigInt` when supported. + // store after the published PR #344 evaluator through `9f794c7` validates + // reached flag data and reports deterministic flag-scoped errors, including + // unsupported feature levels and unsafe integer conversions with and + // without global `BigInt` when supported. const configurationError = errorsByConfiguration .get(request.configuration) ?.get(request.flagKey); diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 984a52021..4a4d4e4df 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -23,15 +23,19 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `41dff20` plus the required SHA digest and no-`BigInt` follow-ups. +// through `9f794c7` plus the required SHA digest and no-`BigInt` follow-ups. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules -// evaluator on the package root. Use `FlagsConfiguration.rules`. The distribution -// layer must put one base64 encoding of the raw dd-source#34959 protobuf response -// in the version 1 `rules.response` field. Do not add that service transport or -// envelope construction here. PR #344 preserves decoded protobuf flags and -// protobuf integers. Its evaluator reports invalid reached data and unsafe -// integer conversion as deterministic `PARSE_ERROR` results. +// evaluator on the package root. The new `@datadog/flagging-core/precomputed` +// subpath is protobuf-free, ignores rules, and is not the parser for this module. +// Use `FlagsConfiguration.rules`. The distribution layer must put one base64 +// encoding of the raw dd-source#34959 protobuf response in the version 1 +// `rules.response` field. Record dd-source#40304 commit `071c4ad` as the schema +// revision and dd-source#34959 as the service producer path. Do not add that +// service transport or envelope construction here. PR #344 preserves decoded +// protobuf flags and integers. Its evaluator reports invalid reached data, +// unsupported feature levels, and unsafe integer conversion as deterministic +// flag-scoped `PARSE_ERROR` results. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -83,7 +87,7 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `41dff20`, plus the final no-`BigInt` runtime decision. + // `9f794c7`, plus the final no-`BigInt` runtime decision. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -108,7 +112,7 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `41dff20` and + // types above after the dependency contains PR #344 through `9f794c7` and // its required follow-ups. The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. From 33e391b2ceac374a77f9271ba282872dfb12d109 Mon Sep 17 00:00:00 2001 From: Blake Date: Wed, 5 Aug 2026 10:33:41 -0400 Subject: [PATCH 11/20] fix(openfeature): treat empty contexts literally --- example-new-architecture/App.tsx | 7 +- .../flags/flagsProvider.ts | 12 +- example/src/flags/flagsProvider.ts | 16 +- packages/core/src/flags/FlagsClient.ts | 9 +- .../src/flags/__tests__/FlagsClient.test.ts | 27 +++ .../configuration/__tests__/context.test.ts | 12 +- .../core/src/flags/configuration/context.ts | 18 +- packages/react-native-openfeature/README.md | 71 +++--- .../src/__tests__/configuration.test.ts | 119 ++++++++++ .../offlineProvider.integration.test.ts | 215 ++++++++++++------ .../src/__tests__/offlineProvider.test.ts | 40 ++-- .../src/configuration.ts | 64 ++++++ .../react-native-openfeature/src/index.ts | 4 +- .../react-native-openfeature/src/mappers.ts | 27 ++- .../src/offlineProvider.ts | 39 ++-- 15 files changed, 504 insertions(+), 176 deletions(-) create mode 100644 packages/react-native-openfeature/src/__tests__/configuration.test.ts create mode 100644 packages/react-native-openfeature/src/configuration.ts diff --git a/example-new-architecture/App.tsx b/example-new-architecture/App.tsx index f753110f7..9f2a7bb6a 100644 --- a/example-new-architecture/App.tsx +++ b/example-new-architecture/App.tsx @@ -95,10 +95,9 @@ import {APPLICATION_ID, CLIENT_TOKEN, ENVIRONMENT} from './ddCredentials'; })(); function AppWithProviders() { - // No OpenFeature.setContext here on purpose: the offline precomputed configuration is a - // single-subject snapshot served against the context it was computed for (see the wire's - // embedded context in flags/). Setting a different runtime context would put the provider into - // the OpenFeature ERROR state and fall back to coded defaults. + // setFlagsProvider gets a supported copy of the precomputed context and sets it on OpenFeature + // before provider registration. A later different context puts the offline provider into ERROR + // and evaluations use their coded defaults. return ( => { if (source === 'offline') { + const configuration = configurationFromString(buildSampleWire()); + const context = getPrecomputedContext(configuration); + + if (context !== undefined) { + await OpenFeature.setContext(context); + } + const provider = new DatadogOfflineOpenFeatureProvider({ clientName: 'offline', }); - provider.setConfiguration( - configurationFromString(buildSampleWire()), - ); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(provider); return; } diff --git a/example/src/flags/flagsProvider.ts b/example/src/flags/flagsProvider.ts index 70be0801d..7228345b4 100644 --- a/example/src/flags/flagsProvider.ts +++ b/example/src/flags/flagsProvider.ts @@ -1,7 +1,8 @@ import { DatadogOpenFeatureProvider, DatadogOfflineOpenFeatureProvider, - configurationFromString + configurationFromString, + getPrecomputedContext } from '@datadog/mobile-react-native-openfeature'; import { OpenFeature } from '@openfeature/react-sdk'; @@ -28,12 +29,19 @@ export const setFlagsProvider = async ( offlineContext?: OfflineWireContext ): Promise => { if (source === 'offline') { + const configuration = configurationFromString( + buildSampleWire(offlineContext) + ); + const context = getPrecomputedContext(configuration); + + if (context !== undefined) { + await OpenFeature.setContext(context); + } + const provider = new DatadogOfflineOpenFeatureProvider({ clientName: 'offline' }); - provider.setConfiguration( - configurationFromString(buildSampleWire(offlineContext)) - ); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(provider); return; } diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 059493dde..869806ebf 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -205,11 +205,10 @@ export class FlagsClient { /** * 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`. + * This is an explicit low-level Datadog reset operation. It drops the external override so a + * loaded precomputed configuration is served against **its embedded context** again. It does + * not represent OpenFeature `clearContext()`, which supplies the resulting effective context + * to a provider. With no configuration loaded the result is `PROVIDER_NOT_READY`. */ resetEvaluationContextWithoutFetching = (): ConfigurationResult => { this.externalContext = undefined; diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index 7df650bf9..bfd5a224f 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -774,6 +774,33 @@ describe('FlagsClient', () => { ).not.toHaveBeenCalled(); }); + it('stores an empty context as an explicit override', () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + + const result = flagsClient.setEvaluationContextWithoutFetching({ + attributes: {} + } as never); + + expect(result).toEqual({ + status: 'error', + errorCode: 'INVALID_CONTEXT' + }); + + // Reloading the snapshot reconciles against the stored empty override. It does not + // silently restore the snapshot's embedded user-1 context. + expect( + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ) + ).toEqual({ + status: 'error', + errorCode: 'INVALID_CONTEXT' + }); + }); + it('recovers to ready when a matching context is set after a mismatch', () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( diff --git a/packages/core/src/flags/configuration/__tests__/context.test.ts b/packages/core/src/flags/configuration/__tests__/context.test.ts index ec0155bad..e9654c4a4 100644 --- a/packages/core/src/flags/configuration/__tests__/context.test.ts +++ b/packages/core/src/flags/configuration/__tests__/context.test.ts @@ -28,9 +28,9 @@ describe('normalizeWireContext', () => { }); }); - it('defaults a missing targeting key to an empty string', () => { + it('preserves a missing targeting key', () => { expect(normalizeWireContext({ country: 'US' })).toEqual({ - targetingKey: '', + targetingKey: undefined, attributes: { country: 'US' } }); }); @@ -80,6 +80,14 @@ describe('contextMatchesConfiguration', () => { ).toBe(true); }); + it('matches empty contexts without inventing a targeting key', () => { + expect( + contextMatchesConfiguration({}, { + attributes: {} + } as EvaluationContext) + ).toBe(true); + }); + it('does not match a different targeting key', () => { expect( contextMatchesConfiguration( diff --git a/packages/core/src/flags/configuration/context.ts b/packages/core/src/flags/configuration/context.ts index 436ad96b5..1810d937c 100644 --- a/packages/core/src/flags/configuration/context.ts +++ b/packages/core/src/flags/configuration/context.ts @@ -19,14 +19,18 @@ export const normalizeWireContext = ( wireContext: WireEvaluationContext ): EvaluationContext => { const { targetingKey, ...attributes } = wireContext; - - return processEvaluationContext({ - // 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. + const context = { + // `processEvaluationContext` removes unsupported nested values from the attributes. attributes: attributes as Record - }); + } as EvaluationContext; + + // The wire is untrusted. Preserve a string (including an empty string), but do not invent a + // targeting key when it is absent or invalid. + if (typeof targetingKey === 'string') { + context.targetingKey = targetingKey; + } + + return processEvaluationContext(context); }; /** diff --git a/packages/react-native-openfeature/README.md b/packages/react-native-openfeature/README.md index 5fd9cca9a..285cae248 100644 --- a/packages/react-native-openfeature/README.md +++ b/packages/react-native-openfeature/README.md @@ -124,58 +124,65 @@ the network** — you supply it with `setConfiguration`. import { DdFlags } from '@datadog/mobile-react-native'; import { DatadogOfflineOpenFeatureProvider, - configurationFromString + configurationFromString, + getPrecomputedContext } from '@datadog/mobile-react-native-openfeature'; import { OpenFeature } from '@openfeature/react-sdk'; await DdFlags.enable(); -const provider = new DatadogOfflineOpenFeatureProvider(); +const domain = 'offline-flags'; +const configuration = configurationFromString(wire); +const context = getPrecomputedContext(configuration); + +// A context-specific precomputed configuration must use its matching OpenFeature context. +if (context !== undefined) { + await OpenFeature.setContext(domain, context); +} -// `wire` is a ConfigurationWire string you fetched yourself. -provider.setConfiguration(configurationFromString(wire)); +const provider = new DatadogOfflineOpenFeatureProvider(); +provider.setConfiguration(configuration); // Set the provider after loading the configuration so it is ready with real flag values. -await OpenFeature.setProviderAndWait(provider); +await OpenFeature.setProviderAndWait(domain, provider); // Evaluate flags — no network request is made. -const client = OpenFeature.getClient(); -const isNewFeatureEnabled = client.getBooleanValue('new-feature-enabled', false); +const client = OpenFeature.getClient(domain); +const isNewFeatureEnabled = client.getBooleanValue( + 'new-feature-enabled', + false +); ``` -The configuration carries the evaluation context it was computed for, and the provider adopts it -automatically. A precomputed configuration is a **single-subject snapshot**: it can only be served -against the context it was computed for. Per-context evaluation is a future (rules-based) capability. +A context-specific precomputed configuration is a **single-subject snapshot**. The effective +OpenFeature context must match the context that was used to compute the snapshot. Use +`getPrecomputedContext(configuration)` to get a detached copy through a supported API. Do not inspect +the parsed configuration or wire format. The helper does not call OpenFeature and does not change +provider state. + +If a context-specific snapshot does not match the effective context, the provider enters the +OpenFeature **`ERROR`** state. Evaluations return their coded default values with +`errorCode: INVALID_CONTEXT`. Set the matching context to recover the provider to `READY`. -> **Warning:** Do **not** call `OpenFeature.setContext` with a _different_ context for the offline -> precomputed flow. A runtime context that does not match the configuration's embedded context -> (compared after the SDK's context normalization, not raw deep-equality) cannot be served (offline -> never fetches), so the provider enters the OpenFeature **`ERROR`** state and evaluations fall back -> to your **coded default values** (evaluation `errorCode: INVALID_CONTEXT`). The provider recovers to -> `READY` once the effective context is empty or matches the snapshot again. Note that a blank -> `{ targetingKey: '' }` is **not** "empty" — an empty string is a real (anonymous) targeting key, a -> distinct subject that must match the snapshot; use `clearContext()` (or omit context) to fall back -> to the embedded context. +An empty context (`{}`) is a real OpenFeature context. It does not restore the context in the +configuration. An empty targeting key (`{ targetingKey: '' }`) is also a real context and is +different from a missing targeting key. A context-agnostic precomputed configuration has no +embedded context. `getPrecomputedContext` returns `undefined` for that configuration, and it can be +used with any effective context. Recommended setup for a hybrid app that also uses other OpenFeature providers, hooks, or domains: -- **Bind the offline provider to a dedicated OpenFeature domain, and give that domain an explicit - empty context** at registration (`OpenFeature.setContext(domain, {})`) — which this provider reads - as "no override, use the embedded context". A domain with no context of its own **inherits the - global context**, so a global `OpenFeature.setContext` (or a mismatching global context) would - otherwise reach the provider and force it into `ERROR`. +- **Bind the offline provider to a dedicated OpenFeature domain.** Set the helper context on that + domain before provider registration. A domain with no context of its own inherits the global + context. - **Use a unique Datadog `clientName`** (`new DatadogOfflineOpenFeatureProvider({ clientName })`): separate OpenFeature domains otherwise share the same underlying `DdFlags.getClient('default')`, and an online provider on that shared client would discard the offline configuration. -Because you do not set an OpenFeature context, note the **context split**: OpenFeature hooks observe -the OpenFeature evaluation context (`{}` when unset), while Datadog exposure tracking attributes -evaluations to the configuration's embedded context. - -> **Note (recovery caveat):** "clearing context recovers" holds only when the resulting *effective* -> context is empty or matches the snapshot. `OpenFeature.clearContext(domain)` removes the domain -> context and **falls back to the global context** — if that global context is non-empty and does not -> match, the provider stays in `ERROR`. +`OpenFeature.clearContext(domain)` removes the domain context and uses the global context. If the +global context is empty or does not match a context-specific snapshot, the provider enters `ERROR`. +Call `OpenFeature.setContext(domain, matchingContext)` to recover. A global `clearContext()` supplies +`{}` to the provider; it does not restore the context in the configuration. > **Note (startup order):** Load the configuration with `setConfiguration` _before_ > `setProviderAndWait`, as shown above. If you register the provider before any successful diff --git a/packages/react-native-openfeature/src/__tests__/configuration.test.ts b/packages/react-native-openfeature/src/__tests__/configuration.test.ts new file mode 100644 index 000000000..00565c195 --- /dev/null +++ b/packages/react-native-openfeature/src/__tests__/configuration.test.ts @@ -0,0 +1,119 @@ +/* + * 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 { ParsedFlagsConfiguration } from '@datadog/mobile-react-native'; +import type { EvaluationContext } from '@openfeature/web-sdk'; + +import { getPrecomputedContext } from '../configuration'; + +const configurationWithContext = ( + context?: EvaluationContext +): ParsedFlagsConfiguration => { + return { + precomputed: { + response: { + data: { + attributes: { + createdAt: '2026-08-05T00:00:00.000Z', + flags: {} + } + } + }, + ...(context === undefined ? {} : { context }) + } + }; +}; + +describe('getPrecomputedContext', () => { + it('returns the context from a precomputed configuration', () => { + const configuration = configurationWithContext({ + targetingKey: 'user-1', + country: 'US' + }); + + expect(getPrecomputedContext(configuration)).toEqual({ + targetingKey: 'user-1', + country: 'US' + }); + }); + + it('preserves an explicit empty context and an empty targeting key', () => { + expect(getPrecomputedContext(configurationWithContext({}))).toEqual({}); + expect( + getPrecomputedContext( + configurationWithContext({ targetingKey: '' }) + ) + ).toEqual({ targetingKey: '' }); + }); + + it('returns a deep copy of the context', () => { + const date = new Date('2026-08-05T00:00:00.000Z'); + const configuration = configurationWithContext({ + targetingKey: 'user-1', + profile: { + groups: ['beta', { name: 'mobile' }], + enrolledAt: date + } + }); + + const first = getPrecomputedContext(configuration) as EvaluationContext; + const firstProfile = first.profile as { + groups: Array; + enrolledAt: Date; + }; + firstProfile.groups[1] = { name: 'changed' }; + firstProfile.enrolledAt.setUTCFullYear(2030); + + const second = getPrecomputedContext(configuration); + expect(second).toEqual({ + targetingKey: 'user-1', + profile: { + groups: ['beta', { name: 'mobile' }], + enrolledAt: date + } + }); + expect(second).not.toBe(first); + expect((second?.profile as { groups: unknown[] }).groups).not.toBe( + firstProfile.groups + ); + expect((second?.profile as { enrolledAt: Date }).enrolledAt).not.toBe( + date + ); + }); + + it.each([ + ['an empty configuration', {}], + ['a rules-only configuration', { rulesBased: { response: {} } }], + [ + 'an invalid precomputed branch with valid rules', + { + precomputedError: new Error('invalid precomputed branch'), + rulesBased: { response: {} } + } + ] + ])('returns undefined for %s', (_name, configuration) => { + expect( + getPrecomputedContext(configuration as ParsedFlagsConfiguration) + ).toBeUndefined(); + }); + + it('returns undefined for context-agnostic precomputed configuration', () => { + expect( + getPrecomputedContext(configurationWithContext()) + ).toBeUndefined(); + }); + + it('returns the precomputed context from a mixed configuration', () => { + const configuration = { + ...configurationWithContext({ targetingKey: 'user-1' }), + rulesBased: { response: {} } + } as ParsedFlagsConfiguration; + + expect(getPrecomputedContext(configuration)).toEqual({ + targetingKey: 'user-1' + }); + }); +}); diff --git a/packages/react-native-openfeature/src/__tests__/offlineProvider.integration.test.ts b/packages/react-native-openfeature/src/__tests__/offlineProvider.integration.test.ts index 6c5d2d4d2..08ff5b726 100644 --- a/packages/react-native-openfeature/src/__tests__/offlineProvider.integration.test.ts +++ b/packages/react-native-openfeature/src/__tests__/offlineProvider.integration.test.ts @@ -13,7 +13,9 @@ import { configurationFromString } from '@datadog/mobile-react-native'; import { ErrorCode, OpenFeature, ProviderStatus } from '@openfeature/web-sdk'; +import type { EvaluationContext } from '@openfeature/web-sdk'; +import { getPrecomputedContext } from '../configuration'; import { DatadogOfflineOpenFeatureProvider } from '../offlineProvider'; // Stub the native flags TurboModule (TurboModuleRegistry.get returns null under jest), so @@ -27,7 +29,7 @@ jest.mock('../../../core/src/specs/NativeDdFlags', () => ({ } })); -const wireFor = (targetingKey: string): string => +const wireFor = (context?: EvaluationContext): string => JSON.stringify({ version: 1, precomputed: { @@ -49,7 +51,7 @@ const wireFor = (targetingKey: string): string => } } }), - context: { targetingKey } + ...(context === undefined ? {} : { context }) } }); @@ -61,6 +63,20 @@ const freshNames = () => { return { domain: `offline-int-${seq}`, clientName: `offline-int-${seq}` }; }; +const requiredPrecomputedContext = ( + configuration: ReturnType +): EvaluationContext => { + const context = getPrecomputedContext(configuration); + + if (context === undefined) { + throw new Error( + 'Expected a context-specific precomputed configuration.' + ); + } + + return context; +}; + describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + OpenFeature)', () => { afterEach(async () => { await OpenFeature.clearProviders(); @@ -68,49 +84,95 @@ describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + Ope await OpenFeature.clearContext(); }); - it('is READY when a matching configuration is loaded before registration', async () => { + it('uses the helper context to start READY with a precomputed configuration', async () => { const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + const context = getPrecomputedContext(configuration); + + expect(context).toEqual({ targetingKey: 'user-123' }); + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); + const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration(configurationFromString(wireFor('user-123'))); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(domain, provider); const client = OpenFeature.getClient(domain); expect(client.providerStatus).toBe(ProviderStatus.READY); expect(client.getBooleanValue('new-feature', false)).toBe(true); + expect( + jest.requireMock('../../../core/src/specs/NativeDdFlags').default + .setEvaluationContext + ).not.toHaveBeenCalled(); }); - it('enters ERROR and serves defaults on a mismatching setContext, then recovers on a matching one', async () => { + it('starts in ERROR when a context-specific configuration has no OpenFeature context', async () => { const { domain, clientName } = freshNames(); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration(configurationFromString(wireFor('user-123'))); + provider.setConfiguration( + configurationFromString(wireFor({ targetingKey: 'user-123' })) + ); + + await expect( + OpenFeature.setProviderAndWait(domain, provider) + ).rejects.toThrow(); + + const client = OpenFeature.getClient(domain); + expect(client.providerStatus).toBe(ProviderStatus.ERROR); + expect(client.getBooleanDetails('new-feature', false)).toMatchObject({ + value: false, + errorCode: ErrorCode.INVALID_CONTEXT + }); + }); + + it('enters ERROR on a mismatching context and recovers on a matching context', async () => { + const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); + const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(domain, provider); - // A runtime context that does not match the snapshot cannot be served. await OpenFeature.setContext(domain, { targetingKey: 'someone-else' }); const client = OpenFeature.getClient(domain); expect(client.providerStatus).toBe(ProviderStatus.ERROR); - // Serving the coded default, with the precise error code. const details = client.getBooleanDetails('new-feature', false); expect(details.value).toBe(false); expect(details.errorCode).toBe(ErrorCode.INVALID_CONTEXT); - // Setting the matching context again recovers automatically and serves the retained value. await OpenFeature.setContext(domain, { targetingKey: 'user-123' }); expect(client.providerStatus).toBe(ProviderStatus.READY); expect(client.getBooleanValue('new-feature', false)).toBe(true); }); - it('errors on an explicit empty-string targeting key (a real anonymous subject, not "cleared")', async () => { + it.each([ + ['an empty context', {}], + ['an empty targeting key', { targetingKey: '' }] + ])('treats %s as a real context', async (_label, nextContext) => { const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration(configurationFromString(wireFor('user-123'))); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(domain, provider); - // `{ targetingKey: '' }` is an anonymous subject, distinct from the user-123 snapshot — not - // the same as clearing context. It must error rather than silently serve user-123's flags. - await OpenFeature.setContext(domain, { targetingKey: '' }); + await OpenFeature.setContext(domain, nextContext); const client = OpenFeature.getClient(domain); expect(client.providerStatus).toBe(ProviderStatus.ERROR); @@ -119,47 +181,67 @@ describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + Ope ); }); - it('stays READY when the context is cleared (empty = re-adopt embedded)', async () => { - const { domain, clientName } = freshNames(); - const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration(configurationFromString(wireFor('user-123'))); - await OpenFeature.setProviderAndWait(domain, provider); + it.each([ + ['context-agnostic', undefined, undefined], + ['explicitly empty', {}, {}] + ])( + 'starts READY with a %s precomputed context', + async (_label, wireContext, expectedHelperContext) => { + const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor(wireContext as EvaluationContext | undefined) + ); + expect(getPrecomputedContext(configuration)).toEqual( + expectedHelperContext + ); - await OpenFeature.setContext(domain, { targetingKey: 'user-123' }); - expect(OpenFeature.getClient(domain).providerStatus).toBe( - ProviderStatus.READY - ); + const provider = new DatadogOfflineOpenFeatureProvider({ + clientName + }); + provider.setConfiguration(configuration); + await OpenFeature.setProviderAndWait(domain, provider); - // clearContext(domain) falls back to the empty global context; empty means "no override", so - // the embedded context is re-adopted and the provider stays READY. - await OpenFeature.clearContext(domain); - expect(OpenFeature.getClient(domain).providerStatus).toBe( - ProviderStatus.READY - ); - }); + expect(OpenFeature.getClient(domain).providerStatus).toBe( + ProviderStatus.READY + ); + } + ); - it('starts in ERROR when registered before any configuration, then recovers via setConfiguration', async () => { + it('recovers a provider-first setup after the application sets the helper context', async () => { const { domain, clientName } = freshNames(); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - // Provider-first: initialize rejects (no usable configuration), so registration surfaces - // an error and the provider is ERROR rather than a misleading READY. await expect( OpenFeature.setProviderAndWait(domain, provider) ).rejects.toThrow(); const client = OpenFeature.getClient(domain); expect(client.providerStatus).toBe(ProviderStatus.ERROR); - // Loading a valid configuration recovers via the emitted PROVIDER_READY. - provider.setConfiguration(configurationFromString(wireFor('user-123'))); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + provider.setConfiguration(configuration); + expect(client.providerStatus).toBe(ProviderStatus.ERROR); + + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); expect(client.providerStatus).toBe(ProviderStatus.READY); expect(client.getBooleanValue('new-feature', false)).toBe(true); }); it('recovers via setConfiguration when a config matching the current context is loaded', async () => { const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration(configurationFromString(wireFor('user-123'))); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(domain, provider); await OpenFeature.setContext(domain, { targetingKey: 'someone-else' }); @@ -169,26 +251,22 @@ describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + Ope // Load a configuration computed for the now-current context: it reconciles to ready and // setConfiguration emits PROVIDER_READY. provider.setConfiguration( - configurationFromString(wireFor('someone-else')) + configurationFromString(wireFor({ targetingKey: 'someone-else' })) ); expect(client.providerStatus).toBe(ProviderStatus.READY); }); describe('domain / global-context isolation', () => { - it('stays READY with the documented setup: global mismatch set, explicit empty domain context, then register', async () => { + it('inherits a matching global context when the domain has no context', async () => { const { domain, clientName } = freshNames(); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); provider.setConfiguration( - configurationFromString(wireFor('user-123')) + configurationFromString(wireFor({ targetingKey: 'user-123' })) ); - // The documented order: a mismatching GLOBAL context is already in place, the dedicated - // domain is given an explicit empty context (isolating it), and only then is the provider - // registered. It initializes against the empty domain context → embedded → READY. - await OpenFeature.setContext({ targetingKey: 'global-user' }); - await OpenFeature.setContext(domain, {}); + await OpenFeature.setContext({ targetingKey: 'user-123' }); await OpenFeature.setProviderAndWait(domain, provider); expect(OpenFeature.getClient(domain).providerStatus).toBe( @@ -196,45 +274,44 @@ describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + Ope ); }); - it('inherits a mismatching global context when the domain has none, entering ERROR', async () => { + it('enters ERROR when a cleared domain inherits an empty global context', async () => { const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + await OpenFeature.setContext( + domain, + requiredPrecomputedContext(configuration) + ); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration( - configurationFromString(wireFor('user-123')) - ); + provider.setConfiguration(configuration); + await OpenFeature.setProviderAndWait(domain, provider); - // A global (non-domain) context is set; the domain has no context of its own, so it - // inherits the global one at registration → mismatch → ERROR. - await OpenFeature.setContext({ targetingKey: 'global-user' }); - await expect( - OpenFeature.setProviderAndWait(domain, provider) - ).rejects.toThrow(); + await OpenFeature.clearContext(domain); expect(OpenFeature.getClient(domain).providerStatus).toBe( ProviderStatus.ERROR ); }); - it('ignores later global context changes once the domain has an explicit empty context', async () => { + it('stays READY when a cleared domain inherits a matching global context', async () => { const { domain, clientName } = freshNames(); + const configuration = configurationFromString( + wireFor({ targetingKey: 'user-123' }) + ); + await OpenFeature.setContext({ targetingKey: 'user-123' }); + await OpenFeature.setContext(domain, { + targetingKey: 'user-123' + }); const provider = new DatadogOfflineOpenFeatureProvider({ clientName }); - provider.setConfiguration( - configurationFromString(wireFor('user-123')) - ); + provider.setConfiguration(configuration); await OpenFeature.setProviderAndWait(domain, provider); - // Give the domain its own (empty) context: this provider reads it as "no override". - await OpenFeature.setContext(domain, {}); - expect(OpenFeature.getClient(domain).providerStatus).toBe( - ProviderStatus.READY - ); - - // A later mismatching GLOBAL context does not reach a domain that has its own context. - await OpenFeature.setContext({ targetingKey: 'global-mismatch' }); + await OpenFeature.clearContext(domain); expect(OpenFeature.getClient(domain).providerStatus).toBe( ProviderStatus.READY ); @@ -246,22 +323,20 @@ describe('DatadogOfflineOpenFeatureProvider (integration, real FlagsClient + Ope clientName }); provider.setConfiguration( - configurationFromString(wireFor('user-123')) + configurationFromString(wireFor({ targetingKey: 'user-123' })) ); - await OpenFeature.setProviderAndWait(domain, provider); await OpenFeature.setContext(domain, { targetingKey: 'user-123' }); + await OpenFeature.setProviderAndWait(domain, provider); expect(OpenFeature.getClient(domain).providerStatus).toBe( ProviderStatus.READY ); - // A mismatching global context does not reach the domain while it has its own context. await OpenFeature.setContext({ targetingKey: 'global-mismatch' }); expect(OpenFeature.getClient(domain).providerStatus).toBe( ProviderStatus.READY ); - // Clearing the domain context falls back to the (mismatching) global context → ERROR. await OpenFeature.clearContext(domain); expect(OpenFeature.getClient(domain).providerStatus).toBe( ProviderStatus.ERROR diff --git a/packages/react-native-openfeature/src/__tests__/offlineProvider.test.ts b/packages/react-native-openfeature/src/__tests__/offlineProvider.test.ts index 7a60b331e..78c5f096d 100644 --- a/packages/react-native-openfeature/src/__tests__/offlineProvider.test.ts +++ b/packages/react-native-openfeature/src/__tests__/offlineProvider.test.ts @@ -57,18 +57,16 @@ describe('DatadogOfflineOpenFeatureProvider', () => { ); }); - it('re-adopts the embedded context on an empty initialize context', async () => { + it('records an empty initialize context without fetching', async () => { const provider = new DatadogOfflineOpenFeatureProvider(); await provider.initialize({}); - // An empty context means "no external override": reset to the embedded context rather - // than setting an (empty) override. - expect( - mockFlagsClient.resetEvaluationContextWithoutFetching - ).toHaveBeenCalled(); expect( mockFlagsClient.setEvaluationContextWithoutFetching + ).toHaveBeenCalledWith({ attributes: {} }); + expect( + mockFlagsClient.resetEvaluationContextWithoutFetching ).not.toHaveBeenCalled(); expect(mockFlagsClient.setEvaluationContext).not.toHaveBeenCalled(); }); @@ -99,7 +97,7 @@ describe('DatadogOfflineOpenFeatureProvider', () => { it('rejects initialize when no configuration is loaded (provider-first)', async () => { const provider = new DatadogOfflineOpenFeatureProvider(); - mockFlagsClient.resetEvaluationContextWithoutFetching.mockReturnValueOnce( + mockFlagsClient.setEvaluationContextWithoutFetching.mockReturnValueOnce( notReady ); @@ -140,32 +138,40 @@ describe('DatadogOfflineOpenFeatureProvider', () => { ).toThrow(InvalidContextError); }); - it('re-adopts the embedded context on clearContext / empty context change', () => { + it('treats a cleared or empty context as the effective context', () => { const provider = new DatadogOfflineOpenFeatureProvider(); provider.onContextChange({ targetingKey: 'user-1' }, {}); - // Clearing context is not a mismatch: it re-adopts the embedded context and does not throw. - expect( - mockFlagsClient.resetEvaluationContextWithoutFetching - ).toHaveBeenCalled(); expect( mockFlagsClient.setEvaluationContextWithoutFetching + ).toHaveBeenCalledWith({ attributes: {} }); + expect( + mockFlagsClient.resetEvaluationContextWithoutFetching ).not.toHaveBeenCalled(); }); - it('treats a context with only an undefined targetingKey as empty', () => { + it('does not invent a targeting key when it is undefined', () => { const provider = new DatadogOfflineOpenFeatureProvider(); provider.onContextChange({}, { targetingKey: undefined }); - // `{ targetingKey: undefined }` carries no information: reset to the embedded context. + expect( + mockFlagsClient.setEvaluationContextWithoutFetching + ).toHaveBeenCalledWith({ attributes: {} }); expect( mockFlagsClient.resetEvaluationContextWithoutFetching - ).toHaveBeenCalled(); + ).not.toHaveBeenCalled(); + }); + + it('preserves an explicit empty targeting key', () => { + const provider = new DatadogOfflineOpenFeatureProvider(); + + provider.onContextChange({}, { targetingKey: '' }); + expect( mockFlagsClient.setEvaluationContextWithoutFetching - ).not.toHaveBeenCalled(); + ).toHaveBeenCalledWith({ targetingKey: '', attributes: {} }); }); it('delegates setConfiguration to the client and emits CONFIGURATION_CHANGED', () => { @@ -226,7 +232,7 @@ describe('DatadogOfflineOpenFeatureProvider', () => { // A pre-registration setConfiguration error had no listeners, and the empty initialize // context reconciles to the same error, so initialize rejects -> the Web SDK starts the // provider in ERROR rather than a misleading READY. - mockFlagsClient.resetEvaluationContextWithoutFetching.mockReturnValueOnce( + mockFlagsClient.setEvaluationContextWithoutFetching.mockReturnValueOnce( generalError ); await expect(provider.initialize({})).rejects.toThrow(GeneralError); diff --git a/packages/react-native-openfeature/src/configuration.ts b/packages/react-native-openfeature/src/configuration.ts new file mode 100644 index 000000000..5868c647d --- /dev/null +++ b/packages/react-native-openfeature/src/configuration.ts @@ -0,0 +1,64 @@ +/* + * 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 { ParsedFlagsConfiguration } from '@datadog/mobile-react-native'; +import type { + EvaluationContext, + EvaluationContextValue +} from '@openfeature/web-sdk'; + +const cloneContextValue = ( + value: EvaluationContextValue +): EvaluationContextValue => { + if (value instanceof Date) { + return new Date(value.getTime()); + } + + if (Array.isArray(value)) { + return value.map(cloneContextValue); + } + + if (value !== null && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value).map(([key, nestedValue]) => [ + key, + cloneContextValue(nestedValue) + ]) + ); + } + + return value; +}; + +const cloneEvaluationContext = ( + context: EvaluationContext +): EvaluationContext => { + return Object.fromEntries( + Object.entries(context).map(([key, value]) => [ + key, + cloneContextValue(value) + ]) + ); +}; + +/** + * Return the evaluation context from a precomputed configuration. + * + * The returned context is a detached copy. Setting it as the OpenFeature context is an explicit + * application operation; this function does not modify OpenFeature or provider state. It returns + * `undefined` when the configuration has no context-specific precomputed branch. + */ +export const getPrecomputedContext = ( + configuration: ParsedFlagsConfiguration +): EvaluationContext | undefined => { + const context = configuration.precomputed?.context; + + if (context === undefined) { + return undefined; + } + + return cloneEvaluationContext(context); +}; diff --git a/packages/react-native-openfeature/src/index.ts b/packages/react-native-openfeature/src/index.ts index 55c571c9e..c9614768e 100644 --- a/packages/react-native-openfeature/src/index.ts +++ b/packages/react-native-openfeature/src/index.ts @@ -6,6 +6,7 @@ import { configurationFromString } from '@datadog/mobile-react-native'; +import { getPrecomputedContext } from './configuration'; import { DatadogOfflineOpenFeatureProvider } from './offlineProvider'; import { DatadogOpenFeatureProvider } from './provider'; import type { DatadogOpenFeatureProviderOptions } from './provider'; @@ -13,6 +14,7 @@ import type { DatadogOpenFeatureProviderOptions } from './provider'; export { DatadogOpenFeatureProvider, DatadogOfflineOpenFeatureProvider, - configurationFromString + configurationFromString, + getPrecomputedContext }; export type { DatadogOpenFeatureProviderOptions }; diff --git a/packages/react-native-openfeature/src/mappers.ts b/packages/react-native-openfeature/src/mappers.ts index ecc7d03b8..80651ed9d 100644 --- a/packages/react-native-openfeature/src/mappers.ts +++ b/packages/react-native-openfeature/src/mappers.ts @@ -31,17 +31,20 @@ export const toDdContext = ( }; /** - * Whether an OpenFeature evaluation context carries no information — no targeting key and no - * attributes with a defined value (so `{}` and `{ targetingKey: undefined }` are both empty). - * Used by the offline provider to avoid overwriting a configuration's embedded context with an - * empty context stamped by the OpenFeature lifecycle. - * - * Note: an explicit `targetingKey: ''` is **not** empty. An empty string is a real (anonymous) - * targeting key — a distinct subject — not the absence of a context. Only a genuinely absent - * context (`{}` / `clearContext()`) re-adopts the configuration's embedded context; `{ targetingKey: - * '' }` is reconciled as a real context, so it must match the precomputed snapshot or the provider - * enters `ERROR` (serving coded defaults) rather than silently serving another subject's flags. + * Convert an OpenFeature context for offline evaluation without inventing a targeting key. + * Rules distinguish a missing targeting key from an empty targeting key. */ -export const isEmptyContext = (context: OFEvaluationContext): boolean => { - return Object.values(context).every(value => value === undefined); +export const toDdContextPreservingTargetingKey = ( + context: OFEvaluationContext +): DdEvaluationContext => { + const { targetingKey, ...attributes } = context; + const ddContext = { + attributes: attributes as Record + } as DdEvaluationContext; + + if (targetingKey !== undefined) { + ddContext.targetingKey = targetingKey; + } + + return ddContext; }; diff --git a/packages/react-native-openfeature/src/offlineProvider.ts b/packages/react-native-openfeature/src/offlineProvider.ts index aa4f868a7..f0aec033b 100644 --- a/packages/react-native-openfeature/src/offlineProvider.ts +++ b/packages/react-native-openfeature/src/offlineProvider.ts @@ -22,7 +22,7 @@ import type { } from '@openfeature/web-sdk'; import { DatadogCoreOpenFeatureProvider } from './coreProvider'; -import { isEmptyContext, toDdContext } from './mappers'; +import { toDdContextPreservingTargetingKey } from './mappers'; // The outcome of a `FlagsClient` reconcile. Derived from the client so the provider maps it to // OpenFeature transitions; not part of the package's public API. @@ -49,28 +49,33 @@ const OF_ERROR_CODE: Record = { * It behaves like the online `DatadogOpenFeatureProvider` — same flag evaluation and * exposure/RUM tracking — **except it never fetches configuration from the network**. * Instead of fetching on `initialize`/`onContextChange`, it evaluates against a configuration - * supplied via {@link DatadogOfflineOpenFeatureProvider.setConfiguration}. A precomputed - * configuration carries the evaluation context it was computed for, so you should **not** call - * `OpenFeature.setContext` for the offline precomputed flow — see the class remarks. + * supplied via {@link DatadogOfflineOpenFeatureProvider.setConfiguration}. * * A runtime context that does not match the configuration's embedded context (compared after * normalization) cannot be served (offline never fetches), so it puts the provider into the * OpenFeature `ERROR` state and evaluations fall back to your coded defaults (`INVALID_CONTEXT`). - * An empty *effective* context re-adopts the embedded context and recovers — but note that - * `clearContext(domain)` falls back to the global context, which may itself be non-empty and - * mismatching (and would keep the provider in `ERROR`). Load the configuration before setting the - * provider so it is ready with real flag values from the start: + * An empty context is a real context. It does not select the embedded context. Use + * `getPrecomputedContext` to get a supported copy of the embedded context, and set it on + * OpenFeature before provider registration. Load the configuration before setting the provider so + * it is ready with real flag values from the start: * * @example * ```ts * import { OpenFeature } from '@openfeature/web-sdk'; * import { * DatadogOfflineOpenFeatureProvider, - * configurationFromString + * configurationFromString, + * getPrecomputedContext * } from '@datadog/mobile-react-native-openfeature'; * + * const configuration = configurationFromString(wire); + * const context = getPrecomputedContext(configuration); + * if (context !== undefined) { + * await OpenFeature.setContext(context); + * } + * * const provider = new DatadogOfflineOpenFeatureProvider(); - * provider.setConfiguration(configurationFromString(wire)); // no network + * provider.setConfiguration(configuration); // no network * await OpenFeature.setProviderAndWait(provider); * * const client = OpenFeature.getClient(); @@ -145,15 +150,11 @@ export class DatadogOfflineOpenFeatureProvider extends DatadogCoreOpenFeaturePro } private applyContext(context: OFEvaluationContext): ConfigurationResult { - // An empty context means "no external override": clear it so a loaded precomputed - // configuration is served against its embedded context. Order-independent — the synthetic - // `initialize({})`, `setContext({})`, and `clearContext()` all re-adopt the embedded - // context rather than being treated as a mismatch. - const result = isEmptyContext(context) - ? this.flagsClient.resetEvaluationContextWithoutFetching() - : this.flagsClient.setEvaluationContextWithoutFetching( - toDdContext(context) - ); + // OpenFeature gives the provider only the effective context. It uses `{}` for an unset or + // cleared global context, so the provider must treat `{}` as the real effective context. + const result = this.flagsClient.setEvaluationContextWithoutFetching( + toDdContextPreservingTargetingKey(context) + ); this.configurationInError = result.status === 'error'; From e392ed4c6936398a8ff511cc2e97b9646860d4a6 Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 6 Aug 2026 20:26:37 -0400 Subject: [PATCH 12/20] chore(openfeature): track upstream context helper --- packages/core/src/flags/configuration/index.ts | 5 +++++ packages/react-native-openfeature/src/configuration.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/packages/core/src/flags/configuration/index.ts b/packages/core/src/flags/configuration/index.ts index 8078428e2..f0fb1fa46 100644 --- a/packages/core/src/flags/configuration/index.ts +++ b/packages/core/src/flags/configuration/index.ts @@ -10,6 +10,11 @@ // the decoder and other helpers stay internal to this boundary. Keeping the surface contained // here makes a future "port -> depend on a shared core" swap easier. +// TODO(FFL-2837): Re-export `getPrecomputedContext` from +// `@datadog/flagging-core/configuration` here after a flagging-core release contains +// DataDog/openfeature-js-client#353 through `499c31b`. Also expose it from the +// public React Native SDK entry point for the OpenFeature package to consume. + export { configurationFromString, configurationToString } from './wire'; export { decodePrecomputedFlags, diff --git a/packages/react-native-openfeature/src/configuration.ts b/packages/react-native-openfeature/src/configuration.ts index 5868c647d..0d59ed6f9 100644 --- a/packages/react-native-openfeature/src/configuration.ts +++ b/packages/react-native-openfeature/src/configuration.ts @@ -10,6 +10,11 @@ import type { EvaluationContextValue } from '@openfeature/web-sdk'; +// TODO(FFL-2837): Delete this local helper and its clone-semantics tests after a +// flagging-core release contains DataDog/openfeature-js-client#353 through +// `499c31b` and `@datadog/mobile-react-native` re-exports the upstream helper. +// Import and re-export `getPrecomputedContext` from the React Native SDK instead. +// Keep the React Native forwarding, provider, and bootstrap integration tests. const cloneContextValue = ( value: EvaluationContextValue ): EvaluationContextValue => { From 753c26a9e378bb42fc6bb99a09408227553d7bca Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 6 Aug 2026 20:50:08 -0400 Subject: [PATCH 13/20] docs(openfeature): clarify helper migration --- packages/react-native-openfeature/src/configuration.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-native-openfeature/src/configuration.ts b/packages/react-native-openfeature/src/configuration.ts index 0d59ed6f9..19f9a9c70 100644 --- a/packages/react-native-openfeature/src/configuration.ts +++ b/packages/react-native-openfeature/src/configuration.ts @@ -14,7 +14,9 @@ import type { // flagging-core release contains DataDog/openfeature-js-client#353 through // `499c31b` and `@datadog/mobile-react-native` re-exports the upstream helper. // Import and re-export `getPrecomputedContext` from the React Native SDK instead. -// Keep the React Native forwarding, provider, and bootstrap integration tests. +// Raise the React Native SDK peer and development dependency minimums to the first +// release that exports it. Replace these semantic tests with one package-root +// forwarding test. Keep the provider and bootstrap integration tests. const cloneContextValue = ( value: EvaluationContextValue ): EvaluationContextValue => { From 4c652876fe4f167b3cd608cd06e3a99ce960bc6e Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 3 Aug 2026 16:23:58 -0400 Subject: [PATCH 14/20] docs(flags): preserve core parser boundary --- packages/core/src/flags/configuration/wire.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 4a4d4e4df..85beccdc5 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -28,6 +28,8 @@ import type { // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. The new `@datadog/flagging-core/precomputed` // subpath is protobuf-free, ignores rules, and is not the parser for this module. +// PR #336 through `33113d2` adds browser providers but does not change this core +// parser boundary. Do not import `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 // encoding of the raw dd-source#34959 protobuf response in the version 1 // `rules.response` field. Record dd-source#40304 commit `071c4ad` as the schema From fa169035b81ae3a33eae282002d6b93af8dbb441 Mon Sep 17 00:00:00 2001 From: Blake Date: Fri, 7 Aug 2026 07:08:24 -0400 Subject: [PATCH 15/20] fix(flags): preserve configuration parse errors --- .../configuration/__tests__/rules.test.ts | 6 +- .../configuration/__tests__/wire.test.ts | 38 +++++++-- .../core/src/flags/configuration/rules.ts | 27 ++++--- packages/core/src/flags/configuration/wire.ts | 77 ++++++++++++++----- 4 files changed, 105 insertions(+), 43 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 2d8b6a431..5ab02515c 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -200,7 +200,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `9f794c7`. Round-trip the + // DataDog/openfeature-js-client#344 through `82bfc2e`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. // Add a fixture with an unsupported minimum feature level and require a // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. @@ -230,8 +230,8 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `9f794c7` plus the no-`BigInt` follow-up. The generated parser must preserve - // the source value as `bigint` where supported. Run the same evaluation with + // `82bfc2e`. The generated parser must preserve the source value as `bigint` + // where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { const source = buildRulesConfiguration(); diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index 0e1cdc0b7..d2e8b3dbb 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -79,17 +79,21 @@ describe('configurationFromString', () => { ).toBe(true); }); - it('returns an empty config for an unsupported version', () => { + it('preserves a configuration error for an unsupported version', () => { const wire = JSON.stringify({ version: 2, precomputed: { response: JSON.stringify(buildResponse()) } }); - expect(configurationFromString(wire)).toEqual({}); + expect(configurationFromString(wire)).toEqual({ + configurationError: 'Invalid flags configuration wire format' + }); }); - it('returns an empty config for invalid JSON', () => { - expect(configurationFromString('not json')).toEqual({}); + it('preserves a configuration error for invalid JSON', () => { + expect(configurationFromString('not json')).toEqual({ + configurationError: 'Invalid flags configuration wire format' + }); }); it('does not treat a raw protobuf response as a portable wire', () => { @@ -97,13 +101,17 @@ describe('configurationFromString', () => { // these bytes in a version 1 `rules.response` JSON envelope. const rawProtobufAsBase64 = 'CgR0ZXN0'; - expect(configurationFromString(rawProtobufAsBase64)).toEqual({}); + expect(configurationFromString(rawProtobufAsBase64)).toEqual({ + configurationError: 'Invalid flags configuration wire format' + }); }); it('does not treat the legacy UFC JSON response as a portable wire', () => { const legacyServiceResponse = JSON.stringify(buildRulesConfiguration()); - expect(configurationFromString(legacyServiceResponse)).toEqual({}); + expect(configurationFromString(legacyServiceResponse)).toEqual({ + configurationError: 'Invalid flags configuration wire format' + }); }); it('returns an empty config when the inner response is invalid JSON', () => { @@ -214,9 +222,27 @@ describe('temporary rules configuration wire compatibility', () => { ) as { precomputed?: unknown; rulesBased?: unknown; + rulesError?: string; }; expect(parsed.precomputed).toBeDefined(); expect(parsed.rulesBased).toBeUndefined(); + expect(parsed.rulesError).toBe( + 'Rules configuration response could not be decoded' + ); + }); + + it('preserves an invalid rules entry error', () => { + const parsed = configurationFromString( + buildWire({ rulesBased: { response: 42 } }) + ) as { + precomputed?: unknown; + rulesError?: string; + }; + + expect(parsed.precomputed).toBeDefined(); + expect(parsed.rulesError).toBe( + 'Invalid rules configuration wire entry' + ); }); }); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 6949a9e96..98f1fcab9 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,12 +14,11 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `9f794c7`, restores -// 32-byte SHA digest validation, and defines or fixes integer evaluation without -// global `BigInt`. Keep the `FlagsConfiguration` type import on the flagging-core -// package root. PR #344 preserves protobuf integers as `bigint`, and its evaluator -// reports unsafe conversions as deterministic per-flag `PARSE_ERROR` results when -// `BigInt` is available. +// release contains DataDog/openfeature-js-client#344 through `82bfc2e` and +// restores 32-byte SHA digest validation. Keep the `FlagsConfiguration` type +// import on the flagging-core package root. PR #344 preserves protobuf integers +// as `bigint`, does not call global `BigInt` during safe conversion, and reports +// unsafe conversions as deterministic per-flag `PARSE_ERROR` results. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -130,8 +129,8 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `9f794c7` and fixes -// or explicitly excludes integer and shard evaluation without global `BigInt`. +// release contains DataDog/openfeature-js-client#344 through `82bfc2e` and the +// required SHA digest validation follow-up. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. // It returns deterministic `PARSE_ERROR` results, including for an integer that @@ -210,7 +209,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `9f794c7` compiles protobuf regular expressions lazily and caches + // `82bfc2e` compiles protobuf regular expressions lazily and caches // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { @@ -465,8 +464,8 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `9f794c7` and the - // no-`BigInt` integer contract is fixed or declared unsupported. That + // flagging-core release contains upstream PR #344 through `82bfc2e` and the + // required SHA digest validation follow-up. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. With `BigInt`, it returns a // deterministic per-flag error when evaluation cannot produce a safe number. @@ -521,7 +520,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `9f794c7`. The protobuf evaluator maps only safely represented integer +// `82bfc2e`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -548,7 +547,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `9f794c7`. Keep the reserved-name contract tests. + // through `82bfc2e`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -559,7 +558,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `9f794c7` validates + // store after the published PR #344 evaluator through `82bfc2e` validates // reached flag data and reports deterministic flag-scoped errors, including // unsupported feature levels and unsafe integer conversions with and // without global `BigInt` when supported. diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 85beccdc5..9acfd5190 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -8,8 +8,9 @@ // that conversion to the opt-in `@datadog/flagging-core/configuration` entry point so the default // entry point does not load Protobuf-ES. In both versions, the input is the complete portable JSON // envelope. It is not the raw protobuf or legacy JSON response from the UFC service. -// `configurationFromString` is lenient: it returns an empty configuration (`{}`) for -// malformed input or an unsupported wire version rather than throwing. +// Published `configurationFromString` is lenient: it returns an empty configuration +// (`{}`) for malformed input or an unsupported wire version rather than throwing. +// PR #344 preserves that failure as `configurationError` instead. // `configurationToString` is the inverse (its fix from // https://github.com/DataDog/openfeature-js-client/pull/331 shipped in flagging-core 2.0.0). import { @@ -23,13 +24,14 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `9f794c7` plus the required SHA digest and no-`BigInt` follow-ups. +// through `82bfc2e` plus the required SHA digest follow-up. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. The new `@datadog/flagging-core/precomputed` // subpath is protobuf-free, ignores rules, and is not the parser for this module. -// PR #336 through `33113d2` adds browser providers but does not change this core -// parser boundary. Do not import `@datadog/openfeature-browser` in React Native. +// PR #336 through `4d0f24e` adds browser providers and shared lifecycle error +// selection but does not change this core parser boundary. Do not import +// `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 // encoding of the raw dd-source#34959 protobuf response in the version 1 // `rules.response` field. Record dd-source#40304 commit `071c4ad` as the schema @@ -39,6 +41,8 @@ import type { // unsupported feature levels, and unsafe integer conversion as deterministic // flag-scoped `PARSE_ERROR` results. type PendingRulesConfiguration = FlagsConfiguration & { + configurationError?: string; + rulesError?: string; rulesBased?: { response: UniversalFlagConfigurationV1; fetchedAt?: number; @@ -46,6 +50,12 @@ type PendingRulesConfiguration = FlagsConfiguration & { }; }; +const INVALID_CONFIGURATION_WIRE_ERROR = + 'Invalid flags configuration wire format'; +const INVALID_RULES_WIRE_ENTRY_ERROR = 'Invalid rules configuration wire entry'; +const INVALID_RULES_RESPONSE_ERROR = + 'Rules configuration response could not be decoded'; + type PendingRulesWire = { version: 1; rulesBased?: { @@ -55,22 +65,41 @@ type PendingRulesWire = { }; }; -const readPendingRulesWire = ( - source: string -): PendingRulesWire['rulesBased'] | undefined => { +type PendingRulesWireResult = + | { status: 'invalid-configuration' } + | { status: 'no-rules' } + | { status: 'invalid-rules' } + | { + status: 'rules'; + rules: NonNullable; + }; + +const readPendingRulesWire = (source: string): PendingRulesWireResult => { try { - const wire = JSON.parse(source) as PendingRulesWire; + const wire = JSON.parse(source) as Partial | null; + if ( + !wire || + typeof wire !== 'object' || + Array.isArray(wire) || + wire.version !== 1 + ) { + return { status: 'invalid-configuration' }; + } + if (wire.rulesBased === undefined) { + return { status: 'no-rules' }; + } if ( - wire.version !== 1 || !wire.rulesBased || + typeof wire.rulesBased !== 'object' || + Array.isArray(wire.rulesBased) || typeof wire.rulesBased.response !== 'string' ) { - return undefined; + return { status: 'invalid-rules' }; } - return wire.rulesBased; + return { status: 'rules', rules: wire.rulesBased }; } catch { - return undefined; + return { status: 'invalid-configuration' }; } }; @@ -89,16 +118,24 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `9f794c7`, plus the final no-`BigInt` runtime decision. + // `82bfc2e`. Its safe-integer conversion does not require global `BigInt`. + // TODO(FFL-2837): Delete this parse-error compatibility behavior when the + // dependency contains PR #344 through `82bfc2e`. The upstream parser uses + // `configurationError` for an invalid envelope and `rulesError` for an + // invalid rules entry or response. It keeps a valid sibling branch. const pendingRules = readPendingRulesWire(source); - if (pendingRules) { + if (pendingRules.status === 'invalid-configuration') { + configuration.configurationError = INVALID_CONFIGURATION_WIRE_ERROR; + } else if (pendingRules.status === 'invalid-rules') { + configuration.rulesError = INVALID_RULES_WIRE_ENTRY_ERROR; + } else if (pendingRules.status === 'rules') { try { configuration.rulesBased = { - ...pendingRules, - response: JSON.parse(pendingRules.response) + ...pendingRules.rules, + response: JSON.parse(pendingRules.rules.response) }; } catch { - return configuration; + configuration.rulesError = INVALID_RULES_RESPONSE_ERROR; } } @@ -114,8 +151,8 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `9f794c7` and - // its required follow-ups. The upstream serializer encodes generated protobuf + // types above after the dependency contains PR #344 through `82bfc2e` and + // its required SHA digest follow-up. The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. if (pendingConfiguration.rulesBased) { From b6ca8ea7856c9f5c67f49cba932b870d41c44b45 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 10 Aug 2026 14:44:29 -0400 Subject: [PATCH 16/20] docs(flags): refresh upstream TODO anchors --- .../flags/configuration/__tests__/rules.test.ts | 4 ++-- packages/core/src/flags/configuration/rules.ts | 14 +++++++------- packages/core/src/flags/configuration/wire.ts | 12 +++++++----- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 5ab02515c..8926d74d8 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -200,7 +200,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `82bfc2e`. Round-trip the + // DataDog/openfeature-js-client#344 through `939da97`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. // Add a fixture with an unsupported minimum feature level and require a // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. @@ -230,7 +230,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `82bfc2e`. The generated parser must preserve the source value as `bigint` + // `939da97`. The generated parser must preserve the source value as `bigint` // where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 98f1fcab9..f9534d12f 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,7 +14,7 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `82bfc2e` and +// release contains DataDog/openfeature-js-client#344 through `939da97` and // restores 32-byte SHA digest validation. Keep the `FlagsConfiguration` type // import on the flagging-core package root. PR #344 preserves protobuf integers // as `bigint`, does not call global `BigInt` during safe conversion, and reports @@ -129,7 +129,7 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `82bfc2e` and the +// release contains DataDog/openfeature-js-client#344 through `939da97` and the // required SHA digest validation follow-up. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. @@ -209,7 +209,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `82bfc2e` compiles protobuf regular expressions lazily and caches + // `939da97` compiles protobuf regular expressions lazily and caches // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { @@ -464,7 +464,7 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `82bfc2e` and the + // flagging-core release contains upstream PR #344 through `939da97` and the // required SHA digest validation follow-up. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. With `BigInt`, it returns a @@ -520,7 +520,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `82bfc2e`. The protobuf evaluator maps only safely represented integer +// `939da97`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -547,7 +547,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `82bfc2e`. Keep the reserved-name contract tests. + // through `939da97`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -558,7 +558,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `82bfc2e` validates + // store after the published PR #344 evaluator through `939da97` validates // reached flag data and reports deterministic flag-scoped errors, including // unsupported feature levels and unsafe integer conversions with and // without global `BigInt` when supported. diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 9acfd5190..edb0e9357 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -24,12 +24,14 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `82bfc2e` plus the required SHA digest follow-up. +// through `939da97` plus the required SHA digest follow-up. The post-rebase +// `ab22ad0` and `939da97` commits update generated Node-server artifacts and +// browser test isolation; they do not change this React Native boundary. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. The new `@datadog/flagging-core/precomputed` // subpath is protobuf-free, ignores rules, and is not the parser for this module. -// PR #336 through `4d0f24e` adds browser providers and shared lifecycle error +// PR #336 through `6d3d6a4` adds browser providers and shared lifecycle error // selection but does not change this core parser boundary. Do not import // `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 @@ -118,9 +120,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `82bfc2e`. Its safe-integer conversion does not require global `BigInt`. + // `939da97`. Its safe-integer conversion does not require global `BigInt`. // TODO(FFL-2837): Delete this parse-error compatibility behavior when the - // dependency contains PR #344 through `82bfc2e`. The upstream parser uses + // dependency contains PR #344 through `939da97`. The upstream parser uses // `configurationError` for an invalid envelope and `rulesError` for an // invalid rules entry or response. It keeps a valid sibling branch. const pendingRules = readPendingRulesWire(source); @@ -151,7 +153,7 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `82bfc2e` and + // types above after the dependency contains PR #344 through `939da97` and // its required SHA digest follow-up. The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. From a8a8d6740bffdfde6316b40481f20ac8a62dd9c0 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 10 Aug 2026 14:45:55 -0400 Subject: [PATCH 17/20] docs(flags): correct BigInt TODO --- packages/core/src/flags/configuration/rules.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index f9534d12f..b87121046 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -467,8 +467,8 @@ export const prepareRulesConfiguration = ( // flagging-core release contains upstream PR #344 through `939da97` and the // required SHA digest validation follow-up. That // implementation preserves protobuf integers as `bigint` and validates only - // the requested flag data that evaluation reaches. With `BigInt`, it returns a - // deterministic per-flag error when evaluation cannot produce a safe number. + // the requested flag data that evaluation reaches. It does not call global + // `BigInt` when it returns a deterministic per-flag error for an unsafe number. // Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { From a4f6bdd368a508857fc55dbda252771a2fa68df6 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 11 Aug 2026 16:03:29 -0400 Subject: [PATCH 18/20] docs(flags): refresh rebased upstream TODOs --- .../configuration/__tests__/rules.test.ts | 4 ++-- packages/core/src/flags/configuration/index.ts | 2 +- packages/core/src/flags/configuration/rules.ts | 14 +++++++------- packages/core/src/flags/configuration/wire.ts | 18 ++++++++++-------- .../src/configuration.ts | 2 +- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 8926d74d8..817893840 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -200,7 +200,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `939da97`. Round-trip the + // DataDog/openfeature-js-client#344 through `03cde21`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. // Add a fixture with an unsupported minimum feature level and require a // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. @@ -230,7 +230,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `939da97`. The generated parser must preserve the source value as `bigint` + // `03cde21`. The generated parser must preserve the source value as `bigint` // where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { diff --git a/packages/core/src/flags/configuration/index.ts b/packages/core/src/flags/configuration/index.ts index f0fb1fa46..77737bb6e 100644 --- a/packages/core/src/flags/configuration/index.ts +++ b/packages/core/src/flags/configuration/index.ts @@ -12,7 +12,7 @@ // TODO(FFL-2837): Re-export `getPrecomputedContext` from // `@datadog/flagging-core/configuration` here after a flagging-core release contains -// DataDog/openfeature-js-client#353 through `499c31b`. Also expose it from the +// DataDog/openfeature-js-client#353 through `caae6ab`. Also expose it from the // public React Native SDK entry point for the OpenFeature package to consume. export { configurationFromString, configurationToString } from './wire'; diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index b87121046..e24bb48f9 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,7 +14,7 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `939da97` and +// release contains DataDog/openfeature-js-client#344 through `03cde21` and // restores 32-byte SHA digest validation. Keep the `FlagsConfiguration` type // import on the flagging-core package root. PR #344 preserves protobuf integers // as `bigint`, does not call global `BigInt` during safe conversion, and reports @@ -129,7 +129,7 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `939da97` and the +// release contains DataDog/openfeature-js-client#344 through `03cde21` and the // required SHA digest validation follow-up. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. @@ -209,7 +209,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `939da97` compiles protobuf regular expressions lazily and caches + // `03cde21` compiles protobuf regular expressions lazily and caches // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { @@ -464,7 +464,7 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `939da97` and the + // flagging-core release contains upstream PR #344 through `03cde21` and the // required SHA digest validation follow-up. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. It does not call global @@ -520,7 +520,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `939da97`. The protobuf evaluator maps only safely represented integer +// `03cde21`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -547,7 +547,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `939da97`. Keep the reserved-name contract tests. + // through `03cde21`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -558,7 +558,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `939da97` validates + // store after the published PR #344 evaluator through `03cde21` validates // reached flag data and reports deterministic flag-scoped errors, including // unsupported feature levels and unsafe integer conversions with and // without global `BigInt` when supported. diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index edb0e9357..d63f8ec71 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -24,15 +24,17 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `939da97` plus the required SHA digest follow-up. The post-rebase -// `ab22ad0` and `939da97` commits update generated Node-server artifacts and -// browser test isolation; they do not change this React Native boundary. +// through `03cde21` plus the required SHA digest follow-up. The `03cde21` tree +// is identical to the previous `939da97` tree. Its final `1db13d4` and +// `03cde21` commits update generated Node-server artifacts and browser test +// isolation; they do not change this React Native boundary. // Import and re-export the wire functions and `FlagsConfigurationWire` type from // `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules // evaluator on the package root. The new `@datadog/flagging-core/precomputed` // subpath is protobuf-free, ignores rules, and is not the parser for this module. -// PR #336 through `6d3d6a4` adds browser providers and shared lifecycle error -// selection but does not change this core parser boundary. Do not import +// PR #336 through `772167b` adds browser providers and shared lifecycle error +// selection. Its tree is identical to the previous `6d3d6a4` tree, so it does +// not change this core parser boundary. Do not import // `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 // encoding of the raw dd-source#34959 protobuf response in the version 1 @@ -120,9 +122,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `939da97`. Its safe-integer conversion does not require global `BigInt`. + // `03cde21`. Its safe-integer conversion does not require global `BigInt`. // TODO(FFL-2837): Delete this parse-error compatibility behavior when the - // dependency contains PR #344 through `939da97`. The upstream parser uses + // dependency contains PR #344 through `03cde21`. The upstream parser uses // `configurationError` for an invalid envelope and `rulesError` for an // invalid rules entry or response. It keeps a valid sibling branch. const pendingRules = readPendingRulesWire(source); @@ -153,7 +155,7 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `939da97` and + // types above after the dependency contains PR #344 through `03cde21` and // its required SHA digest follow-up. The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. diff --git a/packages/react-native-openfeature/src/configuration.ts b/packages/react-native-openfeature/src/configuration.ts index 19f9a9c70..ccf573c0f 100644 --- a/packages/react-native-openfeature/src/configuration.ts +++ b/packages/react-native-openfeature/src/configuration.ts @@ -12,7 +12,7 @@ import type { // TODO(FFL-2837): Delete this local helper and its clone-semantics tests after a // flagging-core release contains DataDog/openfeature-js-client#353 through -// `499c31b` and `@datadog/mobile-react-native` re-exports the upstream helper. +// `caae6ab` and `@datadog/mobile-react-native` re-exports the upstream helper. // Import and re-export `getPrecomputedContext` from the React Native SDK instead. // Raise the React Native SDK peer and development dependency minimums to the first // release that exports it. Replace these semantic tests with one package-root From 0c4ed194250759c329dbc46cce3bf83a9f4817e7 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 17 Aug 2026 17:45:15 -0400 Subject: [PATCH 19/20] fix(flags): guard inherited rules context attributes --- .../configuration/__tests__/rules.test.ts | 88 ++++++++++++++++++- .../core/src/flags/configuration/rules.ts | 39 ++++---- packages/core/src/flags/configuration/wire.ts | 28 +++--- 3 files changed, 122 insertions(+), 33 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 817893840..ad50b6455 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -28,7 +28,7 @@ describe('rules configuration', () => { enabled: true } }) - ).toEqual({ + ).toMatchObject({ targetingKey: 'user-1', country: 'US', enabled: true @@ -46,6 +46,81 @@ describe('rules configuration', () => { ); }); + it.each(['constructor', 'toString'])( + 'shadows an absent inherited %s context attribute', + attribute => { + const rulesContext = toRulesEvaluationContext({ + targetingKey: 'user-1' + }); + + expect(Object.getPrototypeOf(rulesContext)).toBeNull(); + expect( + Object.prototype.hasOwnProperty.call(rulesContext, attribute) + ).toBe(true); + expect(rulesContext[attribute]).toBeUndefined(); + + const configuration = buildRulesConfiguration(); + const condition = + configuration.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + if (!condition) { + throw new Error('The fixture has no condition.'); + } + condition.attribute = attribute; + condition.value = [ + String(({} as Record)[attribute]) + ]; + + expect( + flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: rulesContext, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: false, reason: 'DEFAULT' }); + } + ); + + it.each(['constructor', 'toString'])( + 'preserves an explicit own %s context attribute', + attribute => { + const attributes = Object.create(null) as Record; + attributes[attribute] = 'customer-value'; + + const rulesContext = toRulesEvaluationContext({ + targetingKey: 'user-1', + attributes + }); + expect( + Object.prototype.hasOwnProperty.call(rulesContext, attribute) + ).toBe(true); + expect(rulesContext[attribute]).toBe('customer-value'); + const configuration = buildRulesConfiguration(); + const condition = + configuration.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + if (!condition) { + throw new Error('The fixture has no condition.'); + } + condition.attribute = attribute; + condition.value = ['customer-value']; + + expect( + flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: rulesContext, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: true, reason: 'TARGETING_MATCH' }); + } + ); + it('clones and freezes a valid rules configuration', () => { const source = buildRulesConfiguration(); const prepared = prepareRulesConfiguration(source); @@ -200,10 +275,15 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `03cde21`. Round-trip the + // DataDog/openfeature-js-client#344 through `5a5511e`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. // Add a fixture with an unsupported minimum feature level and require a - // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. + // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. Also cover unsorted + // string and SHA-256 membership indexes, invalid SHA digest lengths, and + // semantic-version components at and above the unsigned 64-bit limit. + // Confirm that an absent inherited `__proto__` attribute does not match and + // that an explicit own `__proto__` context attribute remains usable. The + // legacy evaluator's compiled object spread cannot preserve that key. it('keeps supported known data when an unknown field is present', () => { const source = buildRulesConfiguration(); (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & { @@ -230,7 +310,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `03cde21`. The generated parser must preserve the source value as `bigint` + // `5a5511e`. The generated parser must preserve the source value as `bigint` // where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index e24bb48f9..6bd523dfb 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,11 +14,12 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `03cde21` and -// restores 32-byte SHA digest validation. Keep the `FlagsConfiguration` type +// release contains DataDog/openfeature-js-client#344 through `5a5511e`. +// Keep the `FlagsConfiguration` type // import on the flagging-core package root. PR #344 preserves protobuf integers // as `bigint`, does not call global `BigInt` during safe conversion, and reports -// unsafe conversions as deterministic per-flag `PARSE_ERROR` results. +// unsafe conversions and malformed SHA digests as deterministic per-flag +// `PARSE_ERROR` results. type RulesConfigurationResponse = UniversalFlagConfigurationV1; export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; @@ -119,18 +120,27 @@ export const toRulesEvaluationContext = ( attributes.set(key, value); } - return { - ...Object.fromEntries(attributes), - targetingKey: context.targetingKey - }; + // TODO(FFL-2837): Delete the inherited-name shadows after a flagging-core + // release contains DataDog/openfeature-js-client#344 through `aa93230` and + // this adapter uses its generated protobuf evaluator. That evaluator uses + // own-property lookup for condition and shard context attributes. + const rulesContext = Object.create(null) as RulesEvaluationContext; + for (const key of Object.getOwnPropertyNames(Object.prototype)) { + rulesContext[key] = undefined; + } + for (const [key, value] of attributes) { + rulesContext[key] = value; + } + rulesContext.targetingKey = context.targetingKey; + + return rulesContext; }; const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `03cde21` and the -// required SHA digest validation follow-up. +// release contains DataDog/openfeature-js-client#344 through `5a5511e`. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. // It returns deterministic `PARSE_ERROR` results, including for an integer that @@ -209,7 +219,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `03cde21` compiles protobuf regular expressions lazily and caches + // `5a5511e` compiles protobuf regular expressions lazily and caches // them by configuration and index, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { @@ -464,8 +474,7 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `03cde21` and the - // required SHA digest validation follow-up. That + // flagging-core release contains upstream PR #344 through `5a5511e`. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. It does not call global // `BigInt` when it returns a deterministic per-flag error for an unsafe number. @@ -520,7 +529,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `03cde21`. The protobuf evaluator maps only safely represented integer +// `5a5511e`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -547,7 +556,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `03cde21`. Keep the reserved-name contract tests. + // through `5a5511e`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -558,7 +567,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `03cde21` validates + // store after the published PR #344 evaluator through `5a5511e` validates // reached flag data and reports deterministic flag-scoped errors, including // unsupported feature levels and unsafe integer conversions with and // without global `BigInt` when supported. diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index d63f8ec71..e2fdfdde4 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -24,17 +24,16 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `03cde21` plus the required SHA digest follow-up. The `03cde21` tree -// is identical to the previous `939da97` tree. Its final `1db13d4` and -// `03cde21` commits update generated Node-server artifacts and browser test -// isolation; they do not change this React Native boundary. +// through `5a5511e`. // Import and re-export the wire functions and `FlagsConfigurationWire` type from -// `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules -// evaluator on the package root. The new `@datadog/flagging-core/precomputed` +// `@datadog/flagging-core/configuration`. Do not use the deprecated package-root +// aliases because they parse precomputed data only and ignore rules. Keep +// `FlagsConfiguration` and the rules evaluator on the package root. The +// `@datadog/flagging-core/precomputed` // subpath is protobuf-free, ignores rules, and is not the parser for this module. -// PR #336 through `772167b` adds browser providers and shared lifecycle error -// selection. Its tree is identical to the previous `6d3d6a4` tree, so it does -// not change this core parser boundary. Do not import +// PR #336 through `dde93ea` adds browser providers and shared lifecycle error +// selection. Its latest commit removes unrelated `extraLogging` test coverage +// and does not change this core parser boundary. Do not import // `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 // encoding of the raw dd-source#34959 protobuf response in the version 1 @@ -43,7 +42,8 @@ import type { // service transport or envelope construction here. PR #344 preserves decoded // protobuf flags and integers. Its evaluator reports invalid reached data, // unsupported feature levels, and unsafe integer conversion as deterministic -// flag-scoped `PARSE_ERROR` results. +// flag-scoped `PARSE_ERROR` results. It also validates membership ordering, +// semantic-version bounds, and 32-byte SHA-256 digests. type PendingRulesConfiguration = FlagsConfiguration & { configurationError?: string; rulesError?: string; @@ -122,9 +122,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `03cde21`. Its safe-integer conversion does not require global `BigInt`. + // `5a5511e`. Its safe-integer conversion does not require global `BigInt`. // TODO(FFL-2837): Delete this parse-error compatibility behavior when the - // dependency contains PR #344 through `03cde21`. The upstream parser uses + // dependency contains PR #344 through `5a5511e`. The upstream parser uses // `configurationError` for an invalid envelope and `rulesError` for an // invalid rules entry or response. It keeps a valid sibling branch. const pendingRules = readPendingRulesWire(source); @@ -155,8 +155,8 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `03cde21` and - // its required SHA digest follow-up. The upstream serializer encodes generated protobuf + // types above after the dependency contains PR #344 through `5a5511e`. + // The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. if (pendingConfiguration.rulesBased) { From 98b4fd32ea7e98d666f7b20aac899564c6677b8e Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 20 Aug 2026 11:55:05 -0400 Subject: [PATCH 20/20] docs(flags): align TODOs with final upstream contracts --- .../configuration/__tests__/rules.test.ts | 9 ++++-- .../core/src/flags/configuration/index.ts | 5 ++-- .../core/src/flags/configuration/rules.ts | 23 ++++++++------- packages/core/src/flags/configuration/wire.ts | 28 +++++++++---------- .../src/configuration.ts | 5 ++-- 5 files changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index ad50b6455..5bbf51a67 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -275,12 +275,17 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this legacy JSON compatibility test with a // generated protobuf fixture after a flagging-core release contains - // DataDog/openfeature-js-client#344 through `5a5511e`. Round-trip the + // DataDog/openfeature-js-client#344 through `78a0c14`. Round-trip the // generated fixture and confirm that serialization preserves the unknown field. // Add a fixture with an unsupported minimum feature level and require a // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. Also cover unsorted // string and SHA-256 membership indexes, invalid SHA digest lengths, and // semantic-version components at and above the unsigned 64-bit limit. + // Cover supported primitive coercion, strict finite numeric strings, + // rejected arrays, objects, empty strings, hexadecimal strings and infinity, + // and string-only semantic-version operands. Add non-ASCII membership cases + // that distinguish UTF-8 code-point order from JavaScript UTF-16 order, and + // prove that one shared condition is evaluated once per flag resolution. // Confirm that an absent inherited `__proto__` attribute does not match and // that an explicit own `__proto__` context attribute remains usable. The // legacy evaluator's compiled object spread cannot preserve that key. @@ -310,7 +315,7 @@ describe('rules configuration', () => { // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range // protobuf `int64` fixture after flagging-core contains PR #344 at or after - // `5a5511e`. The generated parser must preserve the source value as `bigint` + // `78a0c14`. The generated parser must preserve the source value as `bigint` // where supported. Run the same evaluation with // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`. it('returns PARSE_ERROR instead of serving an unsafe integer', () => { diff --git a/packages/core/src/flags/configuration/index.ts b/packages/core/src/flags/configuration/index.ts index 77737bb6e..0cadf111d 100644 --- a/packages/core/src/flags/configuration/index.ts +++ b/packages/core/src/flags/configuration/index.ts @@ -11,8 +11,9 @@ // here makes a future "port -> depend on a shared core" swap easier. // TODO(FFL-2837): Re-export `getPrecomputedContext` from -// `@datadog/flagging-core/configuration` here after a flagging-core release contains -// DataDog/openfeature-js-client#353 through `caae6ab`. Also expose it from the +// `@datadog/flagging-core` here after a flagging-core release contains +// DataDog/openfeature-js-client#344 through `78a0c14`, including merged PR #353. +// Also expose it from the // public React Native SDK entry point for the OpenFeature package to consume. export { configurationFromString, configurationToString } from './wire'; diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 6bd523dfb..279e0cbcb 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -14,7 +14,7 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; // TODO(FFL-2837): Replace this legacy UFC v1 alias with // `NonNullable['response']` after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `5a5511e`. +// release contains DataDog/openfeature-js-client#344 through `78a0c14`. // Keep the `FlagsConfiguration` type // import on the flagging-core package root. PR #344 preserves protobuf integers // as `bigint`, does not call global `BigInt` during safe conversion, and reports @@ -121,7 +121,7 @@ export const toRulesEvaluationContext = ( } // TODO(FFL-2837): Delete the inherited-name shadows after a flagging-core - // release contains DataDog/openfeature-js-client#344 through `aa93230` and + // release contains DataDog/openfeature-js-client#344 through `78a0c14` and // this adapter uses its generated protobuf evaluator. That evaluator uses // own-property lookup for condition and shard context attributes. const rulesContext = Object.create(null) as RulesEvaluationContext; @@ -140,11 +140,13 @@ const hasOwn = (value: object, key: PropertyKey): boolean => Object.prototype.hasOwnProperty.call(value, key); // TODO(FFL-2837): Delete this compatibility error store after a flagging-core -// release contains DataDog/openfeature-js-client#344 through `5a5511e`. +// release contains DataDog/openfeature-js-client#344 through `78a0c14`. // The generated protobuf evaluator validates the requested flag and the data // that evaluation reaches. It does not build this error map during parsing. // It returns deterministic `PARSE_ERROR` results, including for an integer that -// is not a safe JavaScript number. +// is not a safe JavaScript number. Its protobuf conditions also use stricter +// primitive coercion than this temporary legacy JSON evaluator; do not copy that +// operator logic into this compatibility layer. const errorsByConfiguration = new WeakMap< RulesConfigurationResponse, ReadonlyMap @@ -219,8 +221,9 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 through - // `5a5511e` compiles protobuf regular expressions lazily and caches - // them by configuration and index, but it does not limit patterns. + // `78a0c14` compiles protobuf regular expressions lazily and caches + // them by configuration and index. It also memoizes a condition result + // during one flag evaluation, but it does not limit patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -474,7 +477,7 @@ export const prepareRulesConfiguration = ( const clone = cloneValue(value); // TODO(FFL-2837): Delete this legacy JSON clone and validator after a - // flagging-core release contains upstream PR #344 through `5a5511e`. That + // flagging-core release contains upstream PR #344 through `78a0c14`. That // implementation preserves protobuf integers as `bigint` and validates only // the requested flag data that evaluation reaches. It does not call global // `BigInt` when it returns a deterministic per-flag error for an unsafe number. @@ -529,7 +532,7 @@ const normalizeVariationType = ( // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the // flagging-core dependency contains DataDog/openfeature-js-client#344 through -// `5a5511e`. The protobuf evaluator maps only safely represented integer +// `78a0c14`. The protobuf evaluator maps only safely represented integer // variations, and all numeric variations, to the OpenFeature type `number`. const recoverVariationType = ( configuration: RulesConfigurationResponse, @@ -556,7 +559,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { // TODO(FFL-2837): Delete this local compatibility guard after the // flagging-core dependency contains DataDog/openfeature-js-client#344 - // through `5a5511e`. Keep the reserved-name contract tests. + // through `78a0c14`. Keep the reserved-name contract tests. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, @@ -567,7 +570,7 @@ export const flaggingCoreRulesEngine: RulesEngine = { } // TODO(FFL-2837): Delete this compatibility check with the local error - // store after the published PR #344 evaluator through `5a5511e` validates + // store after the published PR #344 evaluator through `78a0c14` validates // reached flag data and reports deterministic flag-scoped errors, including // unsupported feature levels and unsafe integer conversions with and // without global `BigInt` when supported. diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index e2fdfdde4..748ed9a9d 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -4,10 +4,10 @@ * Copyright 2016-Present Datadog, Inc. */ -// Published flagging-core 2.0.2 exports wire conversion from its package root. PR #344 moves -// that conversion to the opt-in `@datadog/flagging-core/configuration` entry point so the default -// entry point does not load Protobuf-ES. In both versions, the input is the complete portable JSON -// envelope. It is not the raw protobuf or legacy JSON response from the UFC service. +// Published flagging-core 2.0.2 exports wire conversion from its package root. PR #344 keeps +// precomputed-only conversion on the protobuf-free package root and exposes complete conversion +// from `@datadog/flagging-core/rules-based`. In both versions, the input is the complete portable +// JSON envelope. It is not the raw protobuf or legacy JSON response from the UFC service. // Published `configurationFromString` is lenient: it returns an empty configuration // (`{}`) for malformed input or an unsupported wire version rather than throwing. // PR #344 preserves that failure as `configurationError` instead. @@ -24,16 +24,14 @@ import type { // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers // after a flagging-core release contains DataDog/openfeature-js-client#344 -// through `5a5511e`. +// through `78a0c14`. // Import and re-export the wire functions and `FlagsConfigurationWire` type from -// `@datadog/flagging-core/configuration`. Do not use the deprecated package-root -// aliases because they parse precomputed data only and ignore rules. Keep +// `@datadog/flagging-core/rules-based`. Do not use the package-root parser for +// rules because it parses precomputed data only and ignores rules. Keep // `FlagsConfiguration` and the rules evaluator on the package root. The -// `@datadog/flagging-core/precomputed` -// subpath is protobuf-free, ignores rules, and is not the parser for this module. -// PR #336 through `dde93ea` adds browser providers and shared lifecycle error -// selection. Its latest commit removes unrelated `extraLogging` test coverage -// and does not change this core parser boundary. Do not import +// package-root parser is protobuf-free and is not the parser for this module. +// PR #336 through `9fd61c4` adds browser providers and shared lifecycle error +// selection. It uses the same `/rules-based` parser boundary. Do not import // `@datadog/openfeature-browser` in React Native. // Use `FlagsConfiguration.rules`. The distribution layer must put one base64 // encoding of the raw dd-source#34959 protobuf response in the version 1 @@ -122,9 +120,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { // validator that PR #344 removed in favor of the Protobuf-ES decoder. The // published parser must also include PR #344's unknown-field tolerance, // unknown-field serialization, and lossless integer parsing through - // `5a5511e`. Its safe-integer conversion does not require global `BigInt`. + // `78a0c14`. Its safe-integer conversion does not require global `BigInt`. // TODO(FFL-2837): Delete this parse-error compatibility behavior when the - // dependency contains PR #344 through `5a5511e`. The upstream parser uses + // dependency contains PR #344 through `78a0c14`. The upstream parser uses // `configurationError` for an invalid envelope and `rulesError` for an // invalid rules entry or response. It keeps a valid sibling branch. const pendingRules = readPendingRulesWire(source); @@ -155,7 +153,7 @@ export const configurationToString = ( const pendingConfiguration = configuration as PendingRulesConfiguration; // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending - // types above after the dependency contains PR #344 through `5a5511e`. + // types above after the dependency contains PR #344 through `78a0c14`. // The upstream serializer encodes generated protobuf // rules back to base64 and preserves unknown protobuf fields. // This temporary UFC v1 shim serializes its legacy JSON response instead. diff --git a/packages/react-native-openfeature/src/configuration.ts b/packages/react-native-openfeature/src/configuration.ts index ccf573c0f..ffd64a09c 100644 --- a/packages/react-native-openfeature/src/configuration.ts +++ b/packages/react-native-openfeature/src/configuration.ts @@ -11,8 +11,9 @@ import type { } from '@openfeature/web-sdk'; // TODO(FFL-2837): Delete this local helper and its clone-semantics tests after a -// flagging-core release contains DataDog/openfeature-js-client#353 through -// `caae6ab` and `@datadog/mobile-react-native` re-exports the upstream helper. +// flagging-core release contains DataDog/openfeature-js-client#344 through +// `78a0c14`, including merged PR #353, and `@datadog/mobile-react-native` +// re-exports the upstream package-root helper. // Import and re-export `getPrecomputedContext` from the React Native SDK instead. // Raise the React Native SDK peer and development dependency minimums to the first // release that exports it. Replace these semantic tests with one package-root