From 7951f2b58bddee5af528bf0e4169e3248119e4cc Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Mon, 5 Oct 2026 16:28:35 -0400 Subject: [PATCH] fix(client): make a repeat BYOC initClient return the existing client initClient's BYOC branch sat above the singleton idempotency check, so a repeat `initClient(client)` swapped the stored client and re-ran telemetry setup with the new call's options, which could not take effect since OTel's globals were already held. The JSDoc said a second call returns the existing client and ignores every other option. Move the check ahead of both init paths, matching the Python SDK's _resolve_client. setupTelemetry's one-provider guard already kept a repeat call from building a second provider; this makes the client half hold too. Behaviour change: a repeat BYOC call with a different client now returns the first one. Call shutdown() first to hand the SDK a new client. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- packages/client/agents.md | 2 +- .../client/src/__tests__/lifecycle.test.ts | 51 ++++++++++++++++++- packages/client/src/__tests__/skills.test.ts | 7 ++- packages/client/src/lifecycle.ts | 24 ++++++--- 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0802b5eb..2a6a1943 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,7 +152,7 @@ The client manages a singleton connection to LaunchDarkly and the associated tel | Export | Description | |---|---| | `initClient(options?)` | Auto-discovers and initializes `@launchdarkly/node-server-sdk` (optional peer dep, loaded via dynamic import). Optional — the first AI API call triggers lazy init when `LD_SDK_KEY` is set. Accepts optional overrides for SDK key, base URIs, service name, environment, and OTLP endpoint. Returns `Promise`. On every successful path, including the already-initialized path, flushes `$ld:ai:sdk:info` for any LaunchDarkly AI packages that have not yet reported. | -| `initClient(client)` | **BYOC overload** — accepts a pre-initialized `LDClientInterface` (e.g. from `@launchdarkly/vercel-server-sdk`). Stores it directly without calling the node SDK. Flushes pending `$ld:ai:sdk:info` events. | +| `initClient(client, options?)` | **BYOC overload** — accepts a pre-initialized `LDClientInterface` (e.g. from `@launchdarkly/vercel-server-sdk`). Stores it directly without calling the node SDK. Flushes pending `$ld:ai:sdk:info` events. The optional second argument is the same options bag as the other overload. Idempotent like that overload: once a client is set, a second call returns it and ignores both a different client and the new telemetry options — `shutdown()` first to swap clients. | | `getClient()` | Returns the initialized `LDClientInterface`. Throws if initialization has not completed. | | `shutdown()` | Flushes all pending events and telemetry, then closes the client. Must be called before the process exits. Clears sdk-info reporting so a later client reports again. | | `waitForTelemetry()` | Waits for the OTel provider to be ready. Useful when spans must not be dropped at startup. | diff --git a/packages/client/agents.md b/packages/client/agents.md index 100f38f7..41e159f6 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -431,7 +431,7 @@ When `enabled` is `false`, `config` is always `null`. When `enabled` is `true` b - **Lazy initialization.** Importing the package does not initialize the LD client. The first API call that needs LaunchDarkly (`extractVariation`, graph resolution, etc.) calls `initClient()` internally, provided `LD_SDK_KEY` is set. - **Explicit initialization — Node SDK path.** `initClient(options?)` dynamically imports `@launchdarkly/node-server-sdk` at runtime (optional peer dep). If the package is not installed it throws with a clear message. -- **Explicit initialization — BYOC path.** `initClient(client, options?)` accepts any pre-initialized object that satisfies `LDClientInterface` — this is the path for Vercel, Cloudflare, or other edge runtimes whose SDK has different init semantics. No `@launchdarkly/node-server-sdk` is required. The optional second argument is the same options bag as the other overload, passed whole to telemetry setup (`otlpEndpoint`, `serviceName` and `environment` mean the same thing), and is how a BYOC caller configures `skillStore`. +- **Explicit initialization — BYOC path.** `initClient(client, options?)` accepts any pre-initialized object that satisfies `LDClientInterface` — this is the path for Vercel, Cloudflare, or other edge runtimes whose SDK has different init semantics. No `@launchdarkly/node-server-sdk` is required. The optional second argument is the same options bag as the other overload, passed whole to telemetry setup (`otlpEndpoint`, `serviceName` and `environment` mean the same thing), and is how a BYOC caller configures `skillStore`. Once a client is set, a repeat call — with the same client or a different one — returns the existing client and ignores the new telemetry options; call `shutdown()` first to swap clients. - **`skillStore` is the one option applied on every call.** Every other option is ignored once the client singleton exists. `skillStore` is applied *before* the idempotency check, so a client that initialized lazily, or without a store, can be given one later. A nullish `skillStore` never clears a configured store; only `shutdown()` does, and it clears skills state unconditionally, before its own early return, because that state can exist without a client. - **Return value.** `initClient()` returns `Promise`. Callers that don't need the instance may discard the return value — this is a non-breaking change from the previous `Promise` signature. - `getClient()` throws if `initClient()` has not resolved — any code that calls `getClient()` directly must ensure initialization has occurred. diff --git a/packages/client/src/__tests__/lifecycle.test.ts b/packages/client/src/__tests__/lifecycle.test.ts index 9013ddcc..cee3304a 100644 --- a/packages/client/src/__tests__/lifecycle.test.ts +++ b/packages/client/src/__tests__/lifecycle.test.ts @@ -385,6 +385,55 @@ describe('lifecycle', () => { await expect(initClient()).rejects.toThrow('timeout'); }); + it('does not re-run telemetry setup on a repeat BYOC call', async () => { + // The repeat call's options cannot take effect: OTel's globals are + // already held. Both the early return and setupTelemetry's one-provider + // guard keep the first call's configuration. + const { initClient } = await import('../lifecycle.js'); + const byocClient = makeMockClient(); + await initClient(byocClient, { serviceName: 'first', otlpEndpoint: 'https://first.test' }); + await initClient(byocClient, { serviceName: 'second', otlpEndpoint: 'https://second.test' }); + await initClient(byocClient); + + expect(mockOTLPTraceExporter).toHaveBeenCalledOnce(); + expect(mockOTLPTraceExporter).toHaveBeenCalledWith( + expect.objectContaining({ url: 'https://first.test/v1/traces' }), + ); + expect(mockResourceFromAttributes).toHaveBeenCalledOnce(); + expect(mockResourceFromAttributes).toHaveBeenCalledWith(expect.objectContaining({ 'service.name': 'first' })); + }); + + it('returns the first client on a repeat BYOC call rather than swapping it', async () => { + const { initClient, getClient } = await import('../lifecycle.js'); + const first = makeMockClient(); + const second = makeMockClient(); + expect(await initClient(first)).toBe(first); + expect(await initClient(second)).toBe(first); + expect(getClient()).toBe(first); + }); + + it('swaps in a new BYOC client after shutdown', async () => { + const { initClient, getClient, shutdown } = await import('../lifecycle.js'); + const first = makeMockClient(); + const second = { ...makeMockClient(), close: vi.fn().mockResolvedValue(undefined) }; + await initClient(first); + await shutdown(); + expect(await initClient(second)).toBe(second); + expect(getClient()).toBe(second); + }); + + it('flushes registered AI package information on a repeat BYOC call', async () => { + const client = makeMockClient(); + const { initClient } = await import('../lifecycle.js'); + await initClient(client); + client.track.mockClear(); + registerAiSdkPackage('@launchdarkly/ai-openai-agents', '0.1.1'); + await initClient(client); + + expect(client.track).toHaveBeenCalledOnce(); + expect(client.track.mock.calls[0][2].aiSdkName).toBe('@launchdarkly/ai-openai-agents'); + }); + it('flushes registered AI package information on the BYOC path', async () => { const client = makeMockClient(); registerAiSdkPackage('@launchdarkly/ai-server', '0.1.1'); @@ -614,7 +663,7 @@ describe('lifecycle', () => { it('builds one provider across repeat BYOC calls', async () => { // A second provider would be refused registration yet replace the handle // shutdown flushes, so the live provider would leak and never flush. - const byocClient = { ...makeMockClient(), variation: vi.fn() }; + const byocClient = makeMockClient(); const { initClient, shutdown } = await import('../lifecycle.js'); await initClient(byocClient); await initClient(byocClient); diff --git a/packages/client/src/__tests__/skills.test.ts b/packages/client/src/__tests__/skills.test.ts index 51a171b1..86f21454 100644 --- a/packages/client/src/__tests__/skills.test.ts +++ b/packages/client/src/__tests__/skills.test.ts @@ -774,10 +774,9 @@ describe('store configuration', () => { // afterwards. Both halves // are asserted on the same pair of calls; each is meaningless alone. // - // The second call takes the *options* overload, which is where TypeScript's - // early return lives: `initClient(client)` deliberately replaces the - // singleton on this side, so a second BYOC call would not prove the store - // was applied before an early return. + // The second call takes the *options* overload, so it cannot reach the + // Node SDK path without returning early: with no LD_SDK_KEY set it would + // throw. const first = new InMemorySkillStore(); first.put(rawSkill({ key: 'first' })); const second = new InMemorySkillStore(); diff --git a/packages/client/src/lifecycle.ts b/packages/client/src/lifecycle.ts index 48f0a4f0..bf9b8ae4 100644 --- a/packages/client/src/lifecycle.ts +++ b/packages/client/src/lifecycle.ts @@ -102,8 +102,9 @@ async function setupTelemetry(options: InitBaseClientOptions, sdkKey: string): P // One provider at a time. A second would be refused the global registration, // so it would receive no spans while replacing the handle `shutdownTelemetry` - // flushes, leaking the live one. A repeat BYOC call reuses the existing - // provider, as OTel itself would; a failed init has already torn its own down. + // flushes, leaking the live one. A BYOC call made while an options-path init + // is still in flight reuses that attempt's provider, as OTel itself would; a + // failed init has already torn its own down. // Checked after the imports, since nothing between here and the assignment // below awaits. if (tracerProvider) return false; @@ -324,6 +325,11 @@ function isLDClient(value: unknown): value is LDClientInterface { * configured store; `shutdown()` does that. Without a store, the Agent Skills * accessors throw. * + * That idempotency covers overload 2 too: once a client is set, passing a + * *different* pre-initialized client does not swap it, and the second call's + * telemetry options are ignored rather than re-running telemetry setup. Call + * `shutdown()` first to hand the SDK a new client. + * * The store is installed only once initialization has succeeded, so await * `initClient` before calling the skill accessors. A call that rejects installs * no store and caches neither a client nor the failure: a later call retries @@ -367,6 +373,16 @@ async function resolveClient( ): Promise { const singleton = getSingleton(); + // Ahead of *both* init paths, so "a second call returns the existing client + // and every other option is ignored" holds for BYOC as well, as it does in + // the Python SDK's `_resolve_client`. Below the BYOC branch, a repeat + // `initClient(client)` re-ran telemetry setup with the new options, which + // could not take effect, and swapped the stored client. + if (singleton.client) { + flushAiSdkInfo(singleton.client); + return singleton.client; + } + if (isLDClient(optionsOrClient)) { // Pre-initialized client path (edge / custom runtimes). // Still run telemetry setup (with the caller's options) so OTel traces work @@ -379,10 +395,6 @@ async function resolveClient( return optionsOrClient; } - if (singleton.client) { - flushAiSdkInfo(singleton.client); - return singleton.client; - } if (!singleton.initPromise) { singleton.initPromise = initBaseClient(optionsOrClient); }