feat: Synchronous re-anchor of the upstream SDK client

[keelerm84]

Summary

Final slice of #720 (the re-anchor mechanism, SDK-2542): the synchronous upstream SDK-client swap itself. When sdkKey.value changes, relay swaps its single upstream client to the new anchor synchronously, preserving downstream connections and rolling back cleanly on failure.

Stacked PR. Based on mk/sdk-2542/reanchor-core-base (= feat/concurrent-keys + the three prior slices #736 / #737 / #738), so this diff shows only the core. Once those three merge, this will be retargeted to feat/concurrent-keys and the diff is unchanged.

What it does

• reconcileCredentials runs add -> reanchor -> remove under a dedicated reconcileMu (also held by the cleanup ticker's triggerCredentialChanges, so a credential expiry can't interleave a build).
• reanchor builds the new anchor's client (Case A) or reuses an existing/offline one (Case B), waits for Initialized, CommitAnchors, then repoints event + metrics forwarding. Store handover (from feat: Hand over the data store to the new client on re-anchor #736) means the new client serves the populated store with no empty-store window.
• Rollback: on init failure it does not commit, leaves the previous anchor authoritative and still serving, and does not poison initErr (which would 401 a healthy env). RevertAnchorChange realigns the accepted set.
• Completes the rotator API: Reconcile strips a brand-new anchor from additions so the async startSDKClient can't race the synchronous build; MobilePrimaryRepoint handles a primary-mobile switch to an already-accepted key.
• Inverts PoC H6/H7 to assert the fix (no nil swap-window; rollback keeps the old anchor). Big-segment re-wire is a documented no-op stub for follow-up (T2.d).

Reviewer notes (known trade-offs)

• Serial RAC blocking: the synchronous build blocks the single RAC handler goroutine up to InitTimeout (~10s) for other envs during a re-anchor. The data plane is unaffected (c.mu is released around the build). This is a deliberate v1 trade-off; a possible follow-up is bounded parallelism or a shorter re-anchor init timeout.
• Highest-risk area: the Reconcile strip-from-additions <-> reanchor coupling and the rollback path (RevertAnchorChange + the env-side undo in reconcileCredentials).

Split sequence

PR 4 of 4: store handover (#736) -> relayenv refactors (#737) -> credential deferred-flip (#738) -> synchronous re-anchor core (this PR).

---

Note

High Risk
Changes core credential reconciliation and upstream client lifecycle for every anchor rotation; rollback and stale-build races are subtle, and synchronous builds can block the RAC handler for up to init timeout.

Overview
When sdkKey.value changes, relay now swaps the upstream SDK client synchronously instead of flipping the anchor early and building async.

reconcileCredentials runs add → re-anchor → remove under reconcileMu (shared with the cleanup ticker). reanchor either builds and waits for Initialized() (Case A) or reuses an existing client / skips build offline (Case B), then CommitAnchor, ReplaceCredential on events/metrics, and clears initErr on success. Failed Case A builds roll back via RevertAnchorChange, env-side mapping teardown for brand-new anchors, and skipping expiration of the still-serving previous anchor.

Rotator: signals AnchorChange (with NewAnchorPreviouslyAccepted), strips brand-new anchors from additions to avoid racing startSDKClient, adds MobilePrimaryRepoint, RevertAnchorChange, and StepTime never expires the current anchor (even with a stale grace expiry after rollback).

Concurrency: anchorClientGen discards late startSDKClient builds so they cannot clobber the committed anchor or poison initErr. Design doc updated for Case A/B and reconcile coupling.

Extensive regression tests cover happy path, rollback, ticker serialization, and inverted PoC H6/H7. Big-segment re-wire remains a documented stub.

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

[kinyoklion]

This comment feels much more complex than what it is trying to convey.

[kinyoklion]

The comments overall may need a second pass. I find them reasonably inscrutable.

[keelerm84]

Good call. I cut that StepTime comment down to two lines ("Never expire the current anchor, even if a stale expiry was left on its entry (a rolled-back re-anchor can). Same guard as DeprecatedCredentials.") and did a readability pass on the densest re-anchor comments while I was in there — the reconcileCredentials header, the rollback block, and the two-signals block in reanchor. Left this thread open for you to judge whether they read better now; happy to keep trimming any that are still murky.

[kinyoklion]

Bug: a late-completing initial startSDKClient for the old anchor clobbers initErr after a successful re-anchor, 401ing a healthy env.

If the initial startSDKClient(A) is still blocked in the factory when a rotation synchronously re-anchors to a healthy B, commitReanchor sets initErr = nil — but when A's build finally returns ld.ErrInitializationFailed (the natural outcome for a key being rotated out), startSDKClient writes c.initErr = err unconditionally. The droppedInactive/sdkKeyIsActive logic only guards the client install, not the initErr write. The middleware then 401s every request while GetClient() returns the healthy B client, and nothing re-clears initErr except another successful re-anchor.

The existing suite never hits this because every re-anchor test waits for the initial client before rotating.

Recommended fix: in startSDKClient's post-build path, gate the initErr write (and the install/rebuildEvaluator) on the key still being the current anchor — the same "leave initErr untouched" contract buildNewAnchorClient already documents for the reverse direction.

Repro (fails on this branch at the GetInitError() assertions; verified with go test -race):

// TestReanchor_InitialClientMustNotClobberInitErrAfterReanchor: the initial startSDKClient for the
// OLD anchor A is still in flight when a synchronous re-anchor to healthy B commits (initErr=nil).
// A's build then fails, and startSDKClient's unconditional `c.initErr = err` clobbers the nil —
// 401ing a healthy env at the middleware even though B is the anchor and serving.
func TestReanchor_InitialClientMustNotClobberInitErrAfterReanchor(t *testing.T) {
	envConfig := st.EnvMain.Config

	mockLog := ldlogtest.NewMockLog()
	defer mockLog.DumpIfTestFailed(t)

	clientCh := make(chan *testclient.FakeLDClient, 10)
	healthy := testclient.FakeLDClientFactoryWithChannel(true, clientCh)

	gate := make(chan struct{})       // released to let A's initial build complete
	entered := make(chan struct{}, 1) // signals A's initial build has reached the factory

	factory := func(sdkKey config.SDKKey, cfg ld.Config, timeout time.Duration) (sdks.LDClientContext, error) {
		if sdkKey == envConfig.SDKKey {
			// The ORIGINAL anchor A: block until released, then fail the way a 401 does.
			entered <- struct{}{}
			<-gate
			return nil, ld.ErrInitializationFailed
		}
		// The new anchor B builds fine.
		return healthy(sdkKey, cfg, timeout)
	}

	readyCh := make(chan EnvContext, 1)
	env := makeBasicEnv(t, envConfig, factory, mockLog.Loggers, readyCh)
	defer env.Close()

	envImpl := env.(*envContextImpl)

	// The initial startSDKClient(A) is now blocked in the factory; the anchor is still A.
	<-entered

	// Re-anchor A -> B (B is brand new; A is grace-demoted with a +1h expiry). B builds, commits,
	// and clears initErr.
	now := time.Unix(2000, 0)
	expiry := now.Add(time.Hour)
	set, err := credential.NewAcceptedSetBuilder().
		WithAnchor(credential.SDKKeyParams{Value: reanchorSyncTestKey2}).
		WithSDKKey(credential.SDKKeyParams{Value: envConfig.SDKKey, Expiry: &expiry}).
		WithPrimaryMobileKey(credential.MobileKeyParams{Value: envConfig.MobileKey}).
		WithEnvironmentID(envConfig.EnvID).
		Build()
	require.NoError(t, err)
	envImpl.reconcileCredentials(set, now)

	// Post-commit sanity: B is the healthy anchor and initErr has been cleared.
	require.Equal(t, reanchorSyncTestKey2, envImpl.keyRotator.AnchorKey())
	require.NoError(t, env.GetInitError(), "sanity: after a successful re-anchor initErr is nil")

	// Release the OLD anchor's build; it returns ld.ErrInitializationFailed. startSDKClient(A)
	// sends env on readyCh once it finishes (after writing initErr).
	close(gate)
	<-readyCh

	// DESIRED: a healthy re-anchored env must NOT be marked failed by the old anchor's late failure.
	assert.NoError(t, env.GetInitError(),
		"initErr of a healthy re-anchored env must not be clobbered by the old anchor's late build failure")
	assert.NotEqual(t, ld.ErrInitializationFailed, env.GetInitError(),
		"the request middleware would 401 the whole env if GetInitError() is ErrInitializationFailed")

	// The env genuinely IS healthy: B is the anchor and its client serves.
	assert.NotNil(t, env.GetClient(), "the new anchor's client serves")
}

[keelerm84]

Fixed in 00d53a3. startSDKClient now gates both the client install and the initErr write on sdkKey still being the current anchor — a build whose key is no longer the anchor (a re-anchor moved it while we were building) is treated as stale: the client is closed and initErr is left untouched, mirroring buildNewAnchorClient's contract for the re-anchor direction. That also subsumed the old revoked-key guard, so sdkKeyIsActive is gone. Your repro is in as a permanent regression (TestReanchor_InitialClientMustNotClobberInitErrAfterReanchor) — verified it fails without the gate and passes with it, under -race. Thanks for the catch.

[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 00d53a3. Configure here.}

[keelerm84]

Small correction to my note above: Bugbot flagged that gating the client install on the anchor would drop a grace-demoted key's late-built client. Since your actual bug is only the initErr clobber (your repro has the build fail, so nothing installs anyway), I kept the install as-is and gated only the initErr write on the key still being the anchor. Grace keys keep their client; added a regression for that too. Net for your bug is unchanged. (e81b0a4)

[keelerm84]

Update on the initErr/re-anchor thread: the client-identity gate I described earlier turned out insufficient (a review pass showed a failed SDK build returns a non-nil, uninitialized client, so on an A→B→A re-anchor the stale initial build would be installed — closing the healthy re-anchored client — before any initErr gate ran).

Reworked to the root cause: startSDKClient now captures an anchorClientGen counter at launch (bumped in commitReanchor), and on completion discards a superseded build — closes it, doesn't install it, leaves initErr untouched — rather than clobbering the current anchor client. A genuine failure of the current anchor (no re-anchor since launch) still records initErr, so a broken env still 401s. Regression tests use realistic non-nil uninitialized failing clients and cover the single-reanchor, discarded-successful, and A→B→A cases (all verified failing without the guard). d771b85.