fix: add defensive cycle guard to prerequisite evaluation

.github/workflows/sdk-client.yml

@@ -32,4 +32,4 @@ jobs:
           target_file: 'packages/shared/sdk-client/dist/esm/index.mjs'
           package_name: '@launchdarkly/js-client-sdk-common'
           pr_number: ${{ github.event.number }}
-          size_limit: 39000
+          size_limit: 39300

packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts

@@ -42,6 +42,7 @@ export default class TestHarnessWebSocket {
             'anonymous-redaction',
             'strongly-typed',
             'client-prereq-events',
+            'client-prereq-cycle-detection',
             'client-per-context-summaries',
             'track-hooks',
           ];

packages/shared/sdk-client/__tests__/LDClientImpl.events.test.ts

@@ -240,4 +240,108 @@ describe('sdk-client object', () => {
       }),
     );
   });
+
+  describe('prerequisite cycles', () => {
+    // Cycle-detection tests exercise the ancestor-set cycle guard added to _variationInternal.
+    // Prior to that guard, any of these flag configurations would cause unbounded recursion during
+    // a variation() call. Each test constructs a cyclic prereq graph, drives it through identify(),
+    // evaluates one flag on the cycle, and asserts (a) the returned value equals the cached value
+    // and (b) the emitted feature events match exactly one recording per cycle-safe descent.
+    //
+    // The outer `beforeEach` captures a JSON snapshot of `defaultPutResponse` inside the stream mock,
+    // so mutations here would not otherwise propagate to the SDK's flag store. `installCycleFlags`
+    // both mutates `defaultPutResponse` and rebuilds the mock so the SDK loads the updated set on
+    // the next identify().
+    const makeFlag = (
+      prerequisites?: string[],
+    ): {
+      value: boolean;
+      variation: number;
+      version: number;
+      reason: { kind: string };
+      trackEvents: boolean;
+      prerequisites?: string[];
+    } => ({
+      value: true,
+      variation: 0,
+      version: 1,
+      reason: { kind: 'FALLTHROUGH' },
+      trackEvents: true,
+      ...(prerequisites ? { prerequisites } : {}),
+    });
+    const installCycleFlags = (flags: Flags) => {
+      Object.assign(defaultPutResponse, flags);
+      mockPlatform.requests.createEventSource.mockImplementation(
+        (streamUri: string = '', options: any = {}) => {
+          mockEventSource = new MockEventSource(streamUri, options);
+          mockEventSource.simulateEvents('put', [{ data: JSON.stringify(defaultPutResponse) }]);
+          return mockEventSource;
+        },
+      );
+    };
+    const featureEventKeysAfterIdentify = () =>
+      mockedSendEvent.mock.calls
+        .map((call) => call[0])
+        .filter((event) => event.kind === 'feature')
+        .map((event) => event.key);
+
+    it('skips a self-loop prerequisite and returns the cached value', async () => {
+      installCycleFlags({ flagA: makeFlag(['flagA']) } as unknown as Flags);
+      await ldc.identify({ kind: 'user', key: 'bob' });
+      expect(ldc.variation('flagA', false)).toBe(true);
+      // Only flagA emits a feature event; the self-prereq is cycle-skipped.
+      expect(featureEventKeysAfterIdentify()).toEqual(['flagA']);
+    });
+
+    it('handles a two-cycle evaluating A', async () => {
+      installCycleFlags({
+        flagA: makeFlag(['flagB']),
+        flagB: makeFlag(['flagA']),
+      } as unknown as Flags);
+      await ldc.identify({ kind: 'user', key: 'bob' });
+      expect(ldc.variation('flagA', false)).toBe(true);
+      // A -> B -> [A skipped]. Events (deepest-first): B (as prereq of A), then A.
+      expect(featureEventKeysAfterIdentify()).toEqual(['flagB', 'flagA']);
+    });
+
+    it('handles a two-cycle evaluating B', async () => {
+      installCycleFlags({
+        flagA: makeFlag(['flagB']),
+        flagB: makeFlag(['flagA']),
+      } as unknown as Flags);
+      await ldc.identify({ kind: 'user', key: 'bob' });
+      expect(ldc.variation('flagB', false)).toBe(true);
+      // Symmetric: same graph, entry from B. Events: A (as prereq of B), then B.
+      expect(featureEventKeysAfterIdentify()).toEqual(['flagA', 'flagB']);
+    });
+
+    it('handles a three-cycle', async () => {
+      installCycleFlags({
+        flagA: makeFlag(['flagB']),
+        flagB: makeFlag(['flagC']),
+        flagC: makeFlag(['flagA']),
+      } as unknown as Flags);
+      await ldc.identify({ kind: 'user', key: 'bob' });
+      expect(ldc.variation('flagA', false)).toBe(true);
+      // A -> B -> C -> [A skipped]. Events emitted deepest-first: C, B, A.
+      expect(featureEventKeysAfterIdentify()).toEqual(['flagC', 'flagB', 'flagA']);
+    });
+
+    it('emits the shared descendant once per path in a non-cyclic diamond', async () => {
+      // Diamond: A -> [B, C], B -> [D], C -> [D]. Not a cycle. Ancestor-set (current-path) semantics
+      // must let D be reached on each of the two independent paths -- so D emits twice. A naive
+      // "visited across the whole walk" implementation would drop the second D event; this case
+      // guards against that regression.
+      installCycleFlags({
+        flagA: makeFlag(['flagB', 'flagC']),
+        flagB: makeFlag(['flagD']),
+        flagC: makeFlag(['flagD']),
+        flagD: makeFlag(),
+      } as unknown as Flags);
+      await ldc.identify({ kind: 'user', key: 'bob' });
+      expect(ldc.variation('flagA', false)).toBe(true);
+      // Events (deepest-first per path): D (via B), B, D (via C), C, A. D appears twice.
+      expect(featureEventKeysAfterIdentify()).toEqual(['flagD', 'flagB', 'flagD', 'flagC', 'flagA']);
+    });
+  });
 });

packages/shared/sdk-client/src/LDClientImpl.ts

@@ -618,6 +618,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
     defaultValue: any,
     eventFactory: EventFactory,
     typeChecker?: (value: any) => [boolean, string],
+    visited?: Set<string>,
   ): LDEvaluationDetail {
     // We are letting evaulations happen without a context. The main case for this
     // is when cached data is loaded, but the client is not fully initialized. In this
@@ -679,9 +680,27 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
       successDetail.value = defaultValue;
     }
 
-    prerequisites?.forEach((prereqKey) => {
-      this._variationInternal(prereqKey, undefined, this._eventFactoryDefault);
-    });
+    if (prerequisites && prerequisites.length > 0) {
+      // Recurse on prerequisites to emulate prereq evaluations occurring with desirable side effects
+      // such as events for prereqs.
+      //
+      // `visited` tracks the chain of prerequisite dependencies from the top-level evaluation to
+      // (but not including) the current flag.
+      const ancestors = visited ?? new Set<string>();
+      ancestors.add(flagKey);
+      try {
+        for (const prereqKey of prerequisites) {
+          if (ancestors.has(prereqKey)) {
+            // Cyclic edge: skip descent, continue with remaining prerequisites at this level.
+            // The requested flag's value and reason are unaffected.
+            continue;
+          }
+          this._variationInternal(prereqKey, undefined, this._eventFactoryDefault, undefined, ancestors);
+        }
+      } finally {
+        ancestors.delete(flagKey);
+      }
+    }
     if (hasContext) {
       this._eventProcessor?.sendEvent(
         eventFactory.evalEventClient(