diff --git a/packages/client/agents.md b/packages/client/agents.md index 203cc83f..6c5319ed 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -686,8 +686,13 @@ three signals are emitted by the `record_*` functions there; nothing else calls the allowlist is enforced in one place. **Injection goes through `skills.py`.** `_set_store`, `_set_emitter_for_testing` and -`_clear_state` delegate to `skills_core`. `set_skill_store`, `shutdown` and tests use -those; none should reach into `skills_core` directly. +`_clear_state` delegate to `skills_core`. `set_skill_store` and tests use those; none +should reach into `skills_core` directly. + +**Core never imports this feature.** `skills_core` registers `clear_state` with +`shutdown_hooks.register_shutdown_hook` when it loads, and `shutdown()` runs the registered +hooks. `lifecycle.py` imports no skills module (TESTING.md §0.3, pinned by +`test_core_does_not_import_experimental_code`). ### Descriptor-pinned filesystem access diff --git a/packages/client/src/launchdarkly_ai_server/lifecycle.py b/packages/client/src/launchdarkly_ai_server/lifecycle.py index 5d4bbadc..ed767400 100644 --- a/packages/client/src/launchdarkly_ai_server/lifecycle.py +++ b/packages/client/src/launchdarkly_ai_server/lifecycle.py @@ -6,9 +6,9 @@ import os from typing import Any -from . import skills from .sdk_info import flush_ai_sdk_info, reset_ai_sdk_info from .sdk_usage import flush_sdk_usage, report_usage, reset_sdk_usage +from .shutdown_hooks import run_shutdown_hooks from .types import InitClientOptions, SDKOptions from .utils import model_stamps_from_meta @@ -354,21 +354,14 @@ def _release_otel_globals() -> None: logger.debug("Could not release the global OTel tracer provider", exc_info=True) -def _clear_experimental_state() -> None: - """Clears experimental feature state. A failure is logged, never raised.""" - try: - skills._clear_state() - except Exception: - logger.warning("Could not clear the Agent Skills state", exc_info=True) - - async def shutdown() -> None: """ Shuts down the singleton client. Idempotent — safe to call multiple times even if the client was never initialized or already shut down. - Also clears any experimental feature state, such as a configured skill - store, and restores process-wide SDK defaults from ``configure``. + Also runs the registered shutdown hooks, which clear the state of any + experimental feature in use, and restores process-wide SDK defaults from + ``configure``. When telemetry was running, this also releases the process-global tracer provider so a later ``init_client`` can install its own — see @@ -380,7 +373,7 @@ async def shutdown() -> None: local_provider = _tracer_provider owned_globals = _owns_otel_globals - _clear_experimental_state() + run_shutdown_hooks() # Null the singleton before any awaits so a second call is a no-op. # Options reset even when no client was initialized: configure() does not @@ -434,9 +427,8 @@ def _reset_for_testing() -> None: _owns_otel_globals = False _sdk_options = SDKOptions() _ai_invoked = False - # Directly, not through _clear_experimental_state: a failed reset should - # fail the test rather than leak state into the next one. - skills._clear_state() + # A failed reset should fail the test rather than leak state into the next one. + run_shutdown_hooks(raise_errors=True) # Mirrors shutdown(): without this a suite that inits more than once leaves # every later span on the first test's provider. if owned_globals: diff --git a/packages/client/src/launchdarkly_ai_server/shutdown_hooks.py b/packages/client/src/launchdarkly_ai_server/shutdown_hooks.py new file mode 100644 index 00000000..06f42a7a --- /dev/null +++ b/packages/client/src/launchdarkly_ai_server/shutdown_hooks.py @@ -0,0 +1,44 @@ +""" +Cleanup hooks that ``shutdown()`` runs. + +This is how core reaches experimental state without importing it: an +experimental module registers its own cleanup when it loads, and ``shutdown()`` +runs whatever is registered. Core never imports, names or calls experimental +code (TESTING.md §0.3), so a hook's *name* comes from the module that +registers it. +""" + +from __future__ import annotations + +import logging +from collections.abc import Callable + +# The lifecycle logger: this warning came from shutdown() before the hooks existed. +logger = logging.getLogger("launchdarkly_ai_server.lifecycle") + +_hooks: dict[str, Callable[[], None]] = {} + + +def register_shutdown_hook(name: str, hook: Callable[[], None]) -> None: + """ + Registers *hook* to run on every ``shutdown()``, including before a client + exists. Keyed by *name*, so registering the same feature again replaces the + earlier hook rather than adding another. + """ + _hooks[name] = hook + + +def run_shutdown_hooks(*, raise_errors: bool = False) -> None: + """ + Runs every registered hook. Experimental code must not break a core call, so + a hook that raises is logged and the rest still run. *raise_errors* is for + the test reset, where a failed cleanup should fail the test instead. + """ + for name, hook in list(_hooks.items()): + if raise_errors: + hook() + continue + try: + hook() + except Exception: + logger.warning("Could not clear the %s state", name, exc_info=True) diff --git a/packages/client/src/launchdarkly_ai_server/skills_core.py b/packages/client/src/launchdarkly_ai_server/skills_core.py index a8c8cf03..e5cb6cbc 100644 --- a/packages/client/src/launchdarkly_ai_server/skills_core.py +++ b/packages/client/src/launchdarkly_ai_server/skills_core.py @@ -23,6 +23,7 @@ from dataclasses import dataclass from typing import Any, Literal, Protocol, get_args +from .shutdown_hooks import register_shutdown_hook from .types import Skill, SkillOutcomeReason, SkillReference from .types_validation import is_valid_skill_key, is_valid_skill_version @@ -190,6 +191,11 @@ def clear_state() -> None: _emitter = _NOOP_EMITTER +# Registered on load, so core's ``shutdown()`` clears this state without +# importing any Agent Skills module (TESTING.md §0.3). +register_shutdown_hook("Agent Skills", clear_state) + + def get_store() -> SkillStore | None: """The configured store, or ``None``. The only reader of the global.""" return _store diff --git a/packages/client/tests/test_lifecycle.py b/packages/client/tests/test_lifecycle.py index c1469868..50a7e733 100644 --- a/packages/client/tests/test_lifecycle.py +++ b/packages/client/tests/test_lifecycle.py @@ -4,12 +4,15 @@ """ import os +import subprocess +import sys from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest import launchdarkly_ai_server.lifecycle as lifecycle_module +import launchdarkly_ai_server.shutdown_hooks as shutdown_hooks from launchdarkly_ai_server import get_client, init_client, inspect_config, shutdown from launchdarkly_ai_server.lifecycle import _reset_for_testing from launchdarkly_ai_server.sdk_info import ( @@ -532,18 +535,38 @@ async def test_a_failure_clearing_experimental_state_is_logged_not_raised( stub = _make_stub_client() with patch.object(lifecycle_module, "_setup_telemetry", return_value=None): await init_client(client=stub) + later = MagicMock() + + def failing() -> None: + raise RuntimeError("clear failed") + with ( - patch.object( - lifecycle_module.skills, - "_clear_state", - side_effect=RuntimeError("clear failed"), - ), + patch.dict(shutdown_hooks._hooks, clear=False), caplog.at_level("WARNING", logger="launchdarkly_ai_server.lifecycle"), ): + shutdown_hooks.register_shutdown_hook("Failing Feature", failing) + shutdown_hooks.register_shutdown_hook("Later Feature", later) await shutdown() # must not raise stub.close.assert_called_once() assert lifecycle_module._client is None - assert "Could not clear the Agent Skills state" in caplog.messages + assert "Could not clear the Failing Feature state" in caplog.messages + later.assert_called_once() # one failing hook does not skip the rest + + def test_core_does_not_import_experimental_code(self) -> None: + """Core never hooks into experimental code (TESTING.md §0.3).""" + script = ( + "import asyncio, sys\n" + "import launchdarkly_ai_server\n" + "asyncio.run(launchdarkly_ai_server.shutdown())\n" + "loaded = sorted(m for m in sys.modules if m.startswith(" + "('launchdarkly_ai_server.skills', 'launchdarkly_ai_server.evaluations'," + " 'launchdarkly_ai_server.experimental')))\n" + "print(','.join(loaded))\n" + ) + result = subprocess.run( + [sys.executable, "-c", script], capture_output=True, text=True, check=True + ) + assert result.stdout.strip() == "" async def test_completes_teardown_even_if_flush_throws(self) -> None: stub = _make_stub_client()