epic-1/story-2: Support sdkKey and Direct-Config Initialization - #25
Conversation
Beads: ai-driven-product-dev-vs7n Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SDKConfig/TransportConfig (dataclass, no pydantic), typed error hierarchy (ConvertSDKError/ConfigError/InvalidConfigError/ConfigLoadError/TransportError), NFR8 TLS-only enforced at config time, qs-08 inline URL-redaction shim, httpx>=0.28,<1.0 runtime dep (qs-09 F-060). Extended public surface preserves frozen Story 1.1 boundary. Beads: ai-driven-product-dev-vs7n Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-9i6g Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n (GREEN) config_loader (validators/normalizer/loader) validates raw payloads, deep-copies boundary data, and builds a frozen ConfigSnapshot with precomputed experience/feature key+id indexes. Snapshots are immutable and never alias the caller's input dict. Beads: ai-driven-product-dev-9i6g Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-2rxo Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…GREEN)
Transport Protocol port + httpx adapter (long-lived client, trust_env=False,
TLS-only enforced upstream, optional bearer auth). JS-parity config route
/config/{sdkKey} with conditional environment= and _conv_low_cache=1 params.
Core initializes from direct config (no network) or sdkKey (fetch via
transport), exposes authoritative is_ready and immutable current_config.
Failures surface as typed ConfigLoadError/InvalidConfigError with redacted
endpoints.
Beads: ai-driven-product-dev-2rxo
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…8 NFR23)
The inline redaction shim now masks the /config/{sdkKey} path segment
(first4***last4, short keys fully ***) in addition to stripping the query
string, so full SDK keys never appear in error messages. Added redaction unit
tests; corrected the transport 5xx test to assert masking.
Beads: ai-driven-product-dev-scbi
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements Story 1.2 of the Convert Python SDK, introducing initialization configuration types (SDKConfig, TransportConfig), an httpx-backed transport adapter, a validation and normalization pipeline, an immutable ConfigSnapshot domain model, and a typed exception hierarchy. The reviewer provided high-quality feedback to improve robustness, suggesting that the transport adapter track ownership of the httpx.Client to avoid closing externally managed clients, that ConfigSnapshot recursively freeze nested structures for deep immutability, that validate_config use collections.abc.Mapping instead of strict dict checks, and that SDKConfig explicitly verify the type of sdk_key to prevent masking invalid types.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def __init__(self, transport_config: TransportConfig, *, client: Optional[httpx.Client] = None) -> None: | ||
| self._config = transport_config | ||
| headers: Dict[str, str] = dict(transport_config.headers) | ||
| if transport_config.auth_secret: | ||
| headers["Authorization"] = f"Bearer {transport_config.auth_secret}" | ||
| # A single long-lived client is preferred over one-off request helpers. | ||
| # trust_env=False makes config fetches deterministic: the SDK does not | ||
| # implicitly inherit ambient proxy/SSL environment variables (e.g. a | ||
| # host SOCKS proxy). Explicit transport configuration remains available | ||
| # for proxied deployments. | ||
| self._client = client or httpx.Client( | ||
| base_url=transport_config.base_url, | ||
| timeout=transport_config.timeout, | ||
| verify=transport_config.verify_tls, | ||
| headers=headers, | ||
| trust_env=False, | ||
| ) |
There was a problem hiding this comment.
If an externally managed httpx.Client is passed to the transport adapter, the adapter should not take ownership of its lifecycle. Currently, calling close() on the adapter will close the injected client, which can cause unexpected RuntimeError or ClosedException in other parts of the caller's application if they reuse that client. We should track whether the client was created internally or injected.
| def __init__(self, transport_config: TransportConfig, *, client: Optional[httpx.Client] = None) -> None: | |
| self._config = transport_config | |
| headers: Dict[str, str] = dict(transport_config.headers) | |
| if transport_config.auth_secret: | |
| headers["Authorization"] = f"Bearer {transport_config.auth_secret}" | |
| # A single long-lived client is preferred over one-off request helpers. | |
| # trust_env=False makes config fetches deterministic: the SDK does not | |
| # implicitly inherit ambient proxy/SSL environment variables (e.g. a | |
| # host SOCKS proxy). Explicit transport configuration remains available | |
| # for proxied deployments. | |
| self._client = client or httpx.Client( | |
| base_url=transport_config.base_url, | |
| timeout=transport_config.timeout, | |
| verify=transport_config.verify_tls, | |
| headers=headers, | |
| trust_env=False, | |
| ) | |
| def __init__(self, transport_config: TransportConfig, *, client: Optional[httpx.Client] = None) -> None: | |
| self._config = transport_config | |
| headers: Dict[str, str] = dict(transport_config.headers) | |
| if transport_config.auth_secret: | |
| headers["Authorization"] = f"Bearer {transport_config.auth_secret}" | |
| self._owns_client = client is None | |
| # A single long-lived client is preferred over one-off request helpers. | |
| # trust_env=False makes config fetches deterministic: the SDK does not | |
| # implicitly inherit ambient proxy/SSL environment variables (e.g. a | |
| # host SOCKS proxy). Explicit transport configuration remains available | |
| # for proxied deployments. | |
| self._client = client or httpx.Client( | |
| base_url=transport_config.base_url, | |
| timeout=transport_config.timeout, | |
| verify=transport_config.verify_tls, | |
| headers=headers, | |
| trust_env=False, | |
| ) |
| def close(self) -> None: | ||
| self._client.close() |
There was a problem hiding this comment.
Only close the underlying httpx.Client if it was created internally by the transport adapter to avoid side effects on externally managed clients.
| def close(self) -> None: | |
| self._client.close() | |
| def close(self) -> None: | |
| if self._owns_client: | |
| self._client.close() |
| def _freeze(items: Sequence[Mapping[str, Any]]) -> tuple: | ||
| return tuple(MappingProxyType(dict(item)) for item in items) |
There was a problem hiding this comment.
The current _freeze implementation only creates a shallow copy and wraps the top-level dictionary of each entity in a MappingProxyType. However, nested mutable structures (such as variations lists or nested dictionaries) remain completely mutable. This violates the design requirement of an immutable configuration snapshot. We should recursively freeze all nested dictionaries and lists to ensure deep immutability.
def _deep_freeze(val: Any) -> Any:
if isinstance(val, dict):
return MappingProxyType({k: _deep_freeze(v) for k, v in val.items()})
if isinstance(val, list):
return tuple(_deep_freeze(x) for x in val)
return val
def _freeze(items: Sequence[Mapping[str, Any]]) -> tuple:
return tuple(_deep_freeze(item) for item in items)| if not isinstance(raw, dict): | ||
| raise InvalidConfigError( | ||
| f"config must be a mapping/dict; got {type(raw).__name__}" | ||
| ) | ||
|
|
||
| if "account_id" not in raw or raw.get("account_id") in (None, ""): | ||
| raise InvalidConfigError("config is missing required field 'account_id'") | ||
|
|
||
| project = raw.get("project") | ||
| if project is None: | ||
| raise InvalidConfigError("config is missing required field 'project'") | ||
| if not isinstance(project, dict): | ||
| raise InvalidConfigError("config 'project' must be a mapping/dict") | ||
| if project.get("id") in (None, ""): | ||
| raise InvalidConfigError("config 'project' is missing required field 'id'") |
There was a problem hiding this comment.
The type signature of load_snapshot and normalize_config specifies Mapping[str, Any], but validate_config strictly checks isinstance(raw, dict) and isinstance(project, dict). This will raise a runtime InvalidConfigError if a valid custom Mapping (such as MappingProxyType or a custom read-only dictionary) is passed. We should use collections.abc.Mapping for these checks to align with the type annotations and support standard Python mapping types.
| if not isinstance(raw, dict): | |
| raise InvalidConfigError( | |
| f"config must be a mapping/dict; got {type(raw).__name__}" | |
| ) | |
| if "account_id" not in raw or raw.get("account_id") in (None, ""): | |
| raise InvalidConfigError("config is missing required field 'account_id'") | |
| project = raw.get("project") | |
| if project is None: | |
| raise InvalidConfigError("config is missing required field 'project'") | |
| if not isinstance(project, dict): | |
| raise InvalidConfigError("config 'project' must be a mapping/dict") | |
| if project.get("id") in (None, ""): | |
| raise InvalidConfigError("config 'project' is missing required field 'id'") | |
| from collections.abc import Mapping | |
| if not isinstance(raw, Mapping): | |
| raise InvalidConfigError( | |
| f"config must be a mapping/dict; got {type(raw).__name__}" | |
| ) | |
| if "account_id" not in raw or raw.get("account_id") in (None, ""): | |
| raise InvalidConfigError("config is missing required field 'account_id'") | |
| project = raw.get("project") | |
| if project is None: | |
| raise InvalidConfigError("config is missing required field 'project'") | |
| if not isinstance(project, Mapping): | |
| raise InvalidConfigError("config 'project' must be a mapping/dict") | |
| if project.get("id") in (None, ""): | |
| raise InvalidConfigError("config 'project' is missing required field 'id'") |
| if has_key and not str(self.sdk_key).strip(): | ||
| raise InvalidConfigError("SDKConfig 'sdk_key' must be a non-empty string") |
There was a problem hiding this comment.
Converting self.sdk_key to a string using str(self.sdk_key) can mask invalid types (such as booleans, lists, or custom objects) and allow them to pass validation if their string representation is non-empty. We should explicitly verify that sdk_key is a string.
| if has_key and not str(self.sdk_key).strip(): | |
| raise InvalidConfigError("SDKConfig 'sdk_key' must be a non-empty string") | |
| if has_key and (not isinstance(self.sdk_key, str) or not self.sdk_key.strip()): | |
| raise InvalidConfigError("SDKConfig 'sdk_key' must be a non-empty string") |
|
Superseded — all commits already in main (bc76b64). Closing without merge as part of post-sprint cleanup. |
Summary
Implements Story 1.2 of the
2026-04-06-convert-python-sdksprint: SDK initialization from either direct config data or ansdkKey, with immutable config snapshot ingestion and a sync-firstCorereadiness surface.SDKConfig/TransportConfig+ typed initialization/config error hierarchy exported from the package rootconfig_loader/) into an immutableConfigSnapshotwith precomputed entity indexeshttpx-backed adapter:GET /config/{sdkKey}over HTTPS with conditionalenvironment={environment}and_conv_low_cache=1query params (JS parity, audit F-001/F-014), optional bearer authTransportErrorbefore any network I/Ofirst4***last4) inConfigLoadErrordiagnosticshttpx>=0.28,<1.0(qs-09, audit F-060)Verification
Traceability
sprint/2026-04-06-convert-python-sdkai-driven-product-dev/work/2026-06-06-support-sdkkey-and-direct-config-initialization/readiness-assessment.md🤖 Generated with Claude Code