diff --git a/README.md b/README.md index 0224c0714..26a47b8e1 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,30 @@ const { accessToken } = await workos.userManagement.authenticateWithCode({ }); ``` +## Feature flag runtime + +The runtime client requests the v2 flag payload and evaluates rules in array order: +when a flag is off it serves `off_value`; otherwise the first matching rule wins, +then `default_value`. A matching rule can serve `false`. Conditions within a rule +must all match; `one_of` matches an exact ID for its target type. + +`isEnabled(key, context, defaultValue)` keeps the same API. Unknown rule kinds and +condition operators do not match. If the selected value is not a boolean, the +caller-provided default applies (`false` when omitted). `getAllFlags` also uses +`false` for values this SDK cannot interpret. + +Older APIs that return the flat v1 payload remain supported. `bootstrapFlags` +accepts either that legacy map or the complete `{ version: 2, flags: { ... } }` +envelope. Unrecognized bootstrap data is ignored and readiness resolves; polling +can populate the cache afterward. Failed polls retain the last good snapshot. + +**Configuration shape change:** `getFlag()` and the `previous` / `current` +snapshots in `change` events now return v2 entries with `off_value` and `rules`, +including when the API returns v1. Code inspecting `targets` must switch to +`rules`. Keep the versioned envelope when saving v2 bootstrap data; a bare map of +v2 entries is not a bootstrap payload. Reordering rules emits a change event; +reordering IDs within the same condition does not. + ## SDK Versioning For our SDKs WorkOS follows a Semantic Versioning ([SemVer](https://semver.org/)) process where all releases will have a version X.Y.Z (like 1.0.0) pattern wherein Z would be a bug fix (e.g., 1.0.1), Y would be a minor release (1.1.0) and X would be a major release (2.0.0). We permit any breaking changes to only be released in major versions and strongly recommend reading changelogs before making any major version upgrades. diff --git a/src/feature-flags/evaluator.spec.ts b/src/feature-flags/evaluator.spec.ts index eaac42b61..27e45129d 100644 --- a/src/feature-flags/evaluator.spec.ts +++ b/src/feature-flags/evaluator.spec.ts @@ -1,5 +1,6 @@ import { Evaluator } from './evaluator'; import { InMemoryStore } from './in-memory-store'; +import { toV2 } from './payload'; import { EvaluationContext, FlagPollEntry, @@ -64,12 +65,14 @@ describe('Evaluator', () => { error: jest.fn(), }; evaluator = new Evaluator(store, logger); - store.swap({ + const payload = toV2({ 'enabled-flag': enabledFlag, 'disabled-flag': disabledFlag, 'targeted-flag': targetedFlag, 'default-on-flag': defaultOnFlag, }); + if (!payload) throw new Error('Invalid legacy test payload'); + store.swap(payload.flags); }); describe('isEnabled', () => { diff --git a/src/feature-flags/evaluator.ts b/src/feature-flags/evaluator.ts index 1089293a7..f5e69ea11 100644 --- a/src/feature-flags/evaluator.ts +++ b/src/feature-flags/evaluator.ts @@ -2,7 +2,7 @@ import { InMemoryStore } from './in-memory-store'; import { EvaluationContext, EvaluationResource, - FlagPollEntry, + FlagPollEntryV2, RuntimeClientLogger, } from './interfaces'; @@ -44,17 +44,16 @@ export class Evaluator { // per flag. const normalizedContext = this.normalizeContext(context); const flags = this.store.getAll(); - const result: Record = {}; - - for (const slug of Object.keys(flags)) { - result[slug] = this.evaluate(flags[slug], normalizedContext, false); - } - - return result; + return Object.fromEntries( + Object.entries(flags).map(([slug, flag]) => [ + slug, + this.evaluate(flag, normalizedContext, false), + ]), + ); } private evaluate( - entry: FlagPollEntry | undefined, + entry: FlagPollEntryV2 | undefined, normalizedContext: Map, defaultValue: boolean, ): boolean { @@ -63,18 +62,31 @@ export class Evaluator { } if (!entry.enabled) { - return false; + return this.servedValue(entry.off_value, defaultValue); } - // Evaluation is enable-only: any enabled target matching the context - // turns the flag on, with no precedence between target types. - for (const [targetType, targetId] of normalizedContext) { - if (this.hasEnabledTarget(entry, targetType, targetId)) { - return true; + for (const rule of entry.rules) { + if (rule.kind !== 'conditions' || !rule.conditions?.length) continue; + const matches = rule.conditions.every((condition) => { + if ( + condition.operator !== 'one_of' || + !condition.target_type || + !Array.isArray(condition.values) + ) + return false; + const id = normalizedContext.get(condition.target_type); + return id !== undefined && condition.values.includes(id); + }); + if (matches) { + return this.servedValue(rule.value, defaultValue); } } - return entry.default_value; + return this.servedValue(entry.default_value, defaultValue); + } + + private servedValue(value: unknown, defaultValue: boolean): boolean { + return typeof value === 'boolean' ? value : defaultValue; } /** @@ -85,6 +97,14 @@ export class Evaluator { */ private normalizeContext(context: EvaluationContext): Map { const normalized = new Map(); + if ( + typeof context !== 'object' || + context === null || + Array.isArray(context) + ) { + this.logger?.warn('Ignoring invalid evaluation context'); + return normalized; + } const record: Record = context; const legacyEntries: Array<[string, string]> = []; @@ -161,29 +181,4 @@ export class Evaluator { return normalized; } - - /** - * A target participates in evaluation only while its `enabled` is true. A - * `false` value is reserved for future disabled overrides and is treated - * as if the target were absent. - */ - private hasEnabledTarget( - entry: FlagPollEntry, - targetType: string, - targetId: string, - ): boolean { - if (targetType === 'user') { - return entry.targets.users.some((t) => t.id === targetId && t.enabled); - } - - if (targetType === 'organization') { - return entry.targets.organizations.some( - (t) => t.id === targetId && t.enabled, - ); - } - - return (entry.targets.custom_targets ?? []).some( - (t) => t.type === targetType && t.id === targetId && t.enabled, - ); - } } diff --git a/src/feature-flags/in-memory-store.spec.ts b/src/feature-flags/in-memory-store.spec.ts index c567dd25e..62e36417c 100644 --- a/src/feature-flags/in-memory-store.spec.ts +++ b/src/feature-flags/in-memory-store.spec.ts @@ -1,24 +1,23 @@ import { InMemoryStore } from './in-memory-store'; -import { FlagPollEntry } from './interfaces'; +import { FlagPollEntryV2 } from './interfaces'; describe('InMemoryStore', () => { let store: InMemoryStore; - const flagA: FlagPollEntry = { + const flagA: FlagPollEntryV2 = { slug: 'flag-a', enabled: true, default_value: true, - targets: { users: [], organizations: [] }, + off_value: false, + rules: [], }; - const flagB: FlagPollEntry = { + const flagB: FlagPollEntryV2 = { slug: 'flag-b', enabled: false, default_value: false, - targets: { - users: [{ id: 'user_123', enabled: true }], - organizations: [], - }, + off_value: false, + rules: [], }; beforeEach(() => { diff --git a/src/feature-flags/in-memory-store.ts b/src/feature-flags/in-memory-store.ts index d93c009c7..0a50e3c2d 100644 --- a/src/feature-flags/in-memory-store.ts +++ b/src/feature-flags/in-memory-store.ts @@ -1,17 +1,17 @@ -import { FlagPollEntry, FlagPollResponse } from './interfaces'; +import { FlagPollEntryV2, FlagPollResponseV2 } from './interfaces'; export class InMemoryStore { - private flags: FlagPollResponse = {}; + private flags: FlagPollResponseV2['flags'] = {}; - swap(newFlags: FlagPollResponse): void { + swap(newFlags: FlagPollResponseV2['flags']): void { this.flags = { ...newFlags }; } - get(slug: string): FlagPollEntry | undefined { - return this.flags[slug]; + get(slug: string): FlagPollEntryV2 | undefined { + return Object.hasOwn(this.flags, slug) ? this.flags[slug] : undefined; } - getAll(): FlagPollResponse { + getAll(): FlagPollResponseV2['flags'] { return { ...this.flags }; } diff --git a/src/feature-flags/interfaces/flag-change.interface.ts b/src/feature-flags/interfaces/flag-change.interface.ts index 3b5e29662..a55d17526 100644 --- a/src/feature-flags/interfaces/flag-change.interface.ts +++ b/src/feature-flags/interfaces/flag-change.interface.ts @@ -1,7 +1,7 @@ -import { FlagPollEntry } from './flag-poll-response.interface'; +import { FlagPollEntryV2 } from './flag-poll-response.interface'; export interface FlagChange { key: string; - previous: FlagPollEntry | null; - current: FlagPollEntry | null; + previous: FlagPollEntryV2 | null; + current: FlagPollEntryV2 | null; } diff --git a/src/feature-flags/interfaces/flag-poll-response.interface.ts b/src/feature-flags/interfaces/flag-poll-response.interface.ts index 762f53ed4..801de2405 100644 --- a/src/feature-flags/interfaces/flag-poll-response.interface.ts +++ b/src/feature-flags/interfaces/flag-poll-response.interface.ts @@ -21,4 +21,36 @@ export interface FlagPollEntry { }; } -export type FlagPollResponse = Record; +export type FlagPollResponseV1 = Record; + +export interface FlagConditionV2 { + // Future operators may carry different operands. Unsupported conditions + // never match, even if another condition in the rule matches. + operator: string; + target_type?: string; + values?: unknown; +} + +export interface FlagRuleV2 { + id: string; + kind: string; + // Future multivariate values must fall back to the caller's boolean default. + value: unknown; + conditions?: FlagConditionV2[]; +} + +export interface FlagPollEntryV2 { + slug: string; + enabled: boolean; + default_value: unknown; + off_value: unknown; + /** Evaluated in array order; the first matching rule wins. */ + rules: FlagRuleV2[]; +} + +export interface FlagPollResponseV2 { + version: 2; + flags: Record; +} + +export type FlagPollResponse = FlagPollResponseV1 | FlagPollResponseV2; diff --git a/src/feature-flags/interfaces/runtime-client-options.interface.ts b/src/feature-flags/interfaces/runtime-client-options.interface.ts index 02726bd61..6cb00e536 100644 --- a/src/feature-flags/interfaces/runtime-client-options.interface.ts +++ b/src/feature-flags/interfaces/runtime-client-options.interface.ts @@ -1,4 +1,4 @@ -import { FlagPollEntry } from './flag-poll-response.interface'; +import { FlagPollResponse } from './flag-poll-response.interface'; export interface RuntimeClientLogger { debug(...args: unknown[]): void; @@ -9,7 +9,8 @@ export interface RuntimeClientLogger { export interface RuntimeClientOptions { pollingIntervalMs?: number; - bootstrapFlags?: Record; + /** A legacy flat payload or a versioned v2 envelope, as returned by polling. */ + bootstrapFlags?: FlagPollResponse; requestTimeoutMs?: number; logger?: RuntimeClientLogger; } diff --git a/src/feature-flags/payload.ts b/src/feature-flags/payload.ts new file mode 100644 index 000000000..670ffd61c --- /dev/null +++ b/src/feature-flags/payload.ts @@ -0,0 +1,129 @@ +import { + FlagConditionV2, + FlagCustomTarget, + FlagPollEntry, + FlagPollEntryV2, + FlagPollResponseV2, + FlagRuleV2, + FlagTarget, +} from './interfaces'; + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isTarget(value: unknown): value is FlagTarget { + return ( + isRecord(value) && + typeof value.id === 'string' && + typeof value.enabled === 'boolean' + ); +} + +function isCustomTarget(value: unknown): value is FlagCustomTarget { + return isTarget(value) && 'type' in value && typeof value.type === 'string'; +} + +function isV1Entry(value: unknown): value is FlagPollEntry { + return ( + isRecord(value) && + typeof value.slug === 'string' && + typeof value.enabled === 'boolean' && + typeof value.default_value === 'boolean' && + isRecord(value.targets) && + Array.isArray(value.targets.users) && + value.targets.users.every(isTarget) && + Array.isArray(value.targets.organizations) && + value.targets.organizations.every(isTarget) && + (value.targets.custom_targets === undefined || + (Array.isArray(value.targets.custom_targets) && + value.targets.custom_targets.every(isCustomTarget))) + ); +} + +function isCondition(value: unknown): value is FlagConditionV2 { + return ( + isRecord(value) && + typeof value.operator === 'string' && + (value.target_type === undefined || typeof value.target_type === 'string') + ); +} + +function isRule(value: unknown): value is FlagRuleV2 { + return ( + isRecord(value) && + typeof value.id === 'string' && + typeof value.kind === 'string' && + (value.conditions === undefined || + (Array.isArray(value.conditions) && value.conditions.every(isCondition))) + ); +} + +function isV2Entry(value: unknown): value is FlagPollEntryV2 { + return ( + isRecord(value) && + typeof value.slug === 'string' && + typeof value.enabled === 'boolean' && + 'default_value' in value && + 'off_value' in value && + Array.isArray(value.rules) && + value.rules.every(isRule) + ); +} + +/** Normalize both wire generations before swapping the store. Unknown shapes + * leave the last good snapshot intact; bootstrap callers can still be ready. */ +export function toV2(response: unknown): FlagPollResponseV2 | undefined { + if (!isRecord(response)) return undefined; + + // A legacy flag can itself be named "version". Its entry is an object, + // unlike the numeric marker on a versioned envelope. + if ('version' in response && !isV1Entry(response.version)) { + if (response.version !== 2 || !isRecord(response.flags)) return undefined; + const entries: Array<[string, FlagPollEntryV2]> = []; + for (const [slug, entry] of Object.entries(response.flags)) { + if (!isV2Entry(entry)) return undefined; + entries.push([slug, entry]); + } + return { version: 2, flags: Object.fromEntries(entries) }; + } + + const entries: Array<[string, FlagPollEntryV2]> = []; + for (const [slug, entry] of Object.entries(response)) { + if (!isV1Entry(entry)) return undefined; + const targetsByType = new Map(); + const addTarget = (type: string, target: FlagTarget) => { + if (!target.enabled) return; + const ids = targetsByType.get(type); + if (ids) ids.push(target.id); + else targetsByType.set(type, [target.id]); + }; + for (const target of entry.targets.users) addTarget('user', target); + for (const target of entry.targets.organizations) + addTarget('organization', target); + for (const target of entry.targets.custom_targets ?? []) + addTarget(target.type, target); + + // Legacy target order has no meaning. Keep the synthetic rule order stable + // so reordering a legacy response doesn't generate change events. + const rules: FlagRuleV2[] = [...targetsByType] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([type, values]) => ({ + id: `v1:${type}`, + kind: 'conditions', + value: true, + conditions: [{ target_type: type, operator: 'one_of', values }], + })); + entries.push([ + slug, + { + slug: entry.slug, + enabled: entry.enabled, + default_value: entry.default_value, + off_value: false, + rules, + }, + ]); + } + return { version: 2, flags: Object.fromEntries(entries) }; +} diff --git a/src/feature-flags/runtime-client-v2.spec.ts b/src/feature-flags/runtime-client-v2.spec.ts new file mode 100644 index 000000000..7fc53103d --- /dev/null +++ b/src/feature-flags/runtime-client-v2.spec.ts @@ -0,0 +1,751 @@ +import fetch from 'jest-fetch-mock'; +import { fetchOnce, fetchURL } from '../common/utils/test-utils'; +import { WorkOS } from '../workos'; +import { + EvaluationContext, + FlagChange, + FlagPollEntryV2, + FlagPollResponse, + FlagPollResponseV2, +} from './interfaces'; + +const workos = new WorkOS('sk_test_example'); +const exclusion = { + id: 'rule_org', + kind: 'conditions', + value: false, + conditions: [ + { target_type: 'organization', operator: 'one_of', values: ['org_1'] }, + ], +}; +const inclusion = { + id: 'rule_user', + kind: 'conditions', + value: true, + conditions: [{ target_type: 'user', operator: 'one_of', values: ['user_1'] }], +}; +const flag: FlagPollEntryV2 = { + slug: 'flag', + enabled: true, + default_value: true, + off_value: false, + rules: [exclusion, inclusion], +}; +const payload: FlagPollResponseV2 = { version: 2, flags: { flag } }; + +describe('v2 runtime payloads', () => { + beforeEach(() => { + fetch.resetMocks(); + jest.useFakeTimers(); + jest.spyOn(Math, 'random').mockReturnValue(0.5); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('requests v2 and evaluates exclusions in payload order', async () => { + fetchOnce(payload); + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + await client.waitUntilReady(); + + expect( + new URL(String(fetchURL())).searchParams.get('payload_version'), + ).toBe('2'); + expect( + client.isEnabled('flag', { userId: 'user_1', organizationId: 'org_1' }), + ).toBe(false); + expect(client.isEnabled('flag', { userId: 'user_1' })).toBe(true); + expect(client.getFlag('flag')).toEqual(flag); + expect(client.getStats().flagCount).toBe(1); + client.close(); + }); + + it.each([ + ['future version', { ...payload, version: 3 }], + ['invalid version', { ...payload, version: '2' }], + ['missing version', { flags: { flag } }], + ['invalid flags', { version: 2, flags: null }], + [ + 'invalid entry', + { version: 2, flags: { flag: { ...flag, rules: null } } }, + ], + ['invalid legacy entry', { flag: { slug: 'flag', targets: null } }], + ['null', null], + ['array', []], + ['scalar', false], + ])( + 'ignores %s bootstrap data, resolves readiness, and then polls', + async (_name, input) => { + const bootstrapFlags: FlagPollResponse = JSON.parse( + JSON.stringify(input), + ); + const client = workos.featureFlags.createRuntimeClient({ + bootstrapFlags, + }); + await client.waitUntilReady(); + + expect(client.getStats().flagCount).toBe(0); + expect(client.isEnabled('flag', {}, true)).toBe(true); + + fetchOnce(payload); + await jest.advanceTimersByTimeAsync(0); + expect(client.getStats().flagCount).toBe(1); + client.close(); + }, + ); + + const cases: Array<{ + name: string; + config: Partial; + context?: EvaluationContext; + fallback?: boolean; + expected: boolean; + }> = [ + { + name: 'off value overrides matching rules and both defaults', + config: { enabled: false }, + context: { userId: 'user_1' }, + fallback: true, + expected: false, + }, + { + name: 'a boolean off value is honored', + config: { enabled: false, off_value: true }, + expected: true, + }, + { + name: 'an empty rule list serves the flag default', + config: { rules: [], default_value: false }, + fallback: true, + expected: false, + }, + { + name: 'no match serves the flag default, not the caller default', + config: {}, + expected: true, + }, + { + name: 'a missing context type does not match', + config: { default_value: false, rules: [inclusion] }, + expected: false, + }, + { + name: 'a matching false rule stops evaluation', + config: {}, + context: { user: { id: 'user_1' }, organization: { id: 'org_1' } }, + fallback: true, + expected: false, + }, + { + name: 'reordering conflicting target types changes the winner', + config: { rules: [inclusion, exclusion] }, + context: { userId: 'user_1', organizationId: 'org_1' }, + expected: true, + }, + { + name: 'a later rule can match', + config: { default_value: false }, + context: { userId: 'user_1' }, + expected: true, + }, + { + name: 'custom target membership can match any listed ID', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [ + { + target_type: 'workspace', + operator: 'one_of', + values: ['ws_0', 'ws_1'], + }, + ], + }, + ], + }, + context: { workspace: { id: 'ws_1' } }, + expected: true, + }, + { + name: 'membership uses exact, case-sensitive IDs', + config: { default_value: false, rules: [inclusion] }, + context: { userId: 'USER_1' }, + expected: false, + }, + { + name: 'an ID from another type does not match', + config: { default_value: false, rules: [inclusion] }, + context: { workspace: { id: 'user_1' } }, + expected: false, + }, + { + name: 'all conditions must match', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [...inclusion.conditions, ...exclusion.conditions], + }, + ], + }, + context: { userId: 'user_1', organizationId: 'org_1' }, + expected: true, + }, + { + name: 'one matching condition cannot override a failed condition', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [...inclusion.conditions, ...exclusion.conditions], + }, + ], + }, + context: { userId: 'user_1', organizationId: 'org_other' }, + expected: false, + }, + { + name: 'one matching condition cannot override a missing context type', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [...inclusion.conditions, ...exclusion.conditions], + }, + ], + }, + context: { userId: 'user_1' }, + expected: false, + }, + { + name: 'empty conditions never match', + config: { + default_value: false, + rules: [{ ...inclusion, conditions: [] }], + }, + expected: false, + }, + { + name: 'empty membership never matches', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [ + { target_type: 'user', operator: 'one_of', values: [] }, + ], + }, + ], + }, + context: { userId: 'user_1' }, + expected: false, + }, + { + name: 'unknown rule kinds are skipped even with matching conditions', + config: { + default_value: false, + rules: [{ ...inclusion, kind: 'rollout' }], + }, + context: { userId: 'user_1' }, + expected: false, + }, + { + name: 'unknown rules without conditions allow later rules to match', + config: { + default_value: false, + rules: [{ id: 'future', kind: 'rollout', value: false }, inclusion], + }, + context: { userId: 'user_1' }, + expected: true, + }, + { + name: 'unknown operators invalidate the whole rule', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [...inclusion.conditions, { operator: 'segment' }], + }, + ], + }, + context: { userId: 'user_1' }, + expected: false, + }, + { + name: 'unknown operators allow later rules to match', + config: { + default_value: false, + rules: [ + { + ...exclusion, + conditions: [ + { + target_type: 'user', + operator: 'not_one_of', + values: ['user_1'], + }, + ], + }, + inclusion, + ], + }, + context: { userId: 'user_1' }, + expected: true, + }, + { + name: 'unparseable matching values use the caller default and stop', + config: { + default_value: true, + rules: [{ ...inclusion, value: 'true' }, inclusion], + }, + context: { userId: 'user_1' }, + fallback: false, + expected: false, + }, + { + name: 'unparseable off values use a true caller default', + config: { enabled: false, off_value: 0, default_value: false }, + fallback: true, + expected: true, + }, + { + name: 'unparseable defaults use the caller default', + config: { rules: [], default_value: 'false' }, + expected: false, + }, + { + name: 'unparseable values in unmatched rules have no effect', + config: { rules: [{ ...exclusion, value: null }] }, + context: { organizationId: 'org_other' }, + expected: true, + }, + { + name: 'malformed membership is nonmatching', + config: { + default_value: false, + rules: [ + { + ...inclusion, + conditions: [ + { target_type: 'user', operator: 'one_of', values: 'user_1' }, + ], + }, + ], + }, + context: { userId: 'user_1' }, + expected: false, + }, + ]; + + it.each(cases)('$name', async ({ config, context, fallback, expected }) => { + const client = workos.featureFlags.createRuntimeClient({ + bootstrapFlags: { version: 2, flags: { flag: { ...flag, ...config } } }, + }); + await client.waitUntilReady(); + expect(client.isEnabled('flag', context, fallback)).toBe(expected); + client.close(); + }); + + it('ignores extra payload keys and evaluates all flags with a false fallback', async () => { + fetchOnce({ + ...payload, + future: { version: 9 }, + flags: { flag, unknown: { ...flag, slug: 'unknown', default_value: 1 } }, + }); + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + expect(client.getAllFlags({ organization: { id: 'org_1' } })).toEqual({ + flag: false, + unknown: false, + }); + expect(client.getAllFlags()).toEqual({ flag: true, unknown: false }); + expect(client.isEnabled('missing', {}, true)).toBe(true); + client.close(); + }); + + it('replaces v2 bootstrap data wholesale on the first poll without change events', async () => { + const client = workos.featureFlags.createRuntimeClient({ + bootstrapFlags: payload, + }); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + expect(client.isEnabled('flag')).toBe(true); + fetchOnce({ version: 2, flags: {} }); + await jest.advanceTimersByTimeAsync(0); + expect(client.getAllFlags()).toEqual({}); + expect(client.getStats().flagCount).toBe(0); + expect(changes).toEqual([]); + client.close(); + }); + + it.each([ + ['rule order', { ...flag, rules: [inclusion, exclusion] }], + [ + 'rule value', + { ...flag, rules: [{ ...exclusion, value: true }, inclusion] }, + ], + [ + 'membership', + { + ...flag, + rules: [ + { + ...inclusion, + conditions: [ + { target_type: 'user', operator: 'one_of', values: ['user_2'] }, + ], + }, + ], + }, + ], + ['off value', { ...flag, off_value: true }], + ['default value', { ...flag, default_value: false }], + ['enabled', { ...flag, enabled: false }], + [ + 'rule kind', + { ...flag, rules: [{ ...exclusion, kind: 'rollout' }, inclusion] }, + ], + [ + 'condition operator', + { + ...flag, + rules: [ + { + ...exclusion, + conditions: [ + { ...exclusion.conditions[0], operator: 'not_one_of' }, + ], + }, + inclusion, + ], + }, + ], + [ + 'target type', + { + ...flag, + rules: [ + { + ...exclusion, + conditions: [ + { ...exclusion.conditions[0], target_type: 'workspace' }, + ], + }, + inclusion, + ], + }, + ], + ['removed rule', { ...flag, rules: [inclusion] }], + ])('emits complete v2 snapshots when %s changes', async (_name, current) => { + fetchOnce(payload); + const client = workos.featureFlags.createRuntimeClient(); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + await jest.advanceTimersByTimeAsync(0); + expect(changes).toEqual([]); + fetchOnce({ version: 2, flags: { flag: current } }); + await jest.advanceTimersByTimeAsync(30_000); + expect(changes).toEqual([{ key: 'flag', previous: flag, current }]); + client.close(); + }); + + it('ignores membership and condition ordering when detecting changes', async () => { + const conditions = [ + { target_type: 'user', operator: 'one_of', values: ['user_1', 'user_2'] }, + { target_type: 'workspace', operator: 'one_of', values: ['ws_1'] }, + ]; + fetchOnce({ + version: 2, + flags: { flag: { ...flag, rules: [{ ...inclusion, conditions }] } }, + }); + const client = workos.featureFlags.createRuntimeClient(); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + await jest.advanceTimersByTimeAsync(0); + fetchOnce({ + version: 2, + flags: { + flag: { + ...flag, + rules: [ + { + ...inclusion, + conditions: [ + conditions[1], + { ...conditions[0], values: ['user_2', 'user_1'] }, + ], + }, + ], + }, + }, + }); + await jest.advanceTimersByTimeAsync(30_000); + expect(changes).toEqual([]); + client.close(); + }); + + it('emits additions and removals while replacing the entire snapshot', async () => { + fetchOnce(payload); + const client = workos.featureFlags.createRuntimeClient(); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + await jest.advanceTimersByTimeAsync(0); + const added = { ...flag, slug: 'added' }; + fetchOnce({ version: 2, flags: { added } }); + await jest.advanceTimersByTimeAsync(30_000); + expect(changes).toEqual([ + { key: 'flag', previous: flag, current: null }, + { key: 'added', previous: null, current: added }, + ]); + expect(client.getAllFlags()).toEqual({ added: true }); + client.close(); + }); + + it.each([ + ['network failure', 'network'], + ['request timeout', 'timeout'], + ['unknown version', { ...payload, version: 3 }], + [ + 'malformed payload', + { version: 2, flags: { flag: { ...flag, rules: null } } }, + ], + ])( + 'keeps the last good snapshot after %s and recovers on the next poll', + async (_name, failure) => { + fetchOnce(payload); + const client = workos.featureFlags.createRuntimeClient({ + requestTimeoutMs: 50, + }); + const changes: FlagChange[] = []; + const errors: Error[] = []; + client.on('change', (change) => changes.push(change)); + client.on('error', (error) => errors.push(error)); + await jest.advanceTimersByTimeAsync(0); + const lastSuccessfulPollAt = client.getStats().lastSuccessfulPollAt; + if (failure === 'network') fetch.mockRejectOnce(new Error('offline')); + else if (failure === 'timeout') + fetch.mockResponseOnce(() => new Promise(() => {})); + else fetchOnce(failure); + await jest.advanceTimersByTimeAsync(30_050); + expect(client.isEnabled('flag', { organizationId: 'org_1' })).toBe(false); + expect(client.getAllFlags()).toEqual({ flag: true }); + expect(client.getStats()).toMatchObject({ + flagCount: 1, + pollErrorCount: 1, + lastSuccessfulPollAt, + }); + expect(errors).toHaveLength(1); + expect(changes).toEqual([]); + fetchOnce({ version: 2, flags: {} }); + await jest.advanceTimersByTimeAsync(30_000); + expect(client.getAllFlags()).toEqual({}); + expect(changes).toEqual([{ key: 'flag', previous: flag, current: null }]); + client.close(); + }, + ); + + it('converts legacy enabled targets to one true rule per type with stable change detection', async () => { + const legacy = { + flag: { + slug: 'flag', + enabled: true, + default_value: false, + targets: { + users: [ + { id: 'user_1', enabled: true }, + { id: 'user_off', enabled: false }, + ], + organizations: [{ id: 'org_off', enabled: false }], + custom_targets: [ + { type: 'workspace', id: 'ws_1', enabled: true }, + { type: 'region', id: 'east', enabled: true }, + { type: 'workspace', id: 'ws_2', enabled: true }, + { type: 'workspace', id: 'ws_off', enabled: false }, + ], + }, + }, + }; + fetchOnce(legacy); + const client = workos.featureFlags.createRuntimeClient(); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + await jest.advanceTimersByTimeAsync(0); + expect(client.getFlag('flag')).toEqual({ + slug: 'flag', + enabled: true, + default_value: false, + off_value: false, + rules: [ + { + id: 'v1:region', + kind: 'conditions', + value: true, + conditions: [ + { target_type: 'region', operator: 'one_of', values: ['east'] }, + ], + }, + { + id: 'v1:user', + kind: 'conditions', + value: true, + conditions: [ + { target_type: 'user', operator: 'one_of', values: ['user_1'] }, + ], + }, + { + id: 'v1:workspace', + kind: 'conditions', + value: true, + conditions: [ + { + target_type: 'workspace', + operator: 'one_of', + values: ['ws_1', 'ws_2'], + }, + ], + }, + ], + }); + expect(client.isEnabled('flag', { workspace: { id: 'ws_2' } })).toBe(true); + expect(client.isEnabled('flag', { workspace: { id: 'ws_off' } })).toBe( + false, + ); + fetchOnce({ + flag: { + ...legacy.flag, + targets: { + ...legacy.flag.targets, + custom_targets: [...legacy.flag.targets.custom_targets].reverse(), + }, + }, + }); + await jest.advanceTimersByTimeAsync(30_000); + expect(changes).toEqual([]); + // An API upgrade can start returning rules without restarting the SDK. + fetchOnce(payload); + await jest.advanceTimersByTimeAsync(30_000); + expect(client.isEnabled('flag', { organizationId: 'org_1' })).toBe(false); + // A rollback to an older API still replaces and evaluates the snapshot. + fetchOnce(legacy); + await jest.advanceTimersByTimeAsync(30_000); + expect(client.isEnabled('flag', { workspace: { id: 'ws_2' } })).toBe(true); + expect(client.getStats().pollErrorCount).toBe(0); + client.close(); + }); + + it.each([1, 2])( + 'accepts an empty v%s bootstrap snapshot', + async (version) => { + const bootstrapFlags: FlagPollResponse = + version === 1 ? {} : { version: 2, flags: {} }; + const client = workos.featureFlags.createRuntimeClient({ + bootstrapFlags, + }); + await client.waitUntilReady(); + expect(client.getAllFlags()).toEqual({}); + expect(client.isEnabled('absent', {}, true)).toBe(true); + client.close(); + }, + ); + + it.each([1, 2])( + 'preserves reserved-looking flag slugs in v%s payloads', + async (version) => { + const entries = ['version', 'flags', '__proto__', 'constructor'].map( + (slug) => [ + slug, + version === 1 + ? { + slug, + enabled: true, + default_value: true, + targets: { users: [], organizations: [] }, + } + : { ...flag, slug }, + ], + ); + const flags = Object.fromEntries(entries); + fetchOnce(version === 1 ? flags : { version: 2, flags }); + const client = workos.featureFlags.createRuntimeClient(); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + expect(client.getFlag('constructor')).toBeUndefined(); + expect(client.isEnabled('toString', {}, true)).toBe(true); + await jest.advanceTimersByTimeAsync(0); + expect(client.getAllFlags()).toEqual( + JSON.parse( + '{"version":true,"flags":true,"__proto__":true,"constructor":true}', + ), + ); + expect(client.getFlag('__proto__')?.slug).toBe('__proto__'); + fetchOnce({ version: 2, flags: {} }); + await jest.advanceTimersByTimeAsync(30_000); + expect(changes).toHaveLength(4); + expect(changes.every((change) => change.current === null)).toBe(true); + expect(client.getFlag('constructor')).toBeUndefined(); + client.close(); + }, + ); + + it('does not mark a malformed initial poll ready or emit changes on recovery', async () => { + fetchOnce({ version: 99, flags: {} }); + const client = workos.featureFlags.createRuntimeClient(); + client.on('error', () => {}); + const changes: FlagChange[] = []; + client.on('change', (change) => changes.push(change)); + let ready = false; + const readiness = client.waitUntilReady().then(() => { + ready = true; + }); + await jest.advanceTimersByTimeAsync(0); + expect(ready).toBe(false); + expect(client.getStats().lastSuccessfulPollAt).toBeNull(); + fetchOnce(payload); + await jest.advanceTimersByTimeAsync(30_000); + await readiness; + expect(ready).toBe(true); + expect(changes).toEqual([]); + client.close(); + }); + + it('retains the v2 snapshot and stops polling after a 401', async () => { + fetchOnce(payload); + const client = workos.featureFlags.createRuntimeClient(); + const failures: Error[] = []; + client.on('error', () => {}); + client.on('failed', (error) => failures.push(error)); + await jest.advanceTimersByTimeAsync(0); + fetchOnce({ message: 'Unauthorized' }, { status: 401 }); + await jest.advanceTimersByTimeAsync(30_000); + expect(failures).toHaveLength(1); + expect(client.getAllFlags({ organizationId: 'org_1' })).toEqual({ + flag: false, + }); + await client.waitUntilReady(); + await jest.advanceTimersByTimeAsync(120_000); + expect(fetch.mock.calls).toHaveLength(2); + client.close(); + }); + + it.each([null, false, 'invalid', []])( + 'never throws on a malformed top-level context: %j', + (input) => { + const context: EvaluationContext = JSON.parse(JSON.stringify(input)); + const client = workos.featureFlags.createRuntimeClient({ + bootstrapFlags: payload, + }); + expect(client.isEnabled('flag', context)).toBe(true); + expect(client.getAllFlags(context)).toEqual({ flag: true }); + client.close(); + }, + ); +}); diff --git a/src/feature-flags/runtime-client.spec.ts b/src/feature-flags/runtime-client.spec.ts index 249112565..ad0e798b6 100644 --- a/src/feature-flags/runtime-client.spec.ts +++ b/src/feature-flags/runtime-client.spec.ts @@ -3,7 +3,7 @@ import { fetchOnce, fetchURL } from '../common/utils/test-utils'; import { UnauthorizedException } from '../common/exceptions'; import { WorkOS } from '../workos'; import { FeatureFlagsRuntimeClient } from './runtime-client'; -import { FlagPollResponse } from './interfaces'; +import { FlagPollResponse, FlagPollResponseV2 } from './interfaces'; const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU'); @@ -25,6 +25,32 @@ const pollResponse: FlagPollResponse = { }, }; +const normalizedFlags: FlagPollResponseV2['flags'] = { + 'flag-a': { + slug: 'flag-a', + enabled: true, + default_value: true, + off_value: false, + rules: [], + }, + 'flag-b': { + slug: 'flag-b', + enabled: true, + default_value: false, + off_value: false, + rules: [ + { + id: 'v1:user', + kind: 'conditions', + value: true, + conditions: [ + { target_type: 'user', operator: 'one_of', values: ['user_123'] }, + ], + }, + ], + }, +}; + describe('FeatureFlagsRuntimeClient', () => { beforeEach(() => { fetch.resetMocks(); @@ -178,12 +204,12 @@ describe('FeatureFlagsRuntimeClient', () => { }); describe('getFlag', () => { - it('returns raw flag entry', async () => { + it('returns normalized flag configuration', async () => { const client = createClientAndWait(); await jest.advanceTimersByTimeAsync(0); await client.waitUntilReady(); - expect(client.getFlag('flag-a')).toEqual(pollResponse['flag-a']); + expect(client.getFlag('flag-a')).toEqual(normalizedFlags['flag-a']); expect(client.getFlag('unknown')).toBeUndefined(); client.close(); @@ -266,8 +292,8 @@ describe('FeatureFlagsRuntimeClient', () => { expect(changes).toEqual([ { key: 'flag-a', - previous: pollResponse['flag-a'], - current: updatedResponse['flag-a'], + previous: normalizedFlags['flag-a'], + current: { ...normalizedFlags['flag-a'], enabled: false }, }, ]); @@ -301,8 +327,25 @@ describe('FeatureFlagsRuntimeClient', () => { expect(changes).toEqual([ { key: 'flag-b', - previous: pollResponse['flag-b'], - current: updatedResponse['flag-b'], + previous: normalizedFlags['flag-b'], + current: { + ...normalizedFlags['flag-b'], + rules: [ + ...normalizedFlags['flag-b'].rules, + { + id: 'v1:workspace', + kind: 'conditions', + value: true, + conditions: [ + { + target_type: 'workspace', + operator: 'one_of', + values: ['ws_123'], + }, + ], + }, + ], + }, }, ]); @@ -324,7 +367,7 @@ describe('FeatureFlagsRuntimeClient', () => { expect(changes).toEqual([ { key: 'flag-b', - previous: pollResponse['flag-b'], + previous: normalizedFlags['flag-b'], current: null, }, ]); diff --git a/src/feature-flags/runtime-client.ts b/src/feature-flags/runtime-client.ts index e13841597..c37fa1e8b 100644 --- a/src/feature-flags/runtime-client.ts +++ b/src/feature-flags/runtime-client.ts @@ -3,13 +3,13 @@ import { WorkOS } from '../workos'; import { UnauthorizedException } from '../common/exceptions'; import { InMemoryStore } from './in-memory-store'; import { Evaluator } from './evaluator'; +import { toV2 } from './payload'; import { EvaluationContext, FlagChange, - FlagCustomTarget, - FlagPollEntry, + FlagPollEntryV2, FlagPollResponse, - FlagTarget, + FlagPollResponseV2, RuntimeClientOptions, RuntimeClientLogger, RuntimeClientStats, @@ -80,8 +80,9 @@ export class FeatureFlagsRuntimeClient extends EventEmitter // Prevent unhandled rejection if no one awaits waitUntilReady this.readyPromise.catch(() => {}); - if (options.bootstrapFlags) { - this.store.swap(options.bootstrapFlags); + if (options.bootstrapFlags !== undefined) { + const bootstrap = toV2(options.bootstrapFlags); + if (bootstrap) this.store.swap(bootstrap.flags); this.stats.flagCount = this.store.size; this.resolveReady(); } @@ -133,7 +134,8 @@ export class FeatureFlagsRuntimeClient extends EventEmitter return this.evaluator.getAllFlags(context); } - getFlag(flagKey: string): FlagPollEntry | undefined { + /** Returns normalized v2 configuration, including for legacy poll responses. */ + getFlag(flagKey: string): FlagPollEntryV2 | undefined { return this.store.get(flagKey); } @@ -164,15 +166,16 @@ export class FeatureFlagsRuntimeClient extends EventEmitter this.stats.pollCount++; this.stats.lastPollAt = new Date(); - const data = await this.fetchWithTimeout(); + const data = toV2(await this.fetchWithTimeout()); + if (!data) throw new Error('Unsupported feature flag payload'); - this.store.swap(data); + this.store.swap(data.flags); this.stats.lastSuccessfulPollAt = new Date(); this.stats.flagCount = this.store.size; this.consecutiveErrors = 0; if (this.initialized) { - this.emitChanges(previousFlags, data); + this.emitChanges(previousFlags, data.flags); } this.initialized = true; this.resolveReady(); @@ -208,7 +211,7 @@ export class FeatureFlagsRuntimeClient extends EventEmitter let timeoutId: ReturnType; const fetchPromise = this.workos - .get('/sdk/feature-flags') + .get('/sdk/feature-flags?payload_version=2') .then(({ data }) => data); const timeoutPromise = new Promise((_, reject) => { @@ -262,8 +265,8 @@ export class FeatureFlagsRuntimeClient extends EventEmitter } private emitChanges( - previous: FlagPollResponse, - current: FlagPollResponse, + previous: FlagPollResponseV2['flags'], + current: FlagPollResponseV2['flags'], ): void { if (!previous || !current) { return; @@ -275,8 +278,8 @@ export class FeatureFlagsRuntimeClient extends EventEmitter ]); for (const key of allKeys) { - const prev = previous[key]; - const curr = current[key]; + const prev = Object.hasOwn(previous, key) ? previous[key] : undefined; + const curr = Object.hasOwn(current, key) ? current[key] : undefined; if (this.hasEntryChanged(prev, curr)) { this.emit('change', { @@ -289,41 +292,34 @@ export class FeatureFlagsRuntimeClient extends EventEmitter } private hasEntryChanged( - a: FlagPollEntry | undefined, - b: FlagPollEntry | undefined, + a: FlagPollEntryV2 | undefined, + b: FlagPollEntryV2 | undefined, ): boolean { - if (!a || !b) { - return a !== b; - } - - if (a.enabled !== b.enabled || a.default_value !== b.default_value) { - return true; - } - - const targetsChanged = (xs: FlagTarget[], ys: FlagTarget[]): boolean => { - if (xs.length !== ys.length) return true; - const map = new Map(ys.map((t) => [t.id, t.enabled])); - return xs.some((t) => map.get(t.id) !== t.enabled); - }; - - // Type slugs cannot contain ':', so the first ':' unambiguously ends the - // type in this composite key even though target IDs may contain ':'. - const customTargetsChanged = ( - xs: FlagCustomTarget[], - ys: FlagCustomTarget[], - ): boolean => { - if (xs.length !== ys.length) return true; - const map = new Map(ys.map((t) => [`${t.type}:${t.id}`, t.enabled])); - return xs.some((t) => map.get(`${t.type}:${t.id}`) !== t.enabled); - }; - - return ( - targetsChanged(a.targets.users, b.targets.users) || - targetsChanged(a.targets.organizations, b.targets.organizations) || - customTargetsChanged( - a.targets.custom_targets ?? [], - b.targets.custom_targets ?? [], - ) - ); + if (!a || !b) return a !== b; + + // Rule order determines precedence. Condition and membership order do not. + const signature = (entry: FlagPollEntryV2): string => + JSON.stringify([ + entry.enabled, + entry.default_value, + entry.off_value, + entry.rules.map((rule) => [ + rule.id, + rule.kind, + rule.value, + rule.conditions + ?.map((condition) => + JSON.stringify([ + condition.operator, + condition.target_type, + Array.isArray(condition.values) + ? [...new Set(condition.values)].sort() + : condition.values, + ]), + ) + .sort(), + ]), + ]); + return signature(a) !== signature(b); } }