Skip to content

fix: add defensive cycle guard to prerequisite evaluation#329

Draft
tanderson-ld wants to merge 1 commit into
mainfrom
ta/SDK-2705-cycle-detection
Draft

fix: add defensive cycle guard to prerequisite evaluation#329
tanderson-ld wants to merge 1 commit into
mainfrom
ta/SDK-2705-cycle-detection

Conversation

@tanderson-ld

@tanderson-ld tanderson-ld commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds defensive cycle detection to the recursive prerequisite walk in LDCommonClient._variationInternal, bringing the Flutter client SDK's behavior into line with the LaunchDarkly server SDK evaluators.

Tracker: SDK-2705. 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.

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 Flutter client SDK.

The fix

_variationInternal now threads a lazily-allocated Set<String>? 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 zero collections. The Set<String> 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 / finally remove: no per-descent copy. ancestors.add(flagKey) before recursing, ancestors.remove(flagKey) in a finally — the invariant "the set contains exactly the current path" holds even under early exits.
  • 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 named parameter visited is added last, so all existing callers (the seven typed and untyped variation / variationDetail entry points) 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

  • packages/common_client/lib/src/ld_common_client.dart — cycle guard in _variationInternal.
  • packages/common_client/test/ld_dart_client_test.dart — 5 new tests under the existing given mock flag data with prerequisites group: 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).
  • apps/flutter_client_contract_test_service/bin/contract_test_service.dart — declares client-prereq-cycle-detection so the matching contract tests in sdk-test-harness v2.38.0+ activate for this SDK.

Verification

  • Unit tests: dart test test/ld_dart_client_test.dart → 31 tests, 0 failures. The 5 new cycle-detection tests all pass.

Changelog

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

Adds an ancestor-set (current-path) cycle guard to the recursive
prerequisite walk in _variationInternal, bringing the Flutter client
SDK's behavior into line with the LaunchDarkly server SDK evaluators,
which have detected and gracefully handled cyclic prerequisite graphs
for years.

The Set tracking ancestor keys is allocated lazily: variation calls on
prereq-less flags (the common case) allocate zero collections. Once
created, the set is shared for the rest of the walk via
add-before-recurse / remove-after-recurse, guarded by try/finally so an
exception below cannot leave a stale ancestor entry visible to a
sibling branch.

When a cycle is detected the requested flag's cached value and reason
are returned unchanged; only the recursive prerequisite event walk is
affected.

Also declares the client-prereq-cycle-detection capability so the
matching sdk-test-harness contract tests activate for this SDK.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant