Repository navigation
fix(skills): default FDv2SkillStore endpoints from LD_BASE_URI and LD_STREAM_URI - #207
Merged
Merged
Conversation
…_STREAM_URI init_client() reads LD_BASE_URI / LD_STREAM_URI for the base SDK, but FDv2SkillStore always defaulted to the production hosts, so a process pointed at a non-production instance sent skill delivery to production with a non-production key. With neither URI option given, the constructor now reads both variables in their place, once. A lone LD_BASE_URI moves both endpoints, either option turns the environment off for both, blank counts as unset, and a refused value is reported under the variable's name. Spec: TESTING.md §3.25 and A.12 (launchdarkly/ai-sdks-monorepo#59). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
XieX
marked this pull request as ready for review
October 23, 2026 17:57
jeffdupont
approved these changes
Oct 23, 2026
jeffdupont
left a comment
Contributor
There was a problem hiding this comment.
Approved after the GA 1.0 PR audit. Lands together with js-ai-sdk#134. Nit for a follow-up: strip whitespace on the env values at lifecycle.py:293-294.
…EVENTS_URI in init_client init_client read the three URI variables with os.environ.get, so a value with stray whitespace reached ldclient.Config as-is, and a whitespace-only value was passed as a URI rather than treated as unset. They now go through _env, as LD_SDK_KEY and the other variables already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Applied the nit in 01c1fee: 🤖 Addressed by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
init_client()readsLD_BASE_URI/LD_STREAM_URIfor the base SDK, butFDv2SkillStorealways defaulted to the production hosts. A process pointed at a staging or private instance by its environment resolved flags there, then sent skill delivery to production with a non-production key. That shows up as a fatal 401 instore.failed, not as a configuration error.With neither
base_urinorstream_urigiven, the constructor now reads both variables in their place, once:LD_BASE_URImoves both endpoints, under the store's existing rule for a lonebase_uri. This differs on purpose frominit_client(), where flag streaming stays on its default host.httpsvalue is reported under the variable's name.DEFAULT_BASE_URI/DEFAULT_STREAM_URIkeep the production values as the fallback.The client README and agents.md are updated.
From review:
init_client()now strips whitespace fromLD_BASE_URI,LD_STREAM_URIandLD_EVENTS_URI, through the existing_envhelper, so a whitespace-only value counts as unset there too. That is a core change, so the override below lists it underclient.Spec: TESTING.md §3.25 and A.12, in launchdarkly/ai-sdks-monorepo#59. The TypeScript side is launchdarkly/js-ai-sdk#134.
Tests
TestEndpointsFromTheEnvironmentadds a test for each bullet in §3.25.test_skills_fdv2.pynow clears both variables with an autouse fixture, so a developer shell can't move the default-host assertions.TestTransportLayering's stdlib allowlist addsos.TestInitClientSDKKeyPathadds whitespace and blank-value tests for the threeinit_clientURI variables.Checks:
uv run pytest packages/client/tests: 1648 passed.make lint,make format-checkandmake typecheckare clean.BEGIN_COMMIT_OVERRIDE
fix(skills): default FDv2SkillStore endpoints from LD_BASE_URI and LD_STREAM_URI
fix(client): strip whitespace from LD_BASE_URI, LD_STREAM_URI and LD_EVENTS_URI in init_client
END_COMMIT_OVERRIDE
🤖 Generated with Claude Code
Note
Overview
FDv2SkillStorenow follows the same endpoint env vars asinit_client()when you omit bothbase_uriandstream_uri. Staging/private setups that only setLD_BASE_URI/LD_STREAM_URIno longer resolve flags against one host while skill FDv2 traffic still hits production (often a fatal 401 on a non-prod key).The constructor reads
LD_BASE_URIandLD_STREAM_URIonce via_endpoint_env()(whitespace-only counts as unset). A loneLD_BASE_URIdrives poll and stream, matching a lonebase_urioption—notinit_client(), where streaming stays on the default host unlessLD_STREAM_URIis set. Passing either URI option turns env off for both so hosts are never mixed. HTTPS validation errors name the env var when the value came from the environment.Docs in
README.mdandagents.mddescribe this. Tests addclear_endpoint_envandTestEndpointsFromTheEnvironment.Reviewed by Cursor Bugbot for commit 9deab92. Bugbot is set up for automated code reviews on this repo. Configure here.