fix: add defensive cycle guard to prerequisite evaluation

[tanderson-ld]

Summary

Adds defensive cycle detection to the recursive prerequisite walk in variationDetail (LaunchDarklyClient.brs), bringing the Roku client SDK's behavior into line with the LaunchDarkly server SDK evaluators.

Tracker: SDK-2710. Parent: SDK-2695. Spec change: sdk-specs#246 (CSPE 1.2.5, 1.2.5.1, 1.2.5.2). Contract-test coverage: sdk-test-harness#384 (merged in v2.38.0). Companion PRs: android-client-sdk#377, ios-client-sdk#512, js-core#1816, flutter-client-sdk#329, dotnet-core#317, cpp-sdks#582.

Background

The LaunchDarkly service validates prerequisite graphs on every mutation and rejects any change that would produce a cycle, so under normal operation the SDK does not see a cyclic prerequisite graph. Server-side SDK evaluators nonetheless carry defensive cycle detection for exceptional cases — for example, delivery of updates out of order or a persisted state loaded from disk that predates a subsequent correction. This PR extends the same defensive posture to the Roku client SDK.

The fix

variationDetail now threads a lazily-allocated associative array through the recursion carrying the flag keys on the current evaluation path. Before descending into a prerequisite, the walker checks whether that key is already on the path; if so, it skips that edge and continues with remaining prerequisites at the same level.

• Lazy allocation: variation calls on prereq-less flags (the common case) allocate no associative array. The map is created only when the walker descends into a prerequisite for the first time in a call, then reused for the rest of the walk.
• Mutable add/delete around the loop: no per-descent copy. The current flag's key is inserted before the loop and removed after, so the invariant "the map contains exactly the current path" holds across sibling descents.
• Ancestor-set (current-path) semantics — this is deliberate. A prerequisite reached via multiple non-cyclic paths (a diamond A -> [B, C], B -> [D], C -> [D]) is not a cycle; each path should emit its own prerequisite event. A global visited-set would silently drop the second event; the ancestor-set pattern correctly emits D twice. There is a unit test guarding this.

The optional trailing parameter launchDarklyParamVisited=invalid is added to variationDetail, so existing callers (boolVariation, intVariationDetail, stringVariationDetail, doubleVariationDetail, jsonVariationDetail, and the untyped variation) work unchanged.

Caller-visible behavior on a cycle

The requested flag returns its cached value and reason unchanged. A client-side prerequisite cycle is not surfaced as MALFORMED_FLAG and does not fall back to the caller-provided default value. This differs from server-side behavior — the client already holds an authoritative pre-evaluated result from the server; only the ancillary event walk is affected by the cycle.

Files changed

• rawsrc/LaunchDarklyClient.brs — cycle guard in variationDetail.
• src/test/source/tests/Test__Client.brs — 5 new unit tests: self-loop, two-cycle (evaluating each end), three-cycle, and a non-cyclic diamond that asserts the shared descendant emits an event for each path (the ancestor-set regression fence).
• src/contract-tests/components/HttpServerTask.brs — declares client-prereq-cycle-detection so the matching contract tests in sdk-test-harness v2.38.0+ activate for this SDK.

Verification

• Unit tests: 5 new TestCase__Client_CycleDetection_* cases follow the existing TestCase__Client_* pattern (seed client.private.store, call client.variation(), flush and inspect the event queue). Local execution requires a Roku device (ukor test); CI provides the actual test run.
• Contract tests: local run requires deployment to a Roku device; CI provides the run against the released harness. The new suite (client-prereq-cycle-detection) will activate as soon as CI picks up harness v2.38.0+.

Changelog

Updated prerequisite evaluation event emission to match server SDK behavior for cyclic prerequisite graphs.

---

Note

Medium Risk
Touches the prerequisite event-walk in core flag evaluation, but behavior for normal graphs is preserved and cycles only skip redundant descent; coverage is strong via new unit and contract capability tests.

Overview
Adds defensive cycle detection when the Roku client walks flag prerequisites to emit evaluation events, matching other LaunchDarkly SDKs and avoiding unbounded recursion on malformed graphs.

Core variationDetail logic moves into LaunchDarklyClientSharedPrivateFunctions and is merged into both the standard client and SceneGraph client. The recursive walk threads a current-path ancestor set (launchDarklyParamVisited): before descending into a prerequisite, if that flag is already on the path the edge is skipped; diamonds still emit events per path (not a global visited set). Public variation APIs are unchanged—they delegate to the private implementation with m.status.

Contract tests advertise capability client-prereq-cycle-detection. Five unit tests cover self-loop, 2- and 3-cycles, and a diamond graph (cached values unchanged; feature event order asserted).

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

[cursor]

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

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

^{Reviewed by Cursor Bugbot for commit 04aa234. Configure here.}

[keelerm84]

At some point, we should think about only exposing a wrapper to this method. If someone provided an unexpected type to the param visited field, this would crash.

We have the same problem now with the param strong, which is why I don't think we need to fundamentally change this. But in case you decided to start your roku dev journey, I thought I would mention it. 😄

[tanderson-ld]

Pushed 1e59528 — encapsulates the recursive evaluator so visited doesn't leak into the public API. Approach:

• New file-scope factory LaunchDarklyClientSharedPrivateFunctions() holds the impl. Each client factory (LaunchDarklyClient and LaunchDarklySG) appends it into its own m.private block via .private.append(...), so m.private.variationDetail is reachable in both variants without duplicating the source.
• Public variationDetail / variation / typed *VariationDetail methods become thin wrappers over m.private.variationDetail(m.status, ...). The status object is passed in so the initialized-check inside the impl doesn't need the full merged client.
• Strong is preserved on the public signature per your note — non-breaking. Only visited is removed from the public surface.

Verified on a Roku Express (firmware 15.2.4.3442):

• Unit tests: 93 Success, 0 Fail, no runtime errors, all five CycleDetection_* cases pass
• SG-path smoke: AppScene's m.ld.variation("hello-roku", false) returns cleanly (no Member function not found on the SG client's m.private)
• Contract tests: 784 total, 30 skipped, 754 ran, all passed