[keelerm84]

feat!: Remove the FDv1 data system
fix: Enforce the 30 second minimum poll interval in the polling builders
fix: Prevent TestDataV2 from briefly restoring an older flag when a client starts

Summary

This PR merges two stacked PRs: #487 removes the FDv1 data system, and
#491 fixes a TestDataV2 startup race. The lines above keep their
changelog entries in the squash commit.

TestDataV2 replaces v1 TestData

TestDataV2 gains use_preconfigured_flag and use_preconfigured_segment,
and FlagBuilderV2 gains migration_settings, sampling_ratio,
exclude_from_summaries and a nested FlagMigrationSettingsBuilder. The v1
TestData has all of these, so TestDataV2 must cover them before v1
TestData goes.

A preconfigured flag keeps its own version, as in v1 TestData, because
tests use specific versions to check the evaluation and event output. A
flag that replaces an existing flag gets the higher of its own version
and the next version. A preconfigured flag also discards any stored
builder for its key, so a later flag call starts from a new builder. The
preconfigured methods accept a hash or a JSON string, and raise
ArgumentError for input that is not a valid flag or segment.

TestDataV2 now delivers changes to its data sources in the order it
commits them. A synchronizer gets its initial data and its registration
for later changes in one step, so a change made before it starts does
not arrive again after the initial data. An initializer no longer
registers for changes. A flag built from a FlagBuilderV2 no longer
changes when its rule builder changes later.

The FDv1 data system is removed

The client now always uses the FDv2 data system. When data_system_config
is nil, it uses DataSystem.default.

Removed config options: stream, use_ldd, poll_interval,
initial_reconnect_delay, base_uri, stream_uri, feature_store,
data_source, cache_store, payload_filter_key and data_source_update_sink.
Use the data system builders in their place: DataSystem.persistent_store
or DataSystem.daemon for a persistent store, and the polling and
streaming builders for URIs and intervals.

Removed classes and methods: the v1 TestData and its FlagBuilder,
FileData.data_source, InMemoryFeatureStore, the FDv1 data source mixin
in Interfaces::DataSource, and the Interfaces::DataSource::UpdateSink
and Interfaces::DataStore::UpdateSink interfaces. Use TestDataV2 and
FileData.data_source_v2. LaunchDarkly::Integrations now loads
TestDataV2.

The SDK key is required unless the client is offline, or events are
disabled and no data source connects to LaunchDarkly. Daemon mode is a
data system with no initializers or synchronizers. With events disabled
it reports initialized and a fork does not break it, as LDD mode did.

The diagnostic init event no longer reports customBaseURI,
customStreamURI, pollingIntervalMillis, streamingDisabled or
usingRelayDaemon. The FDv2 fields are still to be decided before 9.0.

The FDv1 fallback that FDv2 uses stays. UnexpectedResponseError moves to
Impl::DataSystem with the FDv2 polling code.

The FDv2 polling builder and the FDv1 fallback polling builder replace
a poll interval below 30 seconds with 30 seconds and log a warning. The
removed poll_interval config option had the same minimum. Without it, a
small or zero interval polls LaunchDarkly in a tight loop.

Specs, contract tests and CI

The shared spec config gives the client an FDv2 data system with no
sources. Specs that need data use TestDataV2 as a synchronizer, and
specs that need a persistent store use a hash-backed MockFeatureStore
through the data system config.

The end-to-end and environment ID specs poll the FDv2 endpoint with an
FDv2 payload. On a 401, FDv2 turns the data source off at once and does
not retry, so those specs now expect OFF, a single request and no full
start wait, in place of the FDv1 retry behavior.

The test service no longer maps the top-level streaming, polling and
persistentDataStore settings that harness v2 sends, and no longer reports
the filtering, filtering-strict and retry-conformance-fdv1 capabilities.
Harness v3 sends a dataSystem block for every test. A client with no
dataSystem block gets a data system with no sources, as LDD mode did.
CI and the Makefile run harness v3 only.

New specs cover the FDv1 specs' cases on the FDv2 code:

• The FDv2 and FDv1 fallback HTTP polling requesters: request headers,
  ETag and 304, gzip, the basis parameter, error status, response
  headers and proxy.
• PollingDataSource#stop ends a long interval wait.
• FileData v2: YAML with a Ruby object and file paths with shell
  metacharacters do not run code, reload without the listen gem, an
  initial sync failure, full flags from flagValues, and evaluation
  through LDClient.
• Every FlagBuilderV2 and FlagRuleBuilderV2 method.
• Streaming sync error paths, and the environment ID through the real
  on_connect handler in place of a copy of its logic.
• The FeatureStoreClientWrapperV2 availability poller.
• FDv2 data availability: CACHED, DEFAULTS and daemon mode.
• StatusProviderV2 listener notifications.
• The FDv2 store flag change cases: flags that a full data set adds or
  removes, a deleted prerequisite, and a removed dependency.

---

Note

Overview
Breaking: The Ruby SDK drops the legacy FDv1 data path and always runs the FDv2 data system. LDClient now uses data_system_config when set, otherwise LaunchDarkly::DataSystem.default. Legacy Config knobs (stream, use_ldd, poll_interval, base_uri, stream_uri, feature_store, data_source, cache_store, payload_filter_key, and related helpers) are removed—streaming, polling, persistent stores, daemon/LDD-style setups, and file/test sources go through DataSystem builders (DataSystem.persistent_store, DataSystem.daemon, polling/streaming builders, FileData.data_source_v2, TestDataV2).

v1 TestData, FlagBuilder, and FileData.data_source are deleted; TestDataV2 picks up preconfigured flags/segments, migration/sampling/summary builder options, ordered delivery via subscribe, and stricter validation. Integrations docs now point persistent stores at the data system instead of feature_store.

Polling builders clamp intervals below 30s to 30s with a warning (replacing the old Config#poll_interval minimum). Diagnostic init events no longer report FDv1-era fields. Large chunks of FDv1 impl (stream/poll processors, InMemoryFeatureStore, RetryState, etc.) are removed; UnexpectedResponseError lives under FDv2 polling.

CI/contract tests run harness v3 only; the test service drops legacy streaming/polling/persistence mapping and several v2-only capabilities; clients without a data system get an empty custom data system (LDD-like).

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

[kinyoklion]

Note

Agent-authored comment. This finding comes from an automated multi-agent review of this PR (general / security / adversarial / spec-conformance agents, graded against sdk-specs @ a75576c), posted with the account owner's review and approval. Each claim below was verified by executing the PR head, not just reading code. Please push back freely if any of it misreads the intent.

use_preconfigured_flag drops v1's boundary validation — silent failures where v1 was loud

The new use_preconfigured_flag (lib/ldclient-rb/integrations/test_data_v2.rb:191-214) stores the caller's hash raw. v1's use_preconfigured_item ran Impl::Model.deserialize at the API boundary, so structurally bad input raised in the caller's thread. V2 defers parsing to the client's data-system thread, which turns several plausible input mistakes into silent or confusing failures:

(a) One malformed flag drops the whole initial basis — and the client reports success.
use_preconfigured_flag({ key: 'bad', version: 1, on: true, variations: [true], prerequisites: ['oops'] }) is accepted without error. When a client fetches the basis, FeatureFlag.new raises inside decode_collection (impl/data_store/in_memory_feature_store.rb:111-123), which rescues and returns nil, so set_basis fails for the entire payload — a well-formed sibling flag in the same payload is lost too. Meanwhile impl/data_system/fdv2.rb:290-297 discards @store.apply's return value, and with a defined selector the data system sets status VALID and releases readiness. Net effect: the constructor returns "success", data source status is VALID, every variation serves the default, and the only signal is one error log line. Proof spec asserting the desired behavior fails today with the well-formed sibling serving "DEFAULT" instead of its configured value.

(b) Exported flag JSON silently mis-evaluates.
transform_keys(&:to_sym) is top-level only, so use_preconfigured_flag(JSON.parse(flag_json)) — the most natural way to obtain a "complete flag data model" — keeps string keys at depth. Verified end-to-end through a real client: fallthrough.variation decodes to nil → evaluation returns the default with ERROR(MALFORMED_FLAG); rules decode with zero clauses and match every context; migration.checkRatio is silently dropped. (v1 was loud for this input shape — nil key → NoMethodError at the call site — and additionally accepted JSON strings, which V2 rejects with NoMethodError on as_json.)

(c) Missing/odd input surfaces as a bare NoMethodError.
No :key → undefined method `to_sym' for nil (raised inside the write lock — the lock itself unwinds correctly, but nothing says what was missing); nil → undefined method `as_json' for nil; a non-string/symbol :key fails the same way.

(d) version is unvalidated.
version: "100" is stored verbatim, and the next use_preconfigured_flag on the same key raises TypeError: no implicit conversion of Integer into String inside the write lock.

Suggestion: validate and deep-normalize at the boundary — e.g. run Impl::Model.deserialize over a deep-symbolized copy of the input, raising ArgumentError naming the offending field. (A deep-dup in the same pass would also fix the shallow-dup aliasing: today, mutating the caller's nested structures after the call changes the stored test data with no version bump and no notification.) Worth a separate ticket beyond this PR: fdv2.rb ignoring the apply result is what makes a failed basis look like VALID + ready — that predates this change, but this API exposes it.

We have four failing proof specs (rspec, ruby 3.4) that encode the desired behavior for (a)–(c) and flip green with a boundary-validation fix — happy to push them to this branch or a follow-up if useful.

[kinyoklion]

Note

Agent-authored comment. From the same automated multi-agent review as the boundary-validation comment above, posted with the account owner's review and approval. Verified by executing the PR head.

Replacement version clamp discards a higher provided version (test_data_v2.rb:201)

updated_flag[:version] = old_flag.nil? ? (updated_flag[:version] || 1) : old_flag[:version] + 1

The replacement branch unconditionally stores old + 1, ignoring the caller's provided version:

td.use_preconfigured_flag({ key: "f", version: 5,   ... })  # new flag → stores 5   ✓
td.use_preconfigured_flag({ key: "f", version: 100, ... })  # replace → stores 6    ✗ (not 100)

This undercuts the PR's own rationale for keeping versions — "tests use specific versions to check the evaluation and event output" — exactly when a test replaces a flag with an explicit higher version: versions surface in all_flags_state and feature events, and nothing tells the author their version was rewritten.

Spec angle. TDS 2.6.3 mandates the bump only conditionally: "MUST automatically increment the version number of the flag if the provided flag's version is less than or equal to the current version." The ≤ case exists because version-checking stores reject non-monotonic writes, and the PR handles it correctly (test_data_v2_spec.rb:69-76 pins 100 then 5 → 101). The spec is silent on the > case, where the natural reading is that an already-higher version needs no intervention. So the clamp doesn't violate the letter of the MUST, but extends it past the spec in a way that destroys caller intent. (The method's doc comment does document the clamp — "a flag that replaces an existing flag gets a version one more than the version of the existing flag" — so if this is the intended semantic, the alternative resolution is to keep it and file the spec-ambiguity note below.)

Cross-SDK divergence. Go's TestDataV2 implements the conditional reading (testhelpers/ldtestdatav2/test_data_source.go:120-133): if f.Version < version { f.Version = version } — replace at current=5 with provided=100 keeps 100. Same spec text, opposite behavior; portable test suites would observe different event output across SDKs.

Same line, minor edge: (updated_flag[:version] || 1) keeps a provided version: 0 for a new flag (0 || 1 → 0 in Ruby); Go normalizes to 1.

Suggested fix (keep-higher with a monotonic floor — Go's semantics and the spec's intent):

floor = old_flag.nil? ? 0 : old_flag[:version]
provided = updated_flag[:version] || 0
updated_flag[:version] = [provided, floor + 1].max  # then map 0 → 1 for a versionless new flag

We have a failing proof spec for both cases (plus one pinning the mandated ≤ case, which passes today and must keep passing) — happy to push them to this branch or a follow-up. Fair disclosure: v1 TestData has the identical unconditional clamp (test_data.rb:176-182), so this is v1 parity faithfully ported; the observation is that the new v2 API had the spec's conditional and the Go reference available, and the > case is currently untested.

[kinyoklion]

Note

Agent-authored comment. Same automated multi-agent review as the two comments above, posted with the account owner's review and approval. Both halves of this were proven deterministically by executing the PR head (a blocking fake instance for the delivery ordering, and a direct store-level test for the missing version check).

Out-of-order upsert delivery + no stale-version rejection in the FDv2 store = clients can stick on a stale flag

Two cooperating layers:

Layer 1 — delivery is not serialized with commit. Both update (test_data_v2.rb:110-132) and the new use_preconfigured_flag (:191-214) compute the new flag under the write lock, copy @instances, then call instance.upsert_flag after releasing the lock. Two concurrent calls therefore serialize their @current_flags updates but not their propagation. Deterministic interleaving (thread A commits v2 and blocks inside a fake instance's upsert_flag; thread B commits and delivers v3; then A is released): the instance receives [1, 3, 2] — the newest write arrives before the stale one.

Layer 2 — the store accepts the regression. InMemoryFeatureStoreV2#apply_delta (impl/data_store/in_memory_feature_store.rb:86-104) does items_of_kind[key] = item with no version check. Feeding a v3 basis and then a v2 delta leaves store.get returning version 2 with the stale value. Contrast with the FDv1 store (in_memory_store.rb:72), which rejects old[:version] >= item[:version] — so the identical race in v1 TestData was self-healing per key, and in FDv2 it's sticky: the client holds the stale flag indefinitely (until the next update for that key), while make_init_data and late-joining clients see the fresh one. Flag-change listeners do still fire for the regressive write, so the symptom is a spurious notification carrying regressed data rather than a missed one.

Concrete trigger in a test suite: a "reset these flags" helper racing a test body that updates the same key — the test then reads back the wrong variation with no error anywhere.

Labeling, for fairness: the compute-inside/deliver-outside pattern is pre-existing in update (and use_preconfigured_segment); this PR adds use_preconfigured_flag as a second caller of the racy path. The missing store version-guard is pre-existing FDv2 infrastructure, untouched by this diff — but it's what makes the race observable, and it also undercuts TDS 2.6.3's stated rationale in the spec ("prevents silent rejection by the data store's Upsert method" — the FDv2 store performs no such rejection).

Suggested direction: cheapest correct fix for this PR is propagating upserts while holding the lock (the critical section is short — an enqueue per instance). The more robust fix, at FDv2-infra scope rather than this PR, is adding stale-version rejection to InMemoryFeatureStoreV2#apply_delta to match the FDv1 store's semantics; that also future-proofs every other synchronizer racing a TestData-style source.

Proof specs for both layers fail today ([1, 3, 2] delivery order; store keeps v2 after a v3 basis) and flip green with either fix — happy to push them to this branch or the follow-up.

[kinyoklion]

Feedback in comments

[keelerm84]

Fixed in f4887c4 and aadae60. use_preconfigured_flag and use_preconfigured_segment now deep-convert the input to symbol keys, store a copy, and raise ArgumentError at the call site for nil or another type, a missing or empty key, a non-integer version, malformed JSON, a value JSON cannot hold (such as a Time, NaN or Infinity), or a data model error. A Set becomes an array. They also accept a JSON string again, as v1 did. Specs cover each case, plus nested string keys from JSON.parse and later mutation of the caller's hash.

Agreed that fdv2.rb ignoring the @store.apply result is the deeper problem, since any synchronizer can hit it. That predates this PR, so I'll handle it separately.

[keelerm84]

Fixed in f4887c4, with Go's semantics: the stored version is max(provided, current + 1), and a new flag with version 0 or no version gets 1. Specs cover replace-with-higher (5 then 100 keeps 100), replace-with-lower (100 then 5 gives 101), and version 0.

[keelerm84]

Layer 1 is fixed in f4887c4. update, use_preconfigured_flag and use_preconfigured_segment now hold a mutex across the commit and the delivery to the instances. The existing read-write lock is not reentrant, and an instance's upsert_flag calls back into get_version, so delivering under that lock would deadlock; the separate mutex avoids that. A spec blocks the first delivery and checks that the second change waits and arrives second.

Layer 2, the missing stale-version rejection in InMemoryFeatureStoreV2#apply_delta, is FDv2 store behavior outside this PR. It goes in its own PR, along with the duplicate flag change events the same missing check causes.

[keelerm84]

Correction on layer 2: there won't be a separate PR. FDv2 guarantees upstream that payloads arrive in order, so apply_delta does not need a version check. Go has none either. FDv1 had no such guarantee, which is why its stores check versions.

TestDataV2 is a local source, so it has to keep that order itself. This PR fixes the order of changes between two updates. #491 fixes the other case, where a change made before a synchronizer starts was sent again after the initial data.