feat: Re-enable blocked FDv2 synchronizers when the SDK key changes

[keelerm84]

Summary

Stacked on #457. Retarget to v7 after #457 merges.

A permanent synchronizer failure often depends on the SDK key, for example a 401. After LDClient.SetSDKKey changes the key, that failure no longer applies. SetSDKKey now tells the data system about the change, and the FDv2 data system unblocks the synchronizers that failed permanently.

• The FDv1 fallback is now a mode of the synchronizer list, separate from the blocked state. After an FDv1 fallback, a key change keeps the data system on FDv1. The FDv2 synchronizers are still unblocked, so they keep their order if the data system later returns to FDv2.
• A running synchronizer is not interrupted. Unblocked synchronizers become available for fallback and recovery, so a client that runs on polling because streaming failed recovers to streaming later.
• When no synchronizer is available, the synchronizer loop now waits for a key change instead of exiting. On a key change, it starts the first available synchronizer, and the status leaves Off (Interrupted if the client has data, Initializing otherwise).
• The key change reaches the loop through a one-slot channel, so SetSDKKey never blocks.
• The FDv1 data system ignores the key change.
• A fallback directive with no FDv1 fallback configured still turns the data system off for good.

---

Note

Overview
SetSDKKey now notifies the data system after updating the key override, so FDv2 can retry data sources that were permanently blocked (e.g. 401 with the old key).

For FDv2, a non-blocking SDKKeyChanged signal unblocks all synchronizer slots and, if every synchronizer had been exhausted, keeps the synchronizer loop alive instead of exiting: it waits for a key change, then restarts the first available synchronizer and moves status off Off (Interrupted or Initializing). A synchronizer that is already running is not stopped; unblocked peers are only used for fallback/recovery. FDv1 fallback is modeled as list mode (onFDv1) separate from per-slot blocked state—after fallback, a key change still stays on FDv1 while clearing blocks on FDv2 slots for a possible later return to FDv2.

FDv1 implements SDKKeyChanged as a no-op. Docs and an integration test cover restart after all synchronizers fail with the original key.

^{Reviewed by Cursor Bugbot for commit eb83123. 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 2 potential issues.

^{❌ 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 eb83123. Configure here.}

[cursor]

Key change signal can be lost

High Severity

consumeSynchronizerResults consumes the one-slot sdkKeyChanged signal and only unblocks other slots, then keeps the current synchronizer. If that synchronizer later fails permanently and it was the last available FDv2 slot, the loop waits for another key-change signal that will not arrive. Official Streaming() and Polling() modes have a single FDv2 slot, so SetSDKKey during an in-flight 401 leaves the data system Off for good.

Additional Locations (1)

• internal/datasystem/fdv2_datasystem.go#L395-L412

 

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

[cursor]

SDKKeyChanged comment mixes outcomes

Low Severity

The SDKKeyChanged godoc is one five-line paragraph that mixes unblocking, FDv1-mode persistence, starting a stopped loop, and leaving a running synchronizer alone. Comments in internal/datasystem need a short paragraph per concern, with a bare // break between them.

 

^{Triggered by learned rule: Go comments: split paragraphs; no Returns() tuples}

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

[tanderson-ld]

I think it is worth a comment here that this is valid when all synchronizers depend on the SDK key, but this may become invalid if a future developer adds more types of synchronizers that can be blocked but that do not depend on the sdk key.

It may possibly be a bug waiting to happen if we unblock a synchronizer that has nothing to do with sdk keys but was blocked for good reason.

[tanderson-ld]

consider unblockAll

[tanderson-ld]

unblock didn't reset the index. Is next appropriate without an index reset? Perhaps it figures itself out and this is intentional and fine.

[tanderson-ld]

Data source status contract: this transition violates the documented API, and we should decide how deliberately we want to do that.

This block moves the data source state from Off back to Interrupted/Initializing. The public status API currently promises that can't happen:

• interfaces/data_source_status_provider.go:165 — "DataSourceStateOff indicates that the data source has been permanently shut down."
• interfaces/data_source_status_provider.go:70 — WaitFor blocks until the desired state, or "the state has become DataSourceStateOff (since that is a permanent condition)", or timeout.

The WaitFor doc has behavioral teeth: both implementations (the FDv1 sink's waitFor and FDv2's dataStatusProvider.WaitFor) return false the instant they observe Off. So with this PR, an application that calls WaitFor(Valid, …) while every synchronizer is blocked on a 401 is told "permanent condition, give up" at exactly the moment this feature says "don't give up — SetSDKKey may fix it." An app that reasonably tears down or recreates the client on Off defeats the recovery this PR builds. (Related: FDv2's WaitFor only short-circuits on Off at call time, not inside its listen loop — tolerable while Off was unreachable mid-wait, incoherent once it's a state you can pass through.)

It also makes Off ambiguous. After this PR there are two kinds: recoverable Off (synchronizer exhaustion, cures on key change) and genuinely-final Off (fallback directive with no FDv1 fallback configured — "off for good" per the description). Observers get no way to tell them apart.

Fleet precedent — I audited the Java, .NET, Python, and Ruby FDv2 implementations for exactly this question:

SDK	Off documented permanent?	Exhaustion → public status	Ever leaves Off?
Java	Yes, + waitFor give-up is test-locked	Off; orchestration thread exits; log: "will not be used again until application restart"	No
.NET	Yes; hard latch in the sink (test UpdateStatusIgnoresEverythingAfterOff)	Interrupted — a sanitizer maps source-level Off → Interrupted; public Off is reserved for truly-final cases	No
Python	FDv1: yes, latched, "final for the lifetime"; FDv2 provider: unlatched	OFF	Yes, but it reads as accidental (transient OFF leaks mid-fallback; no test asserts OFF stickiness)
Ruby	Deliberately scoped: "Unless the SDK is configured with data_system_config, no other state follows this one."	OFF	Yes — documented carve-out

No other SDK has runtime key change, so none has faced deliberate recovery out of Off. But the strict implementations (Java/.NET) and the deliberate exception (Ruby) point at the two coherent ways to resolve this:

1. Don't enter Off when recovery is possible (.NET's shape). At exhaustion, report Interrupted (if data was applied) or stay Initializing (if not) — i.e., run this block's dataApplied branch at the point of exhaustion instead of after the key change, and never report Off in between. Off stays reserved for the genuinely-final cases. Zero public-doc changes, WaitFor stays coherent (a waiter simply keeps waiting through a rotation), and it's arguably more accurate here than it is for .NET: once this PR lands, exhaustion really is "an error that it will attempt to recover from," which is Interrupted's documented meaning.
2. Keep the transition and do the contract work (Ruby's shape). Amend the DataSourceStateOff, WaitFor, DataSourceStateInitializing, and StateSince docs to scope the permanence claim, and decide deliberately what WaitFor should do when Off can recover — either answer changes documented, observable behavior.

Two more data points for the discussion: the DATASYSTEM v2 spec currently agrees with the old reading (the fallback-directive state diagram ends Off --> [*], and the 1.7.10 example log is "…will not be used again until application restart"), and the Initializing branch here is novel fleet-wide — no SDK ever re-enters INITIALIZING after leaving it; Ruby's FDv2 provider actively coerces it away, and in Java it isn't even expressible.

My leaning is option 1 — it's the smaller change, keeps every existing promise intact, and this block is already 90% of the implementation — but either is defensible if we make the choice explicitly rather than as a side effect of the key-change feature.

[tanderson-ld]

Now as I think about this more, we plan to have RETRY conformance in FDv2 at some point, which will make it such that synchronizers never block themselves due to an invalid SDK key, they will just report some sort of long retry interval (max 1 hour) up to the FDv2 data system orchestrator. So exhaustion will just not happen in the invalid SDK key case in the future.

So I think the conclusion of this thread is to pick a stopgap that is well behaved in the interim. As it is right now, the code violates the API documentation. I don't know if that is ok to do temporarily if we think this case is exceedingly rare anyways.

[tanderson-ld]

Concretely, one stopgap shape: report Interrupted at exhaustion (or stay Initializing if no data has ever applied — i.e., run the dataApplied branch from this block at the point of exhaustion), keep the key-change wakeup exactly as implemented, and never emit Off for this condition at all.

Pros

• No API-doc violation, temporary or otherwise. Off keeps its documented meaning (explicit shutdown; fallback directive with no FDv1 fallback configured), and WaitFor stays truthful: a WaitFor(Valid, …) caller keeps waiting while recovery via SetSDKKey is genuinely possible, instead of being told "permanent condition" and giving up.
• Off stays unambiguous — observers never have to guess whether a given Off is recoverable.
• The interim behavior matches the RETRY end state. Under RETRY conformance a 401'd synchronizer keeps retrying and the system shows Interrupted throughout; doing the same at exhaustion now means RETRY adoption later changes nothing an application can observe. No second migration of status semantics.
• Precedent: .NET already surfaces synchronizer exhaustion as Interrupted and reserves public Off for truly-final states (its sink hard-latches Off).
• Implementation cost is ~zero — it's the same branch this diff already contains, anchored at a different event.

Cons

• Interrupted's doc says the data source "will attempt to recover" — in the interim that's a stretch: nothing retries until a key change arrives, so the system is recovery-capable, not recovery-attempting. (This objection dissolves once RETRY lands and it genuinely is retrying.)
• We lose the explicit "the SDK has given up" signal. An operator or alert keying on Off to detect a fatally misconfigured key can't distinguish "down, will self-heal" from "down until someone rotates the key" — Interrupted-forever is less diagnosable than Off. This is the standing criticism of .NET's behavior. Mitigation: synthesize a dedicated ErrorInfo at exhaustion (e.g., "all data sources exhausted; waiting for SDK key change") the way Java/.NET do, so the diagnosis lives in the error payload rather than the state enum.
• The no-data variant shows Initializing indefinitely, which may read as understated severity for what is likely a bad key — though LastError carries the 401, and closeWhenReady still unblocks the constructor path, so nothing hangs.
• Diverges from Java (exhaustion → Off, thread exits) and from Go FDv2's current released behavior — a behavior change for anyone already observing FDv2 statuses, though the FDv2 surface is young and the exhaustion case is rare.

On balance I think the pros win: the cons are diagnosability trade-offs we can largely buy back with a good ErrorInfo, while the pro side avoids shipping a contract violation whose window is bounded by customer upgrade cadence, not by when RETRY merges.