diff --git a/.gitignore b/.gitignore index ba9e5e3..d8a77f4 100644 --- a/.gitignore +++ b/.gitignore @@ -185,3 +185,12 @@ agents-test test_reports/ cleveractors-core-new2/ pretty.output + +# MkDocs build output +site/ + +# ASV benchmark results +.asv/results/ + +# OpenCode development tooling +.opencode/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 60112af..ed4ed47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm - **`ActorResult` and `NodeUsage` types** (`cleveractors.result`): Canonical dataclasses for the router-facing result API, now defined in `cleveractors.result` (ADR-2027). `ActorResult` carries the response string, aggregated `prompt_tokens`/`completion_tokens`, a non-empty `nodes: list[NodeUsage]` breakdown for per-model billing, and an optional opaque `state` blob for stateless graph resumption (ADR-2026). Re-exported from `cleveractors.runtime` for backward compatibility. - **Real LangChain token extraction** (`LLMAgent`): `process_message()` now reads token counts from `response.usage_metadata` (primary path) with fallback to `response.response_metadata["token_usage"]`. Both paths are guarded with `isinstance(dict)` checks to prevent `AttributeError` on truthy non-dict provider values. Non-numeric token values are coerced via `_safe_int()` with fallback to 0 and a warning log. A warning distinguishing the failure cause (empty `usage_metadata`, missing `response_metadata`, or missing `token_usage` key) is emitted when no usage data is available. - **Per-node token accumulation** (`PureLangGraph`, `Node`): `Node._execute_agent()` reads `_last_token_usage`, `provider`, and `model` from each `LLMAgent` after invocation and includes a `_node_token_usage` dict in the state-updates return. `PureLangGraph` accumulates these into a `_node_usages` list and returns a 3-tuple `(response, final_state, node_usages)` from `execute()`. +- **Registry Client-Side Cache** (`cleveractors.registry.RegistryCache` + `CacheFactory` + `PackageContentResolver` integration): LRU cache with TTL for `RegistryClient` responses implementing Package Registry Standard v1.0.0 §10.3 client-side caching requirements. Wraps a `RegistryClient` as a transparent caching layer with configurable max size and TTL. Features SHA-1 content validation via `Canonicalizer` for tamper detection, LRU eviction policy, singleflight coalescing for concurrent-miss thundering-herd prevention, per-entry invalidation (`invalidate`), manual warm insertion (`put` — used by `PackageContentResolver` to seed the content cache from `resolve_package` results), full cache clear, `CacheStats` for observability (hits, misses, evictions), per-entry logging, async context manager support, and `__contains__` lookup. All cache operations are protected by an `asyncio.Lock` for thread-safe concurrent access. Returns defensive copies of stored content to prevent mutable-dict cache poisoning. Validates `package_id` format via `PackageId.from_string` as the first guard in all public methods. SHA-1 validation can be toggled off via `validate_content=False` for environments where the upstream content format does not directly match the stored PackageId (e.g., integration testing against incomplete mock servers). Boolean `max_size` guard rejects `True`/`False`. **CacheFactory** (Factory Method + Dependency Injection patterns) centralises cache creation and isolates cache configuration from consumers. **PackageContentResolver** now integrates a two-tier caching architecture: a resolution cache (`OrderedDict`, reference→enriched content) and per-server content caches (`RegistryCache`, package_id→raw content) created by `CacheFactory`. - **Per-request credential injection** (`AgentFactory` + `LLMAgent`): `AgentFactory` now accepts an optional `credentials: dict[str, dict[str, str]] | None` parameter. When supplied, each credential entry (keyed by provider name) is forwarded to the corresponding `LLMAgent`. The LangChain client is constructed lazily on first access of the `chat_model` property using the injected credentials, so API keys are never baked into the stored actor config dict (ADR-2026). - **Extended provider routing** (`LLMAgent`): Any provider not in `{openai, anthropic, google}` (e.g., `groq`, `fireworks`, `together`, `mistral`, `openrouter`, or the generic `openai_compatible` extension) is now routed to `ChatOpenAI(base_url=..., api_key=...)` using the `base_url` supplied in the credentials entry for that provider. Named providers and `openai_compatible` are treated identically under this routing path (ADR-2028). - **`ReactiveAgentFactory` backward-compatibility alias**: A module-level type alias `ReactiveAgentFactory: type[AgentFactory] = AgentFactory` is provided in `cleveractors.agents.factory` for backward compatibility with existing code that references the old name from v2.0.0. diff --git a/benchmarks/cache_benchmark.py b/benchmarks/cache_benchmark.py new file mode 100644 index 0000000..d1cc6a9 --- /dev/null +++ b/benchmarks/cache_benchmark.py @@ -0,0 +1,255 @@ +"""ASV benchmarks for RegistryCache, CacheFactory, and cached resolution. + +Measures performance of: + - get_package cache hit and cache miss+fetch + - resolve_package cache hit and cache miss+fetch + - singleflight coalescing (concurrent-miss deduplication) + - manual put (content warming), invalidate, and clear + - CacheFactory.create() construction + - End-to-end PackageContentResolver resolution with cache +""" + +from __future__ import annotations + +import asyncio +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +_STANDARD_CONTENT: dict[str, Any] = { + "name": "bench-pkg", + "type": "actor", + "version": "v1.0.0", + "config": { + "agent_type": "llm", + "model": "gpt-4", + "temperature": 0.7, + "system_prompt": "You are a benchmarking assistant.", + }, +} + +_RESOLVE_CONTENT: dict[str, Any] = { + "package_id": "pkg_act_0123456789abcdef0123456789abcdef01234567", + "type": "actor", + "name": "bench-pkg", + "version": "v1.0.0", + "config": { + "agent_type": "llm", + "model": "gpt-4", + "temperature": 0.7, + }, +} + +_STANDARD_PKG_ID: str = "pkg_act_0123456789abcdef0123456789abcdef01234567" + + +def _create_mock_client() -> MagicMock: + from cleveractors.registry.client import RegistryClient + + mock = MagicMock(spec=RegistryClient) + mock.get_package = AsyncMock(return_value=dict(_STANDARD_CONTENT)) + mock.resolve_package = AsyncMock(return_value=dict(_RESOLVE_CONTENT)) + mock.close = AsyncMock() + return mock + + +class CacheGetPackageHitBenchmark: + """Benchmark get_package() when content is already cached.""" + + def setup(self) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=False) + self.pkg_id = _STANDARD_PKG_ID + asyncio.run(self.cache.get_package(self.pkg_id)) + + def time_get_package_hit(self) -> None: + asyncio.run(self.cache.get_package(self.pkg_id)) + + def peakmem_get_package_hit(self) -> None: + asyncio.run(self.cache.get_package(self.pkg_id)) + + +class CacheGetPackageMissBenchmark: + """Benchmark get_package() on cold cache (miss + upstream fetch).""" + + def setup(self) -> None: + from cleveractors.registry.cache import RegistryCache + + self.mock_client = _create_mock_client() + self.pkg_id = _STANDARD_PKG_ID + + def time_get_package_miss(self) -> None: + from cleveractors.registry.cache import RegistryCache + + cache = RegistryCache(self.mock_client, validate_content=False) + asyncio.run(cache.get_package(self.pkg_id)) + + +class CacheResolvePackageHitBenchmark: + """Benchmark resolve_package() when the resolution is cached.""" + + params: list[str] = ["actor", "template", "skill"] + + def setup(self, pkg_type: str) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=False) + asyncio.run( + self.cache.resolve_package( + package_type=pkg_type, + namespace="bench-ns", + name="bench-pkg", + version="v1.0.0", + ) + ) + + def time_resolve_package_hit(self, pkg_type: str) -> None: + asyncio.run( + self.cache.resolve_package( + package_type=pkg_type, + namespace="bench-ns", + name="bench-pkg", + version="v1.0.0", + ) + ) + + +class CacheResolvePackageMissBenchmark: + """Benchmark resolve_package() on cold cache.""" + + params: list[str] = ["actor", "template", "skill"] + + def setup(self, pkg_type: str) -> None: + from cleveractors.registry.cache import RegistryCache + + self.mock_client = _create_mock_client() + + def time_resolve_package_miss(self, pkg_type: str) -> None: + from cleveractors.registry.cache import RegistryCache + + cache = RegistryCache(self.mock_client, validate_content=False) + asyncio.run( + cache.resolve_package( + package_type=pkg_type, + namespace="bench-ns", + name="bench-pkg", + version="v1.0.0", + ) + ) + + +class CacheSingleflightBenchmark: + """Benchmark the singleflight coalescing under concurrent access. + + When multiple coroutines request the same cold key concurrently, + only one upstream fetch is performed — the rest join the in-flight + future. This benchmark measures the throughput of N concurrent + get_package calls for the same cold key. + """ + + params: list[int] = [10, 50] + + def setup(self, concurrency: int) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=False) + self.pkg_id = _STANDARD_PKG_ID + self.concurrency = concurrency + + def time_get_package_concurrent(self, concurrency: int) -> None: + async def _run(): + tasks = [ + asyncio.ensure_future(self.cache.get_package(self.pkg_id)) + for _ in range(self.concurrency) + ] + await asyncio.gather(*tasks) + + asyncio.run(_run()) + + +class CacheInvalidateBenchmark: + """Benchmark per-entry invalidation.""" + + def setup(self) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=False) + self.pkg_id = _STANDARD_PKG_ID + asyncio.run(self.cache.get_package(self.pkg_id)) + + def time_invalidate(self) -> None: + asyncio.run(self.cache.invalidate(self.pkg_id)) + + +class CacheClearBenchmark: + """Benchmark full cache clear with N pre-loaded entries.""" + + params: list[int] = [10, 100] + + def setup(self, num_entries: int) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache( + _create_mock_client(), max_size=max(num_entries, 10), validate_content=False + ) + self.entries = [f"pkg_act_{i:040x}" for i in range(num_entries)] + + async def _warm(): + for pid in self.entries: + await self.cache.put(pid, dict(_STANDARD_CONTENT)) + + asyncio.run(_warm()) + + def time_clear(self, num_entries: int) -> None: + asyncio.run(self.cache.clear()) + + +class CachePutBenchmark: + """Benchmark manual cache warming via put().""" + + def setup(self) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=False) + self.pkg_id = _STANDARD_PKG_ID + self.content = dict(_STANDARD_CONTENT) + + def time_put(self) -> None: + asyncio.run(self.cache.put(self.pkg_id, self.content)) + + +class CacheFactoryBenchmark: + """Benchmark CacheFactory.create() construction.""" + + def setup(self) -> None: + from cleveractors.registry.cache import CacheFactory + + self.factory = CacheFactory(max_size=256, ttl=300.0, validate_content=False) + self.mock_client = _create_mock_client() + + def time_create(self) -> None: + self.factory.create(self.mock_client) + + +class CacheContentValidationBenchmark: + """Benchmark SHA-1 content validation overhead. + + Compares get_package throughput with and without validation + enabled to measure the cost of canonicalize + SHA-1 recompute. + When validation fails (content hash ≠ stored ID) the entry + is evicted and re-fetched — the computational cost of the + failed validation is the same but the re-fetch allocates + additional memory. + """ + + params: list[bool] = [True, False] + + def setup(self, validate: bool) -> None: + from cleveractors.registry.cache import RegistryCache + + self.cache = RegistryCache(_create_mock_client(), validate_content=validate) + self.pkg_id = _STANDARD_PKG_ID + asyncio.run(self.cache.get_package(self.pkg_id)) + + def time_get_package(self, validate: bool) -> None: + asyncio.run(self.cache.get_package(self.pkg_id)) diff --git a/features/cache_coverage.feature b/features/cache_coverage.feature new file mode 100644 index 0000000..39cb345 --- /dev/null +++ b/features/cache_coverage.feature @@ -0,0 +1,120 @@ +Feature: Registry Cache Coverage + + As a developer + I want to ensure all code paths in RegistryCache, CacheFactory, + and PackageContentResolver caching integration are covered + So that the project maintains the 97% coverage threshold + + Background: + Given a clean test environment for cache coverage + + # ── RegistryCache.resolve_package() ────────────────────────────────── + + Scenario: Resolve package returns content from mock client + When I create a RegistryCache wrapping a mock client + When I call resolve_package with type=actor ns=bench name=pkg version=v1.0.0 + Then the resolve result should contain package_id + And the resolve result should contain the test content + + Scenario: Resolve package cache hit + When I create a RegistryCache wrapping a mock client + When I call resolve_package twice with type=actor ns=bench name=pkg version=v1.0.0 + Then the resolve result should contain package_id + And the cache stats misses should be 1 + + Scenario: Resolve package with validation enabled + When I create a RegistryCache with validation enabled and mock client + When I call resolve_package twice with type=actor ns=bench name=pkg version=v1.0.0 + Then the resolve result should contain package_id + And the cache stats misses should be 1 + + # ── RegistryCache.put() ────────────────────────────────────────────── + + Scenario: Manual put inserts entry into cache + When I create a RegistryCache wrapping a mock client + When I put a package entry into the cache + Then the package should be present in the cache + + Scenario: Put triggers eviction when at capacity + When I put 3 entries into the cache at max_size 2 + Then the cache stats evictions should be 1 + + # ── CacheFactory ───────────────────────────────────────────────────── + + Scenario: CacheFactory with default settings + When I create a CacheFactory with default settings + Then the factory should be created with max_size=256 + And the factory should be created with ttl=300.0 + + Scenario: CacheFactory with custom settings + When I create a CacheFactory with max_size=128 ttl=60.0 + Then the factory should be created with max_size=128 + And the factory should be created with ttl=60.0 + + Scenario: CacheFactory create produces a RegistryCache + When I create a CacheFactory with default settings + When I call factory.create with a mock client + Then the cache should wrap the mock client + + Scenario: CacheFactory with invalid max_size raises ValueError + When I try to create a CacheFactory with max_size=0 + Then a ValueError should be raised by the cache factory + + Scenario: CacheFactory with invalid ttl raises ValueError + When I try to create a CacheFactory with ttl=-1.0 + Then a ValueError should be raised by the cache factory + + Scenario: CacheFactory with boolean max_size raises ValueError + When I try to create a CacheFactory with max_size boolean True + Then a ValueError should be raised by the cache factory + + # ── PackageContentResolver with CacheFactory ───────────────────────── + + Scenario: Resolver with CacheFactory resolves local reference + When I create a PackageContentResolver with a CacheFactory + When I resolve a local reference via the resolver + Then the resolve result should not be None + + Scenario: Resolver with CacheFactory resolves registry reference + When I create a PackageContentResolver with a CacheFactory + When I inject a mock registry client into the resolver + When I resolve a registry reference via the resolver + Then the resolve result should contain package_id + And the resolve result should contain the test content + + Scenario: Resolver total_stats is accessible + When I create a PackageContentResolver with a CacheFactory + When I inject a mock registry client into the resolver + When I resolve the same registry reference twice + When I read the resolver total_stats + Then the total_stats should have been accessible without error + And the resolver total stats hits should be 0 + And the resolver total stats misses should be 1 + + Scenario: Resolver clear_cache clears resolution cache + When I create a PackageContentResolver with a CacheFactory + When I resolve a local reference via the resolver + When I clear the resolver cache + Then the resolver cache should be empty + + Scenario: Resolver close_all closes clients and content caches + When I create a PackageContentResolver with a CacheFactory + When I resolve a local reference via the resolver + When I close all resolver resources + Then the resolver clients and content caches should be empty + + # ── Edge cases for coverage ────────────────────────────────────────── + + Scenario: CacheStats tracks operations after cache use + When I create a RegistryCache wrapping a mock client + When I put a package entry into the cache + When I call resolve_package with type=actor ns=bench name=pkg version=v1.0.0 + Then the cache stats misses should be 1 + + Scenario: Resolve package triggers eviction + When I create a RegistryCache with max_size 2 and mock client + When I call resolve_package with type=actor ns=bench name=pkg1 version=v1.0.0 + When I call resolve_package with type=actor ns=bench name=pkg2 version=v1.0.0 + When I call resolve_package with type=actor ns=bench name=pkg3 version=v1.0.0 + Then the resolve result should contain package_id + Then the cache stats evictions should be 1 diff --git a/features/environment.py b/features/environment.py index 09bf590..fc6cca8 100644 --- a/features/environment.py +++ b/features/environment.py @@ -114,6 +114,10 @@ def after_scenario(context, scenario): pending = asyncio.all_tasks(context.loop) for task in pending: task.cancel() + # Drain cancellations so tasks get cleaned up and do not + # accumulate on the shared loop across scenarios. + _drain = context.loop.create_task(asyncio.sleep(0)) + context.loop.run_until_complete(_drain) except Exception: pass diff --git a/features/registry_cache.feature b/features/registry_cache.feature new file mode 100644 index 0000000..dd72cf1 --- /dev/null +++ b/features/registry_cache.feature @@ -0,0 +1,208 @@ +Feature: Registry Client-Side Cache + + As a developer using the Package Registry Standard v1.0.0, + I want a client-side LRU cache that stores retrieved packages locally, + validates cached content via SHA-1, and expires entries based on TTL, + so that I can reduce network roundtrips and ensure content integrity. + + Background: + Given a clean test environment for registry cache + + # ── Cache Initialisation ────────────────────────────────────────────── + + Scenario: Create cache with default max size and TTL + When I create a RegistryCache with default settings + Then the cache max_size should be 256 + Then the cache ttl should be 300 + + Scenario: Create cache with custom max size and TTL + When I create a RegistryCache with max_size 64 and ttl 60 + Then the cache max_size should be 64 + Then the cache ttl should be 60 + + Scenario: Create cache with invalid client raises TypeError + When I try to create a RegistryCache with a non-client argument + Then a TypeError should be raised by the registry cache + + Scenario: Create cache with invalid max_size raises ValueError + When I try to create a RegistryCache with max_size 0 + Then a ValueError should be raised by the registry cache + + Scenario: Create cache with invalid ttl raises ValueError + When I try to create a RegistryCache with ttl -1 + Then a ValueError should be raised by the registry cache + + Scenario: Create cache with boolean max_size raises ValueError + When I try to create a RegistryCache with max_size boolean True + Then a ValueError should be raised by the registry cache + + # ── Cache Miss — Fresh Fetch ─────────────────────────────────────────── + + Scenario: First access triggers upstream fetch + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + Then the cache stats hits should be 0 + Then the cache stats misses should be 1 + + Scenario: First access returns upstream content + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + Then the cache content should match the standard package + + # ── Cache Hit ────────────────────────────────────────────────────────── + + Scenario: Second access hits cache and does not call upstream + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I call cache get_package for the standard package + Then the cache stats hits should be 1 + + # ── Content Validation ───────────────────────────────────────────────── + + Scenario: Valid content passes SHA-1 validation and serves from cache + When I create a RegistryCache with validation enabled and label "validated" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I call cache get_package for the standard package + Then the cache stats hits should be 1 + Then the cache stats misses should be 1 + + Scenario: Tampered cache entry fails validation and re-fetches + When I create a RegistryCache with validation enabled and label "validated" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I tamper with the cached standard package + When I call cache get_package for the standard package + Then the cache stats misses should be 2 + + # ── TTL Expiration ────────────────────────────────────────────────────── + + Scenario: Expired TTL triggers re-fetch + When I create a RegistryCache with max_size 256 and ttl 0.01 and no validation and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + Then I wait 0.05 seconds for TTL to expire + When I call cache get_package for the standard package + Then the cache stats misses should be 2 + + # ── LRU Eviction ──────────────────────────────────────────────────────── + + Scenario: Exceeding max_size evicts least-recently-used entry + When I create a RegistryCache with max_size 2 and ttl 300 and no validation and label "dummy" + When the mock upstream records package A + When the mock upstream records package B + When the mock upstream records package C + When I call cache get_package for package A + When I call cache get_package for package B + When I call cache get_package for package C + Then the cache stats evictions should be 1 + + Scenario: LRU eviction removes the least-recently-used entry not the MRU + When I create a RegistryCache with max_size 2 and ttl 300 and no validation and label "dummy" + When the mock upstream records package A + When the mock upstream records package B + When I call cache get_package for package A + When I call cache get_package for package B + When I call cache get_package for package A + When the mock upstream records package C + When I call cache get_package for package C + Then the cache stats evictions should be 1 + Then package B should NOT be in cache + Then package A should be in cache + Then package C should be in cache + + # ── Cache Statistics ──────────────────────────────────────────────────── + + Scenario: Statistics track hits and misses across multiple accesses + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I call cache get_package for the standard package + Then the cache stats hits should be 1 + Then the cache stats misses should be 1 + + # ── Invalidation ──────────────────────────────────────────────────────── + + Scenario: Invalidating an entry causes a cache miss on next access + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I invalidate the standard package from cache + When the mock upstream records a standard package + When I call cache get_package for the standard package + Then the cache stats misses should be 2 + + Scenario: Invalidating a missing entry returns False + When I create a RegistryCache with default settings + When I invalidate a missing cache entry + Then the cache should report the entry was NOT invalidated + + # ── Clear ─────────────────────────────────────────────────────────────── + + Scenario: Clearing the cache resets statistics and removes all entries + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package + When I clear the cache + Then the cache stats hits should be 0 + Then the cache stats misses should be 0 + Then the standard package should NOT be in cache + + # ── Wrapping RegistryClient ───────────────────────────────────────────── + + Scenario: Cache wraps RegistryClient transparently + When I create a RegistryCache wrapping a real RegistryClient + Then the cache should have the same base URL "https://registry.example.com" + + # ── Concurrent Access ─────────────────────────────────────────────────── + + Scenario: Concurrent access does not corrupt cache state + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package for the standard package 8 times concurrently + Then the cache stats hits should be 0 + Then the cache stats misses should be 1 + Then the total stats hits plus misses should be 1 + Then the cache content should match the standard package + Then the standard package should be in cache + + # ── get_package_content Alias ─────────────────────────────────────────── + + Scenario: get_package_content alias delegates to get_package + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I call cache get_package_content for the standard package + Then the cache stats misses should be 1 + Then the cache content should match the standard package + + # ── Error Propagation ─────────────────────────────────────────────────── + + Scenario: Invalid PackageId raises InvalidPackageIdError before upstream call + When I create a RegistryCache with validation disabled and label "dummy" + When I try to call cache get_package with an invalid package ID + Then an InvalidPackageIdError should be raised by the registry cache + + Scenario: Upstream PackageNotFoundError propagates to caller + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When the mock upstream raises PackageNotFoundError for the standard package + When I call cache get_package for the standard package and catch the error + Then a PackageNotFoundError should be raised by the registry cache + + Scenario: Upstream RegistryNetworkError propagates to caller + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When the mock upstream raises RegistryNetworkError for the standard package + When I call cache get_package for the standard package and catch the error + Then a RegistryNetworkError should be raised by the registry cache + + # ── Async Context Manager ─────────────────────────────────────────────── + + Scenario: Async context manager works correctly and closes client + When I create a RegistryCache with validation disabled and label "dummy" + When the mock upstream records a standard package + When I use the RegistryCache as an async context manager + Then the underlying client should be closed after context exit diff --git a/features/steps/cache_coverage_steps.py b/features/steps/cache_coverage_steps.py new file mode 100644 index 0000000..2050c84 --- /dev/null +++ b/features/steps/cache_coverage_steps.py @@ -0,0 +1,345 @@ +"""Step definitions for RegistryCache coverage gap tests.""" + +from __future__ import annotations + +import asyncio +import time +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +from behave import given, then, when + +from cleveractors.registry.cache import CacheFactory, RegistryCache +from cleveractors.registry.client import RegistryClient +from cleveractors.registry.reference_resolver import PackageContentResolver +from cleveractors.registry.types import PackageId, PackageReference + + +async def _async_noop() -> None: + pass + + +def _run_async(context: Any, coro: Any) -> Any: + return context.loop.run_until_complete(coro) + + +def _create_mock_client(context: Any) -> dict[str, Any]: + mock = MagicMock(spec=RegistryClient) + mock.get_package = AsyncMock(return_value=dict(context._test_content)) + resolve_return = dict(context._test_content) + resolve_return["package_id"] = context._pid + mock.resolve_package = AsyncMock(return_value=resolve_return) + mock.close = AsyncMock() + return mock + + +# ── Common test data ─────────────────────────────────────────────────── + + +@given("a clean test environment for cache coverage") +def step_clean_coverage_env(context: Any) -> None: + context.cache = None + context.resolver = None + context.error = None + context.result = None + context._test_content = { + "name": "cov-pkg", + "type": "actor", + "version": "v1.0.0", + "config": {"agent_type": "llm", "model": "gpt-4"}, + } + context._pid = "pkg_act_0123456789abcdef0123456789abcdef01234567" + + +# ── RegistryCache.resolve_package ────────────────────────────────────── + + +@when("I create a RegistryCache wrapping a mock client") +def step_create_cache_mock(context: Any) -> None: + context.cache = RegistryCache(_create_mock_client(context), validate_content=False) + + +@when( + "I call resolve_package with type={pkg_type} ns={ns} name={name} version={version}" +) +def step_call_resolve_package( + context: Any, pkg_type: str, ns: str, name: str, version: str +) -> None: + async def _call() -> None: + context.resolve_result = await context.cache.resolve_package( + package_type=pkg_type, + namespace=ns, + name=name, + version=version, + ) + + _run_async(context, _call()) + + +@when( + "I call resolve_package twice with type={pkg_type} ns={ns} name={name} version={version}" +) +def step_call_resolve_package_twice( + context: Any, pkg_type: str, ns: str, name: str, version: str +) -> None: + async def _call() -> None: + await context.cache.resolve_package( + package_type=pkg_type, + namespace=ns, + name=name, + version=version, + ) + context.resolve_result = await context.cache.resolve_package( + package_type=pkg_type, + namespace=ns, + name=name, + version=version, + ) + + _run_async(context, _call()) + + +@when("I create a RegistryCache with validation enabled and mock client") +def step_create_cache_val_enabled_mock(context: Any) -> None: + context.cache = RegistryCache(_create_mock_client(context), validate_content=True) + + +# ── RegistryCache.put ────────────────────────────────────────────────── + + +@when("I put a package entry into the cache") +def step_put_into_cache(context: Any) -> None: + async def _call() -> None: + await context.cache.put(context._pid, dict(context._test_content)) + + _run_async(context, _call()) + + +@when("I put {count:d} entries into the cache at max_size {max_size:d}") +def step_put_entries_exceeding_max(context: Any, count: int, max_size: int) -> None: + mock = MagicMock(spec=RegistryClient) + mock.get_package = AsyncMock(return_value=dict(context._test_content)) + mock.resolve_package = AsyncMock(return_value=dict(context._test_content)) + mock.close = AsyncMock() + context.cache = RegistryCache(mock, max_size=max_size, validate_content=False) + + async def _call() -> None: + for i in range(count): + pid = f"pkg_act_{i:040x}" + await context.cache.put( + pid, {**context._test_content, "name": f"cov-pkg-{i}"} + ) + + _run_async(context, _call()) + + +# ── CacheFactory ─────────────────────────────────────────────────────── + + +@when("I create a CacheFactory with default settings") +def step_create_factory_default(context: Any) -> None: + context.factory = CacheFactory() + + +@when("I create a CacheFactory with max_size={max_size:d} ttl={ttl}") +def step_create_factory_custom(context: Any, max_size: int, ttl: str) -> None: + context.factory = CacheFactory(max_size=max_size, ttl=float(ttl)) + + +@when("I try to create a CacheFactory with max_size={max_size:d}") +def step_create_factory_bad_size(context: Any, max_size: int) -> None: + try: + CacheFactory(max_size=max_size) + except ValueError as exc: + context.error = exc + + +@when("I try to create a CacheFactory with ttl={ttl}") +def step_create_factory_bad_ttl(context: Any, ttl: str) -> None: + try: + CacheFactory(ttl=float(ttl)) + except ValueError as exc: + context.error = exc + + +@when("I try to create a CacheFactory with max_size boolean True") +def step_create_factory_bool(context: Any) -> None: + try: + CacheFactory(max_size=True) + except ValueError as exc: + context.error = exc + + +@when("I call factory.create with a mock client") +def step_factory_create(context: Any) -> None: + context.cache = context.factory.create(_create_mock_client(context)) + + +# ── PackageContentResolver with CacheFactory ─────────────────────────── + + +@when("I create a PackageContentResolver with a CacheFactory") +def step_create_resolver_with_factory(context: Any) -> None: + context.factory = CacheFactory(max_size=64, ttl=300.0, validate_content=False) + context.resolver = PackageContentResolver(cache_factory=context.factory) + context.resolver.set_server_alias("test.registry", "http://127.0.0.1:1") + + +@when("I inject a mock registry client into the resolver") +def step_inject_mock_client(context: Any) -> None: + resolve_return = dict(context._test_content) + resolve_return["package_id"] = context._pid + mock = MagicMock(spec=RegistryClient) + mock.resolve_package = AsyncMock(return_value=resolve_return) + mock.get_package = AsyncMock(return_value=dict(context._test_content)) + mock.close = AsyncMock() + context.resolver.clients["http://127.0.0.1:1"] = mock + + +@when("I resolve a registry reference via the resolver") +def step_resolve_registry_ref(context: Any) -> None: + pr = PackageReference.from_string("test.registry:acme/pkg@v1.0.0") + context.resolve_result = context.resolver.resolve(pr, package_type="actor") + + +@when("I resolve the same registry reference twice") +def step_resolve_registry_ref_twice(context: Any) -> None: + pr = PackageReference.from_string("test.registry:acme/pkg@v1.0.0") + context.resolver.resolve(pr, package_type="actor") + context.resolve_result = context.resolver.resolve(pr, package_type="actor") + + +@when("I resolve a local reference via the resolver") +def step_resolve_local_ref(context: Any) -> None: + pr = PackageReference.from_string("local:test/file.yaml") + context.resolve_result = context.resolver.resolve(pr) + + +@when("I resolve the same local reference twice") +def step_resolve_local_ref_twice(context: Any) -> None: + pr = PackageReference.from_string("local:test/file.yaml") + context.resolver.resolve(pr) + context.resolve_result = context.resolver.resolve(pr) + + +# ── total_stats ──────────────────────────────────────────────────────── + + +@when("I read the resolver total_stats") +def step_read_total_stats(context: Any) -> None: + context.stats = context.resolver.total_stats + + +# ── clear_cache ──────────────────────────────────────────────────────── + + +@when("I clear the resolver cache") +def step_clear_resolver_cache(context: Any) -> None: + context.resolver.clear_cache() + + +# ── close_all ────────────────────────────────────────────────────────── + + +@when("I close all resolver resources") +def step_close_all_resources(context: Any) -> None: + async def _close() -> None: + await context.resolver.close_all() + + _run_async(context, _close()) + + +# ── Assertions ───────────────────────────────────────────────────────── + + +@then("the resolve result should contain package_id") +def step_resolve_result_has_package_id(context: Any) -> None: + assert context.resolve_result is not None + assert "package_id" in context.resolve_result, ( + f"Expected package_id in {context.resolve_result}" + ) + + +@then("the resolve result should contain the test content") +def step_resolve_result_has_content(context: Any) -> None: + assert context.resolve_result is not None + assert context.resolve_result.get("name") == context._test_content["name"] + + +@then("the package should be present in the cache") +def step_pkg_in_cache(context: Any) -> None: + assert context._pid in context.cache, f"Expected {context._pid} in cache" + + +@then("the factory should be created with max_size={expected:d}") +def step_factory_max_size(context: Any, expected: int) -> None: + assert context.factory.max_size == expected + + +@then("the factory should be created with ttl={expected}") +def step_factory_ttl(context: Any, expected: str) -> None: + assert context.factory.ttl == float(expected) + + +@then("a ValueError should be raised by the cache factory") +def step_factory_value_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, ValueError), ( + f"Expected ValueError, got {type(context.error).__name__}" + ) + + +@then("the cache should wrap the mock client") +def step_cache_has_client(context: Any) -> None: + assert context.cache is not None + assert context.cache._client is not None + + +@then("the resolve result should not be None") +def step_result_not_none(context: Any) -> None: + assert context.resolve_result is not None + + +@then("the total_stats should have been accessible without error") +def step_stats_accessible(context: Any) -> None: + assert context.stats is not None + + +@then("the resolver cache should be empty") +def step_resolver_cache_empty(context: Any) -> None: + assert len(context.resolver.cache) == 0, ( + f"Expected empty cache, got {len(context.resolver.cache)} entries" + ) + + +@when("I create a RegistryCache with max_size {max_size:d} and mock client") +def step_create_cache_max_size_mock(context: Any, max_size: int) -> None: + context.cache = RegistryCache( + _create_mock_client(context), + max_size=max_size, + validate_content=False, + ) + + +@then("the resolver clients and content caches should be empty") +def step_resolver_clients_empty(context: Any) -> None: + assert len(context.resolver.clients) == 0, ( + f"Expected 0 clients, got {len(context.resolver.clients)}" + ) + assert len(context.resolver._content_caches) == 0, ( + f"Expected 0 content caches, got {len(context.resolver._content_caches)}" + ) + + +@then("the resolver total stats hits should be {expected:d}") +def step_total_stats_hits(context: Any, expected: int) -> None: + assert context.stats.hits == expected, ( + f"Expected total_stats.hits={expected}, got {context.stats.hits}" + ) + + +@then("the resolver total stats misses should be {expected:d}") +def step_total_stats_misses(context: Any, expected: int) -> None: + assert context.stats.misses == expected, ( + f"Expected total_stats.misses={expected}, got {context.stats.misses}" + ) diff --git a/features/steps/registry_cache_steps.py b/features/steps/registry_cache_steps.py new file mode 100644 index 0000000..963ebe2 --- /dev/null +++ b/features/steps/registry_cache_steps.py @@ -0,0 +1,528 @@ +"""Step definitions for RegistryCache BDD tests.""" + +from __future__ import annotations + +import asyncio +import hashlib +import time +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +from behave import given, then, when + +from cleveractors.registry.cache import RegistryCache +from cleveractors.registry.canonical import Canonicalizer +from cleveractors.registry.client import RegistryClient +from cleveractors.registry.exceptions import ( + InvalidPackageIdError, + PackageNotFoundError, + RegistryNetworkError, +) +from cleveractors.registry.types import PackageContent, PackageId + + +def _canonical_sha1(content: dict[str, Any]) -> str: + canonicalizer = Canonicalizer() + canonical = canonicalizer.canonicalize(content) + return hashlib.sha1(canonical.encode("utf-8"), usedforsecurity=False).hexdigest() + + +_STANDARD_CONTENT = {"name": "test-pkg", "type": "actor"} +_PKG_A_CONTENT = {"name": "pkg-a", "type": "actor"} +_PKG_B_CONTENT = {"name": "pkg-b", "type": "actor"} +_PKG_C_CONTENT = {"name": "pkg-c", "type": "actor"} + +_LABEL_CONTENT_MAP: dict[str, dict[str, Any]] = { + "A": _PKG_A_CONTENT, + "B": _PKG_B_CONTENT, + "C": _PKG_C_CONTENT, +} + + +def _run_async(context: Any, coro: Any) -> Any: + """Run a coroutine using the scenario event loop.""" + return context.loop.run_until_complete(coro) + + +@given("a clean test environment for registry cache") +def step_clean_cache_env(context: Any) -> None: + context.cache = None + context.error = None + context._mock_responses: dict[str, dict[str, Any]] = {} + context._content_labels: dict[str, dict[str, Any]] = {} + context._current_standard_pkg_id = "" + + context._upstream_mock = AsyncMock() + context._real_upstream_get_package = None + + _mock_client = MagicMock(spec=RegistryClient) + _mock_client.get_package = context._upstream_mock + _mock_client.close = MagicMock() + context._mock_client_ref = _mock_client + + +# ── Cache Initialisation ────────────────────────────────────────────────── + + +def _create_cache(context: Any, **kwargs: Any) -> None: + mock_client = MagicMock(spec=RegistryClient) + mock_client.get_package = context._upstream_mock + mock_client.close = _async_noop + mock_client._get_client = MagicMock() + context.cache = RegistryCache(mock_client, **kwargs) + context._mock_client_ref = mock_client + + +async def _async_noop() -> None: + pass + + +@when("I create a RegistryCache with default settings") +def step_create_cache_default(context: Any) -> None: + _create_cache(context, validate_content=False) + + +@when('I create a RegistryCache with validation disabled and label "{label}"') +def step_create_cache_no_val(context: Any, label: str) -> None: + _create_cache(context, validate_content=False) + context._cache_label = label + + +@when('I create a RegistryCache with validation enabled and label "{label}"') +def step_create_cache_with_val(context: Any, label: str) -> None: + _create_cache(context, validate_content=True) + context._cache_label = label + + +@when("I create a RegistryCache with max_size {max_size:d} and ttl {ttl:d}") +def step_create_cache_custom(context: Any, max_size: int, ttl: int) -> None: + _create_cache(context, max_size=max_size, ttl=ttl, validate_content=False) + + +@when( + 'I create a RegistryCache with max_size {max_size:d} and ttl {ttl} and no validation and label "{label}"' +) +def step_create_cache_custom_no_val( + context: Any, max_size: int, ttl: str, label: str +) -> None: + _create_cache(context, max_size=max_size, ttl=float(ttl), validate_content=False) + context._cache_label = label + + +@when("I try to create a RegistryCache with a non-client argument") +def step_create_cache_bad_client(context: Any) -> None: + bad_client: Any = "not-a-client" + try: + RegistryCache(bad_client) + except TypeError as exc: + context.error = exc + + +@when("I try to create a RegistryCache with max_size {max_size:d}") +def step_create_cache_bad_size(context: Any, max_size: int) -> None: + mock = MagicMock(spec=RegistryClient) + mock.get_package = context._upstream_mock + mock.close = _async_noop + try: + RegistryCache(mock, max_size=max_size) + except ValueError as exc: + context.error = exc + + +@when("I try to create a RegistryCache with ttl {ttl:d}") +def step_create_cache_bad_ttl(context: Any, ttl: int) -> None: + mock = MagicMock(spec=RegistryClient) + mock.get_package = context._upstream_mock + mock.close = _async_noop + try: + RegistryCache(mock, ttl=ttl) + except ValueError as exc: + context.error = exc + + +@then("the cache max_size should be {expected:d}") +def step_check_max_size(context: Any, expected: int) -> None: + assert context.cache is not None + assert context.cache.max_size == expected + + +@then("the cache ttl should be {expected:d}") +def step_check_ttl(context: Any, expected: int) -> None: + assert context.cache is not None + assert context.cache.ttl == expected + + +@then("a TypeError should be raised by the registry cache") +def step_check_type_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, TypeError) + + +@then("a ValueError should be raised by the registry cache") +def step_check_value_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, ValueError) + + +@when("I try to create a RegistryCache with max_size boolean True") +def step_create_cache_bool_max_size(context: Any) -> None: + mock = MagicMock(spec=RegistryClient) + mock.get_package = context._upstream_mock + mock.close = _async_noop + try: + RegistryCache(mock, max_size=True) + except ValueError as exc: + context.error = exc + + +# ── Mock upstream content ───────────────────────────────────────────────── + + +def _register_content(context: Any, content: dict[str, Any], label: str) -> str: + pkg_id = f"pkg_act_{_canonical_sha1(content)}" + context._content_labels[label] = content + context._mock_responses[pkg_id] = content + + async def _get_package(pid: str) -> dict[str, Any]: + data = context._mock_responses.get(pid) + if data is not None: + return data.copy() + raise RuntimeError(f"No content for {pid}") + + context._upstream_mock.side_effect = _get_package + return pkg_id + + +@when("the mock upstream records a standard package") +def step_record_standard(context: Any) -> None: + pkg_id = _register_content(context, _STANDARD_CONTENT, "standard") + context._current_standard_pkg_id = pkg_id + + +@when("the mock upstream records package A") +def step_record_a(context: Any) -> None: + _register_content(context, _PKG_A_CONTENT, "A") + + +@when("the mock upstream records package B") +def step_record_b(context: Any) -> None: + _register_content(context, _PKG_B_CONTENT, "B") + + +@when("the mock upstream records package C") +def step_record_c(context: Any) -> None: + _register_content(context, _PKG_C_CONTENT, "C") + + +# ── Calling cache get_package ────────────────────────────────────────────── + + +@when("I call cache get_package for the standard package") +def step_call_standard(context: Any) -> None: + async def _call() -> None: + context._cache_result = await context.cache.get_package( + context._current_standard_pkg_id + ) + + _run_async(context, _call()) + + +@when("I call cache get_package for package {label}") +def step_call_labeled(context: Any, label: str) -> None: + content = _LABEL_CONTENT_MAP[label] + pkg_id = f"pkg_act_{_canonical_sha1(content)}" + + async def _call() -> None: + context._cache_result = await context.cache.get_package(pkg_id) + + _run_async(context, _call()) + + +@when("I call cache get_package_content for the standard package") +def step_call_get_package_content_standard(context: Any) -> None: + async def _call() -> None: + context._cache_result = await context.cache.get_package_content( + context._current_standard_pkg_id + ) + + _run_async(context, _call()) + + +# ── Concurrent access ────────────────────────────────────────────────────── + + +@when("I call cache get_package for the standard package {count:d} times concurrently") +def step_call_concurrent(context: Any, count: int) -> None: + pkg_id = context._current_standard_pkg_id + + async def _concurrent() -> None: + tasks = [ + asyncio.create_task(context.cache.get_package(pkg_id)) for _ in range(count) + ] + context._concurrent_results = list(await asyncio.gather(*tasks)) + + _run_async(context, _concurrent()) + context._cache_result = context._concurrent_results[0] + + +# ── Tamper ───────────────────────────────────────────────────────────────── + + +@when("I tamper with the cached standard package") +def step_tamper_standard(context: Any) -> None: + async def _tamper() -> None: + async with context.cache._lock: + pkg_id = context._current_standard_pkg_id + if pkg_id in context.cache._store: + stored_content, stored_at = context.cache._store[pkg_id] + tampered_data = dict(stored_content.content) + tampered_data["tampered"] = True + new_content = PackageContent( + id=stored_content.id, + content=tampered_data, + ) + context.cache._store[pkg_id] = (new_content, stored_at) + + _run_async(context, _tamper()) + + +# ── Error propagation ────────────────────────────────────────────────────── + + +@when("the mock upstream raises PackageNotFoundError for the standard package") +def step_upstream_not_found(context: Any) -> None: + pkg_id = context._current_standard_pkg_id + + async def _raise_not_found(pid: str) -> dict[str, Any]: + if pid == pkg_id: + raise PackageNotFoundError("Package not found") + return context._mock_responses.get(pid, {}) + + context._upstream_mock.side_effect = _raise_not_found + + +@when("the mock upstream raises RegistryNetworkError for the standard package") +def step_upstream_network_error(context: Any) -> None: + pkg_id = context._current_standard_pkg_id + + async def _raise_network(pid: str) -> dict[str, Any]: + if pid == pkg_id: + raise RegistryNetworkError("Connection refused") + return context._mock_responses.get(pid, {}) + + context._upstream_mock.side_effect = _raise_network + + +# ── Statistics assertions ────────────────────────────────────────────────── + + +@then("the cache stats hits should be {expected:d}") +def step_check_stats_hits(context: Any, expected: int) -> None: + assert context.cache is not None + assert context.cache.stats.hits == expected, ( + f"Expected {expected} hits, got {context.cache.stats.hits}" + ) + + +@then("the cache stats misses should be {expected:d}") +def step_check_stats_misses(context: Any, expected: int) -> None: + assert context.cache is not None + assert context.cache.stats.misses == expected, ( + f"Expected {expected} misses, got {context.cache.stats.misses}" + ) + + +@then("the cache stats evictions should be {expected:d}") +def step_check_stats_evictions(context: Any, expected: int) -> None: + assert context.cache is not None + assert context.cache.stats.evictions == expected, ( + f"Expected {expected} evictions, got {context.cache.stats.evictions}" + ) + + +@then("the total stats hits plus misses should be {expected:d}") +def step_check_total_hits_misses(context: Any, expected: int) -> None: + assert context.cache is not None + total = context.cache.stats.hits + context.cache.stats.misses + assert total == expected, f"Expected total {expected}, got {total}" + + +# ── Content assertions ───────────────────────────────────────────────────── + + +@then("the cache content should match the standard package") +def step_match_standard(context: Any) -> None: + assert context._cache_result is not None + assert context._cache_result == _STANDARD_CONTENT, ( + f"Expected {_STANDARD_CONTENT}, got {context._cache_result}" + ) + + +@then("the standard package should NOT be in cache") +def step_not_in_cache(context: Any) -> None: + assert context.cache is not None + pkg_id = context._current_standard_pkg_id + assert pkg_id not in context.cache, f"Cache unexpectedly contains {pkg_id}" + + +@then("the standard package should be in cache") +def step_in_cache(context: Any) -> None: + assert context.cache is not None + pkg_id = context._current_standard_pkg_id + assert pkg_id in context.cache, f"Package {pkg_id} not in cache" + + +@then("package {label} should NOT be in cache") +def step_label_not_in_cache(context: Any, label: str) -> None: + content = _LABEL_CONTENT_MAP[label] + pkg_id = f"pkg_act_{_canonical_sha1(content)}" + assert context.cache is not None + assert pkg_id not in context.cache, ( + f"Cache unexpectedly contains {pkg_id} for {label}" + ) + + +@then("package {label} should be in cache") +def step_label_in_cache(context: Any, label: str) -> None: + content = _LABEL_CONTENT_MAP[label] + pkg_id = f"pkg_act_{_canonical_sha1(content)}" + assert context.cache is not None + assert pkg_id in context.cache, f"Package {label} not in cache" + + +# ── TTL wait ─────────────────────────────────────────────────────────────── + + +@then("I wait {seconds:f} seconds for TTL to expire") +def step_wait_ttl(context: Any, seconds: float) -> None: + time.sleep(seconds) + + +# ── Invalidation ─────────────────────────────────────────────────────────── + + +@when("I invalidate the standard package from cache") +def step_invalidate_standard(context: Any) -> None: + async def _inv() -> None: + context._invalidate_result = await context.cache.invalidate( + context._current_standard_pkg_id + ) + + _run_async(context, _inv()) + + +@when("I invalidate a missing cache entry") +def step_invalidate_missing(context: Any) -> None: + async def _inv() -> None: + context._invalidate_result = await context.cache.invalidate( + "pkg_act_0000000000000000000000000000000000000000" + ) + + _run_async(context, _inv()) + + +@then("the cache should report the entry was NOT invalidated") +def step_check_not_invalidated(context: Any) -> None: + assert context._invalidate_result is False + + +# ── Clear ────────────────────────────────────────────────────────────────── + + +@when("I clear the cache") +def step_clear_cache(context: Any) -> None: + async def _clear() -> None: + await context.cache.clear() + + _run_async(context, _clear()) + + +# ── Wrapping RegistryClient ──────────────────────────────────────────────── + + +@when("I create a RegistryCache wrapping a real RegistryClient") +def step_create_cache_wrapping_real(context: Any) -> None: + real_client = RegistryClient(base_url="https://registry.example.com") + context.cache = RegistryCache(real_client) + + +@then('the cache should have the same base URL "{expected_url}"') +def step_check_base_url_via_cache(context: Any, expected_url: str) -> None: + assert context.cache is not None + assert context.cache._client.base_url == expected_url.rstrip("/") + + +# ── Async context manager ────────────────────────────────────────────────── + + +@when("I use the RegistryCache as an async context manager") +def step_async_context_manager(context: Any) -> None: + async def _use_context() -> None: + mock_client = MagicMock(spec=RegistryClient) + mock_client.get_package = context._upstream_mock + mock_client.close = AsyncMock() + mock_client._get_client = MagicMock() + + async with RegistryCache(mock_client) as cache: + context._ctx_cache = cache + context._ctx_client = mock_client + pkg_id = context._current_standard_pkg_id + await cache.get_package(pkg_id) + + _run_async(context, _use_context()) + + +@then("the underlying client should be closed after context exit") +def step_client_closed(context: Any) -> None: + context._ctx_client.close.assert_called_once() + + +# ── Error propagation ────────────────────────────────────────────────────── + + +@when("I call cache get_package for the standard package and catch the error") +def step_call_standard_catch(context: Any) -> None: + try: + + async def _call() -> None: + await context.cache.get_package(context._current_standard_pkg_id) + + _run_async(context, _call()) + except Exception as exc: + context.error = exc + + +@when("I try to call cache get_package with an invalid package ID") +def step_call_invalid_id(context: Any) -> None: + try: + + async def _call() -> None: + await context.cache.get_package("not-a-valid-id") + + _run_async(context, _call()) + except Exception as exc: + context.error = exc + + +@then("an InvalidPackageIdError should be raised by the registry cache") +def step_check_invalid_package_id_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, InvalidPackageIdError), ( + f"Expected InvalidPackageIdError, got {type(context.error).__name__}" + ) + + +@then("a PackageNotFoundError should be raised by the registry cache") +def step_check_not_found_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, PackageNotFoundError), ( + f"Expected PackageNotFoundError, got {type(context.error).__name__}" + ) + + +@then("a RegistryNetworkError should be raised by the registry cache") +def step_check_network_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, RegistryNetworkError), ( + f"Expected RegistryNetworkError, got {type(context.error).__name__}" + ) diff --git a/robot/CleverActorsLib.py b/robot/CleverActorsLib.py index 1222364..e75c45d 100644 --- a/robot/CleverActorsLib.py +++ b/robot/CleverActorsLib.py @@ -38,6 +38,7 @@ from cleveractors.templates.base import ( InstantiationContext, TemplateType, ) +from cleveractors.registry.cache import CacheFactory from cleveractors.registry.reference_resolver import PackageContentResolver from cleveractors.registry.types import PackageReference @@ -674,6 +675,60 @@ class CleverActorsLib: # pragma: no cover - integration test library f"Expected empty cache, got {len(self._ref_resolver.cache)} entries" ) + # ── cache factory + transparent caching (issue #28) ─────────────── + + def create_reference_resolver_with_cache_factory( + self, max_size: str = "256", ttl: str = "300.0" + ) -> None: + factory = CacheFactory( + max_size=int(max_size), ttl=float(ttl), validate_content=False + ) + self._ref_resolver = PackageContentResolver(cache_factory=factory) + + def set_resolver_server_alias(self, alias: str, resolved_url: str) -> None: + self._ref_resolver.set_server_alias(alias, resolved_url) + + def resolve_reference_and_collect_stats( + self, ref_string: str, package_type: str = "actor" + ) -> None: + pr = PackageReference.from_string(ref_string) + self._resolved = self._ref_resolver.resolve(pr, package_type=package_type) + + def total_stats_hits_greater_than(self, expected: str) -> None: + actual = self._ref_resolver.total_stats.hits + if actual <= int(expected): + raise AssertionError( + f"Expected total_stats.hits > {expected}, got {actual}" + ) + + def resolver_clear_all_caches(self) -> None: + self._ref_resolver.clear_cache() + + def resolver_close_all_resources(self) -> None: + asyncio.run(self._ref_resolver.close_all()) + if len(self._ref_resolver.clients) != 0: + raise AssertionError( + f"Expected 0 clients after close_all, " + f"got {len(self._ref_resolver.clients)}" + ) + if len(self._ref_resolver._content_caches) != 0: + raise AssertionError( + f"Expected 0 content caches after close_all, " + f"got {len(self._ref_resolver._content_caches)}" + ) + + def resolver_resolution_cache_is_empty(self) -> None: + if len(self._ref_resolver.cache) != 0: + raise AssertionError( + f"Expected empty resolution cache, " + f"got {len(self._ref_resolver.cache)} entries" + ) + + def resolver_total_stats_misses_equals(self, expected: str) -> None: + actual = self._ref_resolver.total_stats.misses + if actual != int(expected): + raise AssertionError(f"Expected {expected} misses, got {actual}") + # ── instantiation context with registry awareness (issue #27) ────── def create_context_with_resolver(self) -> None: diff --git a/robot/RegistryCacheLib.py b/robot/RegistryCacheLib.py new file mode 100644 index 0000000..fa65875 --- /dev/null +++ b/robot/RegistryCacheLib.py @@ -0,0 +1,99 @@ +"""Robot Framework keyword library for RegistryCache integration tests. + +Provides keywords for testing the RegistryCache against a fake registry +server started by the test suite. +""" + +from __future__ import annotations + +import asyncio +from typing import Any, Optional + +from cleveractors.registry.cache import RegistryCache +from cleveractors.registry.client import RegistryClient + +ROBOT_LIBRARY_SCOPE = "TEST SUITE" # pragma: no cover - integration test library + + +class RegistryCacheLib: # pragma: no cover - integration test library + """Keyword library for RegistryCache Robot Framework integration tests.""" + + def __init__(self) -> None: + self._client: Optional[RegistryClient] = None + self._cache: Optional[RegistryCache] = None + self._result: Any = None + + def create_registry_cache(self, base_url: str) -> None: + self._client = RegistryClient(base_url=base_url) + self._cache = RegistryCache(self._client, validate_content=False) + + def create_registry_cache_with_config( + self, base_url: str, max_size: str, ttl: str + ) -> None: + self._client = RegistryClient(base_url=base_url) + self._cache = RegistryCache( + self._client, max_size=int(max_size), ttl=float(ttl), validate_content=False + ) + + def cache_get_package(self, package_id: str) -> None: + async def _call() -> None: + self._result = await self._cache.get_package(package_id) + + self._run(_call) + + def cache_stats_hits_equals(self, expected: str) -> None: + actual = str(self._cache.stats.hits) + if actual != expected: + raise AssertionError(f"Hits: expected {expected!r}, got {actual!r}") + + def cache_stats_misses_equals(self, expected: str) -> None: + actual = str(self._cache.stats.misses) + if actual != expected: + raise AssertionError(f"Misses: expected {expected!r}, got {actual!r}") + + def cache_stats_evictions_equals(self, expected: str) -> None: + actual = str(self._cache.stats.evictions) + if actual != expected: + raise AssertionError(f"Evictions: expected {expected!r}, got {actual!r}") + + def cache_should_not_contain(self, package_id: str) -> None: + if package_id in self._cache: + raise AssertionError(f"Cache still contains {package_id!r}") + + def cache_should_contain(self, package_id: str) -> None: + if package_id not in self._cache: + raise AssertionError(f"Cache does not contain {package_id!r}") + + def cache_clear(self) -> None: + async def _call() -> None: + await self._cache.clear() + + self._run(_call) + + def cache_invalidate(self, package_id: str) -> None: + async def _call() -> None: + self._result = await self._cache.invalidate(package_id) + + self._run(_call) + + def invalidate_result_should_be(self, expected: str) -> None: + result_str = str(self._result) + if result_str != expected: + raise AssertionError( + f"Invalidate result: expected {expected!r}, got {result_str!r}" + ) + + def close_cache(self) -> None: + async def _call() -> None: + await self._cache.close() + + self._run(_call) + + @staticmethod + def _run(coro_fn: Any, timeout: float = 30.0) -> None: + try: + loop = asyncio.get_running_loop() + future = asyncio.run_coroutine_threadsafe(coro_fn(), loop) + future.result(timeout=timeout) + except RuntimeError: + asyncio.run(asyncio.wait_for(coro_fn(), timeout=timeout)) diff --git a/robot/fake_registry_server.py b/robot/fake_registry_server.py index 12e7929..fa963e6 100644 --- a/robot/fake_registry_server.py +++ b/robot/fake_registry_server.py @@ -63,9 +63,66 @@ class FakeRegistryHandler(SimpleHTTPRequestHandler): "v1.0.0": "pkg_grh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", } + _PACKAGE_CONTENT: dict[str, dict[str, Any]] = { + "pkg_act_1111111111111111111111111111111111111111": { + "name": "test-actor", + "type": "actor", + "version": "v1.0.0", + "config": {"agent_type": "llm", "model": "gpt-4"}, + }, + "pkg_act_2222222222222222222222222222222222222222": { + "name": "test-actor", + "type": "actor", + "version": "v1.1.0", + "config": {"agent_type": "llm", "model": "gpt-4-turbo"}, + }, + "pkg_act_3333333333333333333333333333333333333333": { + "name": "test-actor", + "type": "actor", + "version": "v1.1.3", + "config": {"agent_type": "llm", "model": "gpt-4-turbo"}, + }, + "pkg_act_4444444444444444444444444444444444444444": { + "name": "test-actor", + "type": "actor", + "version": "v2.0.0", + "config": {"agent_type": "graph", "nodes": 5}, + }, + "pkg_act_5555555555555555555555555555555555555555": { + "name": "test-actor", + "type": "actor", + "version": "v2.0.1", + "config": {"agent_type": "graph", "nodes": 5}, + }, + "pkg_act_6666666666666666666666666666666666666666": { + "name": "test-actor", + "type": "actor", + "version": "v3.0.0", + "config": {"agent_type": "composite", "agents": 3}, + }, + "pkg_grh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": { + "name": "test-graph", + "type": "graph", + "version": "v1.0.0", + "config": {"route_type": "graph"}, + }, + } + + _TYPE_ALIAS_MAP: dict[str, str] = { + "act": "actor", + "grh": "graph", + "str": "stream", + "agt": "agent", + "tpl": "template", + "skl": "skill", + } + @classmethod def _resolve_version(cls, version: str, pkg_type: str) -> str: - versions = cls._ACTOR_VERSIONS if pkg_type == "actor" else cls._GRAPH_VERSIONS + resolved_type = cls._TYPE_ALIAS_MAP.get(pkg_type, pkg_type) + versions = ( + cls._ACTOR_VERSIONS if resolved_type in ("actor",) else cls._GRAPH_VERSIONS + ) def _semver_key(v: str) -> tuple[int, int, int]: m = re.match(r"^v(\d+)\.(\d+)\.(\d+)$", v) @@ -112,6 +169,19 @@ class FakeRegistryHandler(SimpleHTTPRequestHandler): def _version_not_found(cls, version: str) -> str: raise _VersionNotFound(version) + @classmethod + def _build_resolve_response( + cls, package_id: str, pkg_type: str, namespace: str, name: str + ) -> dict[str, Any]: + """Build a resolve response including full content for cache warming.""" + response: dict[str, Any] = { + "package_id": package_id, + "type": pkg_type, + } + if package_id in cls._PACKAGE_CONTENT: + response.update(cls._PACKAGE_CONTENT[package_id]) + return response + def do_GET(self) -> None: parsed = urlparse(self.path) path = parsed.path @@ -155,17 +225,30 @@ class FakeRegistryHandler(SimpleHTTPRequestHandler): "package_id": package_id, }, ) + elif package_id in self._PACKAGE_CONTENT: + data = dict(self._PACKAGE_CONTENT[package_id]) + data["package_id"] = package_id + self._json(200, data) else: self._json(404, {"message": "Package not found"}) - elif ( - path.startswith("/actor/") - or path.startswith("/graph/") - or path.startswith("/stream/") - or path.startswith("/agent/") - or path.startswith("/template/") - or path.startswith("/skill/") - or path.startswith("/mcp/") - or path.startswith("/lsp/") + elif any( + path.startswith(f"/{t}/") + for t in ( + "actor", + "act", + "graph", + "grh", + "stream", + "str", + "agent", + "agt", + "template", + "tpl", + "skill", + "skl", + "mcp", + "lsp", + ) ): parts = path.strip("/").split("/") if len(parts) >= 3: @@ -185,10 +268,9 @@ class FakeRegistryHandler(SimpleHTTPRequestHandler): return self._json( 200, - { - "package_id": resolved, - "type": parts[0], - }, + self._build_resolve_response( + resolved, parts[0], parts[1], parts[2] + ), ) else: self._json(404, {"message": "Not found"}) diff --git a/robot/registry_cache_integration.robot b/robot/registry_cache_integration.robot new file mode 100644 index 0000000..83dc709 --- /dev/null +++ b/robot/registry_cache_integration.robot @@ -0,0 +1,84 @@ +*** Settings *** +Documentation Integration tests for the RegistryCache against a fake server. +Library RegistryCacheLib.py +Library RegistryClientLib.py +Library Process +Library OperatingSystem +Suite Setup Start Fake Registry Server +Suite Teardown Stop Fake Registry Server + +*** Variables *** +${FAKE_SERVER_HOST} 127.0.0.1 +${FAKE_SERVER_PORT} ${9292} +${VALID_PKG_ID} pkg_act_a1b2c3d4e5f67890abcdef1234567890abcdefab +${OTHER_PKG_ID} pkg_tpl_1111111111111111111111111111111111111111 +${THIRD_PKG_ID} pkg_tpl_2222222222222222222222222222222222222222 +${NONEXIST_PKG_ID} pkg_act_deadbeef00000000000000000000000000000000 + +*** Keywords *** +Start Fake Registry Server + [Documentation] Launch a fake registry server as a subprocess. + Start Process + ... python robot${/}fake_registry_server.py ${FAKE_SERVER_HOST} ${FAKE_SERVER_PORT} + ... alias=fake_registry + Sleep 0.5s + ${server_url}= Set Variable http://${FAKE_SERVER_HOST}:${FAKE_SERVER_PORT} + Set Suite Variable ${SERVER_URL} ${server_url} + +Stop Fake Registry Server + [Documentation] Stop the fake server subprocess. + Terminate Process fake_registry kill=${True} + +*** Test Cases *** +Cache Hit Returns Stored Content Without HTTP Call + [Documentation] Second get_package call should return from cache (hit=1). + Create Registry Cache ${SERVER_URL} + Cache Get Package ${VALID_PKG_ID} + Cache Get Package ${VALID_PKG_ID} + Cache Stats Hits Equals 1 + Cache Stats Misses Equals 1 + +Cache Miss Increments On First Access + [Documentation] First get_package call counts as a miss. + Create Registry Cache ${SERVER_URL} + Cache Get Package ${VALID_PKG_ID} + Cache Stats Misses Equals 1 + Cache Stats Hits Equals 0 + +LRU Eviction Evicts Least Recently Used + [Documentation] When max_size=2 and 3 entries are loaded, the LRU is evicted. + Create Registry Cache With Config ${SERVER_URL} 2 300 + Cache Get Package ${VALID_PKG_ID} + Cache Get Package ${OTHER_PKG_ID} + Cache Stats Evictions Equals 0 + Cache Get Package ${THIRD_PKG_ID} + Cache Stats Evictions Equals 1 + Cache Should Not Contain ${VALID_PKG_ID} + Cache Should Contain ${OTHER_PKG_ID} + Cache Should Contain ${THIRD_PKG_ID} + +Cache Invalidate Removes Entry + [Documentation] Invalidating a cached entry returns True. + Create Registry Cache ${SERVER_URL} + Cache Get Package ${VALID_PKG_ID} + Cache Invalidate ${VALID_PKG_ID} + Invalidate Result Should Be True + +Cache Invalidate Missing Returns False + [Documentation] Invalidating a non-existent entry returns False. + Create Registry Cache ${SERVER_URL} + Cache Invalidate ${NONEXIST_PKG_ID} + Invalidate Result Should Be False + +Cache Clear Resets Statistics + [Documentation] clear() resets hits and misses to 0. + Create Registry Cache ${SERVER_URL} + Cache Get Package ${VALID_PKG_ID} + Cache Clear + Cache Stats Hits Equals 0 + Cache Stats Misses Equals 0 + +Cache Close Works Cleanly + [Documentation] close() shuts down the underlying client and cache. + Create Registry Cache ${SERVER_URL} + Close Cache diff --git a/robot/registry_full_cache_integration.robot b/robot/registry_full_cache_integration.robot new file mode 100644 index 0000000..815bb90 --- /dev/null +++ b/robot/registry_full_cache_integration.robot @@ -0,0 +1,128 @@ +*** Settings *** +Documentation Integration tests for the full registry + cache pipeline: +... PackageContentResolver with CacheFactory, transparent +... caching via RegistryCache wrapping resolve_package and +... get_package, observable via aggregated CacheStats. +Library Process +Library OperatingSystem +Library CleverActorsLib.py +Suite Setup Start Fake Registry Server +Suite Teardown Stop Fake Registry Server + +*** Variables *** +${FAKE_SERVER_HOST} 127.0.0.1 +${FAKE_SERVER_PORT} ${9393} +${SERVER_ALIAS} test.registry +${ACTOR_REF} ${SERVER_ALIAS}:example/test-actor@v1.1.0 +${GRAPH_REF} ${SERVER_ALIAS}:example/test-graph@v1.0.0 +${BAD_REF} ${SERVER_ALIAS}:example/nonexistent@v1.0.0 +${LATEST_ACTOR_REF} ${SERVER_ALIAS}:example/test-actor@latest +${ACTOR_V1_REF} ${SERVER_ALIAS}:example/test-actor@v1.0.0 + +*** Keywords *** +Start Fake Registry Server + [Documentation] Launch a fake registry server as a subprocess + ... on port ${FAKE_SERVER_PORT}. + Start Process + ... python robot${/}fake_registry_server.py ${FAKE_SERVER_HOST} ${FAKE_SERVER_PORT} + ... alias=fake_registry_full + Sleep 0.5s + ${server_url}= Set Variable http://${FAKE_SERVER_HOST}:${FAKE_SERVER_PORT} + Set Suite Variable ${FAKE_SERVER_URL} ${server_url} + +Stop Fake Registry Server + [Documentation] Stop the fake server subprocess. + Terminate Process fake_registry_full kill=${True} + +Setup Resolver With Cache And Alias + [Documentation] Create resolver with CacheFactory and map the + ... logical server name to the actual test URL. + Create Reference Resolver With Cache Factory 256 300.0 + Set Resolver Server Alias ${SERVER_ALIAS} ${FAKE_SERVER_URL} + +*** Test Cases *** +Resolve Warms Cache Transparently + [Documentation] Resolving a registry reference through the + ... resolver automatically caches the result in + ... the per-server RegistryCache. Stats reflect + ... the miss on first call and hit on second. + [Tags] transparent-caching stats + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolved Has Key package_id + Resolved Has Key name + Resolver Total Stats Misses Equals 1 + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolver Total Stats Misses Equals 1 + +Resolution Cache Serves Repeat Lookups + [Documentation] The resolution cache serves repeated resolve() + ... calls for the same reference without incrementing + ... content cache stats. + [Tags] transparent-caching resolution-cache + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolved Has Key package_id + Resolver Total Stats Misses Equals 1 + +Content Cache Is Shared Across References To Same Server + [Documentation] Both actor and graph references resolve through + ... the same per-server RegistryCache. Total misses + ... should equal 2 after resolving two different + ... references. + [Tags] transparent-caching shared-cache + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolve Reference And Collect Stats ${GRAPH_REF} graph + Resolver Total Stats Misses Equals 2 + +Unresolved Package Returns None + [Documentation] A reference for a non-existent package returns + ... None without affecting cache stats. + [Tags] transparent-caching error-handling + Setup Resolver With Cache And Alias + Resolve Should Return None ${BAD_REF} + +Clear Cache Removes Resolution And Content Entries + [Documentation] clear_cache() removes entries from both the + ... resolution cache and all content caches. + [Tags] transparent-caching cache-clear + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolver Clear All Caches + Resolver Resolution Cache Is Empty + +Close All Closes Clients And Content Caches + [Documentation] close_all() closes all RegistryClient instances + ... and clears both the client pool and content cache + ... map. + [Tags] transparent-caching resource-lifecycle + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Create Ref Resolver Client For Server ${FAKE_SERVER_URL} + Resolver Client Pool Has Server ${FAKE_SERVER_URL} + Resolver Close All Resources + +Default Resolver Without CacheFactory Still Works + [Documentation] Creating a PackageContentResolver without a + ... CacheFactory should still resolve references + ... normally (backward compatibility). + [Tags] transparent-caching backward-compat + Create Reference Resolver + Set Resolver Server Alias ${SERVER_ALIAS} ${FAKE_SERVER_URL} + Resolve Reference And Collect Stats ${ACTOR_REF} actor + Resolved Has Key package_id + +Version Aliases Resolve Independently + [Documentation] Resolving the same package via different version + ... aliases produces independent cache entries because + ... each alias resolves to a distinct package_id. + [Tags] transparent-caching version-aliases + Setup Resolver With Cache And Alias + Resolve Reference And Collect Stats ${ACTOR_V1_REF} + Resolved Has Key package_id + Resolver Total Stats Misses Equals 1 + Resolve Reference And Collect Stats ${LATEST_ACTOR_REF} + Resolved Has Key package_id + Resolver Total Stats Misses Equals 2 diff --git a/src/cleveractors/core/application.py b/src/cleveractors/core/application.py index 6e52ebf..d885bee 100644 --- a/src/cleveractors/core/application.py +++ b/src/cleveractors/core/application.py @@ -118,6 +118,7 @@ class ReactiveCleverAgentsApp: # pylint: disable=too-many-instance-attributes self.unsafe = unsafe self.verbose = verbose self.temperature_override = temperature_override + self.single_shot_timeout: float = 30.0 # Initialize reactive components # Note: self.loop and scheduler will be set when needed @@ -391,9 +392,9 @@ class ReactiveCleverAgentsApp: # pylint: disable=too-many-instance-attributes # Wait for completion (same timeout as interactive mode) try: - await asyncio.wait_for(completion_future, timeout=30.0) - - # Check for global context updates from tools + await asyncio.wait_for( + completion_future, timeout=self.single_shot_timeout + ) from cleveractors.agents.tool import _CONTEXT_UPDATES if _CONTEXT_UPDATES: @@ -758,7 +759,7 @@ class ReactiveCleverAgentsApp: # pylint: disable=too-many-instance-attributes # Wait for actual completion (not a guess!) try: await asyncio.wait_for( - completion_future, timeout=30.0 + completion_future, timeout=self.single_shot_timeout ) # Safety timeout # Save assistant response to context if context manager provided diff --git a/src/cleveractors/registry/__init__.py b/src/cleveractors/registry/__init__.py index 596b711..0044d14 100644 --- a/src/cleveractors/registry/__init__.py +++ b/src/cleveractors/registry/__init__.py @@ -1,5 +1,6 @@ """Package Registry client and core types (Package Registry Standard v1.0.0).""" +from cleveractors.registry.cache import CacheFactory, CacheStats, RegistryCache from cleveractors.registry.canonical import Canonicalizer from cleveractors.registry.client import RegistryClient from cleveractors.registry.exceptions import ( @@ -31,27 +32,30 @@ from cleveractors.registry.types import ( ) __all__ = [ - "Canonicalizer", - "PackageContentResolver", - "ReferenceResolver", - "RegistryClient", - "RegistryError", - "PackageNotFoundError", - "InvalidPackageIdError", - "InvalidPackageReferenceError", - "VersionNotFoundError", - "ValidationError", - "AuthenticationRequiredError", "AccessDeniedError", + "AuthenticationRequiredError", + "CacheFactory", + "CacheStats", + "Canonicalizer", "ConflictError", "InternalServerError", - "RegistryNetworkError", + "InvalidPackageIdError", + "InvalidPackageReferenceError", "PackageContent", + "PackageContentResolver", "PackageId", + "PackageNotFoundError", "PackageReference", "PackageType", + "ReferenceResolver", "ReferenceType", - "resolve_version", + "RegistryCache", + "RegistryClient", + "RegistryError", + "RegistryNetworkError", + "ValidationError", + "VersionNotFoundError", "is_concrete_version", "is_version_alias", + "resolve_version", ] diff --git a/src/cleveractors/registry/cache.py b/src/cleveractors/registry/cache.py new file mode 100644 index 0000000..e25dca8 --- /dev/null +++ b/src/cleveractors/registry/cache.py @@ -0,0 +1,509 @@ +"""Client-side LRU cache with TTL for the Package Registry Standard v1.0.0. + +Implements §10.3 client-side caching requirements: local storage of retrieved +packages, SHA-1 content validation, and TTL-based refresh. + +Provides a ``RegistryCache`` that wraps a ``RegistryClient`` as a transparent +caching layer with LRU eviction, configurable TTL, and exposure of cache +statistics for observability. + +Concurrent access is protected by an ``asyncio.Lock`` and a singleflight +(promise-cache) mechanism that coalesces concurrent misses for the same key +into a single upstream fetch, preventing thundering-herd cache stampedes. +""" + +from __future__ import annotations + +import asyncio +import logging +import time +from collections import OrderedDict +from dataclasses import dataclass +from types import TracebackType +from typing import Any + +from cleveractors.registry.canonical import Canonicalizer +from cleveractors.registry.client import RegistryClient +from cleveractors.registry.types import PackageContent, PackageId + +logger = logging.getLogger(__name__) + +_DEFAULT_MAX_SIZE: int = 256 +_DEFAULT_TTL: float = 300.0 + + +@dataclass +class CacheStats: + """Observable cache statistics.""" + + hits: int = 0 + misses: int = 0 + evictions: int = 0 + + def record_hit(self) -> None: + self.hits += 1 + + def record_miss(self) -> None: + self.misses += 1 + + def record_eviction(self) -> None: + self.evictions += 1 + + def reset(self) -> None: + self.hits = 0 + self.misses = 0 + self.evictions = 0 + + +class RegistryCache: + """LRU cache with TTL for RegistryClient responses. + + Wraps a ``RegistryClient`` to provide transparent client-side caching + of retrieved packages per §10.3 of the Package Registry Standard. + + Key behaviours: + - **LRU eviction**: when the cache exceeds ``max_size``, the + least-recently-used entry is evicted. + - **TTL expiration**: entries older than ``ttl`` seconds are + treated as stale and re-fetched. + - **Content validation**: cached content is validated by + recomputing the SHA-1 hash against the stored PackageId. + Mismatched hashes trigger a re-fetch (tamper detection). + - **Singleflight coalescing**: concurrent requests for the same + cold key are coalesced into a single upstream fetch, preventing + thundering-herd cache stampedes under concurrent load. + - **Thread safety**: all cache operations are protected by an + ``asyncio.Lock``, allowing safe concurrent access. + + Attributes: + stats: A ``CacheStats`` instance exposing hit/miss/eviction counters. + """ + + def __init__( + self, + client: RegistryClient, + *, + max_size: int = _DEFAULT_MAX_SIZE, + ttl: float = _DEFAULT_TTL, + validate_content: bool = True, + ) -> None: + if not isinstance(client, RegistryClient): + raise TypeError( + f"client must be RegistryClient, got {type(client).__name__}" + ) + if not isinstance(max_size, int) or isinstance(max_size, bool) or max_size < 1: + raise ValueError(f"max_size must be a positive int, got {max_size!r}") + if not isinstance(ttl, (int, float)) or ttl <= 0: + raise ValueError(f"ttl must be a positive number, got {ttl!r}") + + self._client = client + self._max_size = max_size + self._ttl = ttl + self._validate_content_enabled = validate_content + self._lock = asyncio.Lock() + self._store: OrderedDict[str, tuple[PackageContent, float]] = OrderedDict() + self._resolve_store: OrderedDict[ + tuple[str, str, str, str], tuple[dict[str, Any], float] + ] = OrderedDict() + self._in_flight: dict[str, asyncio.Future[dict[str, Any]]] = {} + self.stats = CacheStats() + self._canonicalizer = Canonicalizer() + + @property + def max_size(self) -> int: + return self._max_size + + @property + def ttl(self) -> float: + return self._ttl + + async def get_package(self, package_id: str) -> dict[str, Any]: + """Retrieve package content, using cache when available and valid. + + On a cache hit the stored content is validated by recomputing its + canonical SHA-1. If the hash matches and the entry is within TTL + the cached content is returned. Otherwise (stale, tampered, or + missing) a fresh copy is fetched from the upstream client. + + Concurrent callers that miss simultaneously for the same key are + coalesced into a single upstream fetch via a singleflight + mechanism, preventing thundering-herd cache stampedes. + + Args: + package_id: The globally unique Package ID string. + + Returns: + The package content as a dictionary. + + Raises: + InvalidPackageIdError: If *package_id* is not a valid + Package ID. + RegistryError: Subclass appropriate for the upstream response. + """ + PackageId.from_string(package_id) + + # Phase 1: check cache under lock, take validation snapshot + content_snapshot: PackageContent | None = None + snapshot_stored_at: float | None = None + + async with self._lock: + cached = self._store.get(package_id) + if cached is not None: + content, stored_at = cached + age = time.monotonic() - stored_at + if age <= self._ttl: + content_snapshot = content + snapshot_stored_at = stored_at + else: + del self._store[package_id] + logger.debug("TTL expired for %s", package_id) + + # Phase 2: validate outside lock (CPU-bound SHA-1 computation) + if content_snapshot is not None and self._validate_content(content_snapshot): + async with self._lock: + self.stats.record_hit() + self._touch(package_id) + logger.debug("Cache hit for %s", package_id) + return dict(content_snapshot.content) + + # Phase 3: cache miss, stale, or tampered + if content_snapshot is not None and snapshot_stored_at is not None: + async with self._lock: + current = self._store.get(package_id) + if current is not None and current[1] == snapshot_stored_at: + self._store.pop(package_id, None) + logger.debug("Tampered entry evicted for %s", package_id) + + return await self._singleflight_fetch(package_id) + + async def get_package_content(self, package_id: str) -> dict[str, Any]: + """Retrieve raw package content via cache (alias for get_package). + + Args: + package_id: The globally unique Package ID string. + + Returns: + The package content as a dictionary. + """ + return await self.get_package(package_id) + + async def resolve_package( + self, + package_type: str, + namespace: str, + name: str, + version: str = "latest", + ) -> dict[str, Any]: + """Resolve a package reference, using cache when available. + + Mirrors ``RegistryClient.resolve_package`` with transparent + caching: cache hits return the stored content; cache misses + fetch from the upstream client and populate the cache. + + Cached entries are evicted by LRU and TTL, each store independently + bounded by ``max_size``. + + Args: + package_type: One of the defined package types (§3.2). + namespace: The namespace owning the package. + name: The package name within its namespace. + version: Optional version string or alias. + + Returns: + A dict with ``package_id``, ``type`` and resolved content. + + Raises: + RegistryError: Subclass appropriate for the upstream response. + """ + cache_key = (package_type, namespace, name, version) + + async with self._lock: + cached = self._resolve_store.get(cache_key) + if cached is not None: + entry, stored_at = cached + if time.monotonic() - stored_at <= self._ttl: + self.stats.record_hit() + self._resolve_store.move_to_end(cache_key) + logger.debug( + "Resolve cache hit for %s:%s/%s@%s", + package_type, + namespace, + name, + version, + ) + return dict(entry) + del self._resolve_store[cache_key] + logger.debug( + "Resolve TTL expired for %s:%s/%s@%s", + package_type, + namespace, + name, + version, + ) + self.stats.record_miss() + logger.debug( + "Resolve cache miss for %s:%s/%s@%s", + package_type, + namespace, + name, + version, + ) + + result = await self._client.resolve_package( + package_type=package_type, + namespace=namespace, + name=name, + version=version, + ) + stored = dict(result) + async with self._lock: + self._resolve_store[cache_key] = (stored, time.monotonic()) + # Defensive loop: see _evict_if_needed docstring for rationale. + while len(self._resolve_store) > self._max_size: + self._resolve_store.popitem(last=False) + self.stats.record_eviction() + logger.debug( + "Resolve cached %s:%s/%s@%s (resolve store size=%d)", + package_type, + namespace, + name, + version, + len(self._resolve_store), + ) + + return dict(result) + + async def _singleflight_fetch(self, package_id: str) -> dict[str, Any]: + """Coalesce concurrent misses into a single upstream fetch.""" + async with self._lock: + inflight = self._in_flight.get(package_id) + if inflight is not None: + logger.debug("Joining in-flight fetch for %s", package_id) + future = inflight + else: + self.stats.record_miss() + logger.debug("Initiating fetch for %s (miss)", package_id) + task_future = asyncio.create_task(self._fetch(package_id)) + wait_future: asyncio.Future[dict[str, Any]] = ( + asyncio.get_running_loop().create_future() + ) + + def _on_done(t: asyncio.Task[dict[str, Any]]) -> None: + if t.cancelled(): + if not wait_future.done(): + wait_future.cancel() + else: + exc = t.exception() + if exc is not None: + if not wait_future.done(): + wait_future.set_exception(exc) + else: + if not wait_future.done(): + wait_future.set_result(t.result()) + self._in_flight.pop(package_id, None) + + task_future.add_done_callback(_on_done) + self._in_flight[package_id] = wait_future + future = wait_future + + return await asyncio.shield(future) + + async def _fetch(self, package_id: str) -> dict[str, Any]: + """Fetch from upstream client and populate the cache.""" + raw = await self._client.get_package(package_id) + + pid = PackageId.from_string(package_id) + stored = dict(raw) + content = PackageContent( + id=pid, + content=stored, + ) + + async with self._lock: + self._store[package_id] = (content, time.monotonic()) + self._evict_if_needed() + logger.debug( + "Fetched and cached %s (store size=%d)", package_id, len(self._store) + ) + + return dict(raw) + + def _touch(self, key: str) -> None: + """Move *key* to the most-recently-used end, if present.""" + if key in self._store: + self._store.move_to_end(key) + + def _evict_if_needed(self) -> None: + """Evict LRU entries while the store exceeds max_size. + + The ``while`` loop is defensive: callers currently insert at most + one entry at a time, but this protects against future multi-insert + or iteration patterns that could push the store beyond max_size. + """ + while len(self._store) > self._max_size: + key, _ = self._store.popitem(last=False) + self.stats.record_eviction() + logger.debug("LRU evicted %s", key) + + def _validate_content(self, content: PackageContent) -> bool: + """Recompute SHA-1 and compare against the stored PackageId.""" + if not self._validate_content_enabled: + return True + try: + computed = self._canonicalizer.compute_package_id( + content.content, content.id.package_type + ) + return computed.sha1_hex == content.id.sha1_hex + except (TypeError, ValueError): + logger.exception("SHA-1 validation failed for %s", content.id) + return False + + async def invalidate(self, package_id: str) -> bool: + """Remove a specific entry from the cache. + + Args: + package_id: The Package ID to remove. + + Returns: + ``True`` if the entry was present and removed, ``False`` otherwise. + + Raises: + InvalidPackageIdError: If *package_id* is not a valid + Package ID. + """ + PackageId.from_string(package_id) + + async with self._lock: + if package_id in self._store: + del self._store[package_id] + logger.debug("Invalidated %s", package_id) + return True + return False + + async def clear(self) -> None: + """Clear all entries from both stores and reset statistics.""" + async with self._lock: + self._store.clear() + self._resolve_store.clear() + self.stats.reset() + logger.debug("Cache cleared") + + async def put(self, package_id: str, content: dict[str, Any]) -> None: + """Insert content directly into the cache without an upstream fetch. + + Useful when content has been obtained through a different channel + (e.g. via ``resolve_package``) and the caller wishes to populate + the cache for subsequent ``get_package`` hits. + + Args: + package_id: The globally unique Package ID string. + content: The package content dictionary to cache. + + Raises: + InvalidPackageIdError: If *package_id* is not a valid + Package ID. + """ + pid = PackageId.from_string(package_id) + stored = dict(content) + entry = PackageContent(id=pid, content=stored) + + async with self._lock: + self._store[package_id] = (entry, time.monotonic()) + self._evict_if_needed() + logger.debug( + "Manual insert %s (store size=%d)", package_id, len(self._store) + ) + + async def close(self) -> None: + """Clear all caches, reset statistics, and close the underlying client. + + The underlying ``httpx.AsyncClient`` is torn down which may cause + in-flight fetches to fail with a transport error. Statistics are + reset alongside the store clear — use ``CacheStats`` snapshots + before ``close()`` when post-mortem observability is needed. + """ + async with self._lock: + self._store.clear() + self._resolve_store.clear() + self.stats.reset() + await self._client.close() + logger.debug("Cache closed") + + async def __aenter__(self) -> RegistryCache: + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> None: + await self.close() + + def __contains__(self, package_id: str) -> bool: + """Return ``True`` if *package_id* is present in the cache store. + + Note: returns ``True`` for expired entries; use ``get_package`` + to check freshness. This method performs a raw membership test + that does not consult TTL. + """ + return package_id in self._store + + +class CacheFactory: + """Factory for creating ``RegistryCache`` instances. + + Encapsulates cache configuration so that cache-creation knowledge + is centralised at a single point and the choice of cache + implementation is isolated from consumers. + + The **Factory Method** pattern decouples the construction of a + cache from its use. ``PackageContentResolver`` and other + components depend on the factory abstraction rather than + constructing ``RegistryCache`` directly. This allows the factory + to select between alternative implementations in the future (e.g. + disk-backed cache, distributed cache) without changing consumer + code. + + **Dependency Injection** is used at the consumer level: the + factory is passed as a constructor argument so that different + configurations can be supplied for testing without altering + production code. + + Usage:: + + factory = CacheFactory(max_size=100, ttl=300.0) + cache = factory.create(client) + content = await cache.get_package(package_id) + """ + + def __init__( + self, + max_size: int = _DEFAULT_MAX_SIZE, + ttl: float = _DEFAULT_TTL, + validate_content: bool = True, + ) -> None: + if not isinstance(max_size, int) or isinstance(max_size, bool) or max_size < 1: + raise ValueError(f"max_size must be a positive int, got {max_size!r}") + if not isinstance(ttl, (int, float)) or ttl <= 0: + raise ValueError(f"ttl must be a positive number, got {ttl!r}") + + self.max_size = max_size + self.ttl = ttl + self.validate_content = validate_content + + def create(self, client: RegistryClient) -> RegistryCache: + """Create a ``RegistryCache`` wrapping *client*. + + Args: + client: The ``RegistryClient`` to cache responses for. + + Returns: + A configured ``RegistryCache`` instance. + """ + return RegistryCache( + client, + max_size=self.max_size, + ttl=self.ttl, + validate_content=self.validate_content, + ) diff --git a/src/cleveractors/registry/reference_resolver.py b/src/cleveractors/registry/reference_resolver.py index 3a8b123..89a5005 100644 --- a/src/cleveractors/registry/reference_resolver.py +++ b/src/cleveractors/registry/reference_resolver.py @@ -1,7 +1,9 @@ """Reference resolver with multi-server RegistryClient pool. Resolves PackageReference objects to content dictionaries by maintaining -one RegistryClient per server URL, with caching of fetched packages. +one RegistryClient per server URL, with two-tier caching of fetched packages: +a resolution-level cache (reference → enriched content) and an optional +content-level cache (package_id → raw content) via RegistryCache. """ from __future__ import annotations @@ -13,6 +15,7 @@ import threading from collections import OrderedDict from typing import Any, Optional +from cleveractors.registry.cache import CacheFactory, CacheStats, RegistryCache from cleveractors.registry.client import RegistryClient from cleveractors.registry.types import PackageReference, ReferenceType @@ -39,6 +42,23 @@ class PackageContentResolver: Maintains one ``RegistryClient`` per server URL, reusing connections. Caches fetched package content by reference string and package type. + **Transparent caching architecture:** + + When a ``CacheFactory`` is provided, every ``RegistryClient`` is wrapped + by a ``RegistryCache`` that intercepts both ``get_package`` and + ``resolve_package`` calls transparently. This means: + + - The resolution cache (``self.cache`` — an ``OrderedDict`` keyed by + ``reference:type``) provides fast lookups for reference→content + mappings that have already been resolved. + - The per-server ``RegistryCache`` provides spec §10.3 content-level + caching for all client calls, with LRU eviction, TTL expiration, + SHA-1 validation, and singleflight coalescing. + + Consumers call ``resolve()`` / ``aresolve()`` and get content back + without ever seeing or managing cache internals. The ``total_stats`` + property surfaces aggregated observability data. + The cache is an LRU with a maximum of 128 entries. Cache operations are synchronised — ``threading.Lock`` guards synchronous code paths and ``asyncio.Lock`` guards asynchronous code paths. The two locks @@ -52,12 +72,23 @@ class PackageContentResolver: clients: Map of server URL to RegistryClient. cache: Cache of resolved content dicts keyed by reference+type. api_key: Optional API key for authenticated operations. + cache_factory: Factory for creating per-server RegistryCache + instances. When ``None`` the content cache tier is + disabled. + total_stats: Aggregated CacheStats across all content caches. """ - def __init__(self, api_key: Optional[str] = None) -> None: + def __init__( + self, + api_key: Optional[str] = None, + cache_factory: Optional[CacheFactory] = None, + ) -> None: self.clients: dict[str, RegistryClient] = {} self.cache: OrderedDict[str, dict[str, Any]] = OrderedDict() self.api_key: Optional[str] = api_key + self.cache_factory: Optional[CacheFactory] = cache_factory + self._content_caches: dict[str, RegistryCache] = {} + self._server_aliases: dict[str, str] = {} self._lock: threading.Lock = threading.Lock() self._async_lock: Optional[asyncio.Lock] = None @@ -66,12 +97,55 @@ class PackageContentResolver: self._async_lock = asyncio.Lock() return self._async_lock + @staticmethod + def _normalize_server(server: str) -> str: + if "://" not in server: + return f"http://{server}" + return server + + def set_server_alias(self, alias: str, resolved_url: str) -> None: + """Map a logical server name to an actual base URL. + + When a reference uses *alias* in its server position, + ``_get_client`` connects to *resolved_url* instead. + + Args: + alias: The server name that appears in package references + (e.g. ``\"test.registry\"``). + resolved_url: The actual base URL for the ``RegistryClient`` + (e.g. ``\"http://127.0.0.1:9393\"``). + """ + normalized = self._normalize_server(alias) + resolved = self._normalize_server(resolved_url) + self._server_aliases[normalized] = resolved + + async def _get_cache_for_resolved(self, resolved_key: str) -> RegistryCache | None: + """Return the content cache for *resolved_key*, creating it if needed. + + The *resolved_key* must already be a normalised, alias-resolved key + and the corresponding client must already exist in ``self.clients``. + + Acquires the async lock around the check-and-create to prevent + duplicate ``RegistryCache`` instances from concurrent callers. + """ + if self.cache_factory is None: + return None + async with self._get_async_lock(): + if resolved_key not in self._content_caches: + client = self.clients.get(resolved_key) + if client is None: + return None + self._content_caches[resolved_key] = self.cache_factory.create(client) + return self._content_caches[resolved_key] + async def _get_client(self, server: str) -> RegistryClient: async with self._get_async_lock(): - if server in self.clients: - return self.clients[server] - client = RegistryClient(base_url=server, api_key=self.api_key) - self.clients[server] = client + key = self._normalize_server(server) + resolved_key = self._server_aliases.get(key, key) + if resolved_key in self.clients: + return self.clients[resolved_key] + client = RegistryClient(base_url=resolved_key, api_key=self.api_key) + self.clients[resolved_key] = client return client def resolve( @@ -274,15 +348,16 @@ class PackageContentResolver: ) -> dict[str, Any]: """Resolve a REGISTRY reference against a live registry server. - Fetches package content via ``RegistryClient.resolve_package``, - builds a content dict with trace metadata, and caches the result. + Uses the per-server ``RegistryCache`` transparently — the + cache intercepts both ``resolve_package`` and ``get_package`` + calls, so consumers never see cache internals. Args: - server: Registry server base URL. + server: Registry server hostname (from reference). namespace: Package namespace. name: Package name. version: Version string or alias. - cache_key: Normalised cache key. + cache_key: Normalised cache key for the resolution cache. package_type: Mapped package-type code (e.g. ``\"tpl\"``). original_reference: The verbatim original reference string. @@ -291,13 +366,28 @@ class PackageContentResolver: ``_namespace``, ``_name``, ``_version`` merged into the resolved content. """ - client = await self._get_client(server) - resolved = await client.resolve_package( - package_type=package_type, - namespace=namespace, - name=name, - version=version, - ) + key = self._normalize_server(server) + resolved_key = self._server_aliases.get(key, key) + await self._get_client(server) + cache = await self._get_cache_for_resolved(resolved_key) + if cache is not None: + resolved = await cache.resolve_package( + package_type=package_type, + namespace=namespace, + name=name, + version=version, + ) + else: + client = self.clients.get(resolved_key) + if client is None: + client = await self._get_client(server) + resolved = await client.resolve_package( + package_type=package_type, + namespace=namespace, + name=name, + version=version, + ) + content: dict[str, Any] = { "_original_reference": original_reference, "_server": server, @@ -307,18 +397,27 @@ class PackageContentResolver: } if isinstance(resolved, dict): content.update(resolved) + async with self._get_async_lock(): self._put_cache(cache_key, content) + return copy.deepcopy(content) async def close_all(self) -> None: - """Close all open RegistryClient connections. + """Close all open RegistryClient connections and content caches. - Closes each client under the async lock so that concurrent - ``_get_client()`` calls cannot create a new client while the - close operation is in progress. + Closes each client and its associated content cache under the + async lock so that concurrent ``_get_client()`` calls cannot + create a new client while the close operation is in progress. """ async with self._get_async_lock(): + for cache in self._content_caches.values(): + try: + await cache.close() + except Exception: + logger.debug("Error closing content cache", exc_info=True) + self._content_caches.clear() + clients = list(self.clients.values()) for client in clients: try: @@ -330,10 +429,29 @@ class PackageContentResolver: self.clients.clear() def clear_cache(self) -> None: - """Clear the resolution cache. + """Clear the resolution cache and all content caches. Only takes ``self._lock``. Callers that may run concurrently - with async paths should use ``aclear_cache()`` instead. + with async paths should call this from a coroutine via + ``asyncio.run(resolver.clear_cache())``. """ with self._lock: self.cache.clear() + for content_cache in self._content_caches.values(): + asyncio.run(content_cache.clear()) + + @property + def total_stats(self) -> CacheStats: + """Aggregated ``CacheStats`` across all per-server content caches. + + Returns a single ``CacheStats`` instance with hits, misses, and + evictions summed from every content cache. The resolution cache + (``OrderedDict``) is not instrumented — this property reflects + only the spec §10.3 content-cache tier. + """ + aggregated = CacheStats() + for cache in self._content_caches.values(): + aggregated.hits += cache.stats.hits + aggregated.misses += cache.stats.misses + aggregated.evictions += cache.stats.evictions + return aggregated