From c694e37db54bb3750691b5853f1587e0fa189e68 Mon Sep 17 00:00:00 2001 From: Sudeep Ganguly Date: Fri, 16 Oct 2026 20:41:37 +0530 Subject: [PATCH 1/2] feat(agent-memory): add thread registry and in-memory checkpoint support Introduces a thread registry for managing LangGraph agent thread lifecycle (create, get, list, update, delete) with TTL-based expiry, plus production HANA checkpoint support in create_checkpointer(). - create_thread_registry() factory auto-detects Agent Memory Service credentials, returning SAPAgentMemoryThreadRegistry (persistent) or InMemoryThreadRegistry (in-process fallback) - BaseThreadRegistry contract with ThreadRegistryEntry and ThreadPage models - threadDays retention field on the thread model - IAS ContextVar token flow forwarded transparently through the checkpointer and thread registry - Deprecate create_checkpointer(ttl_seconds=...) in favor of registry-owned TTL; the parameter still works but emits a DeprecationWarning and metric - Telemetry instrumentation for all thread registry operations - Unit and integration tests, plus user-guide documentation --- pyproject.toml | 2 + src/sap_cloud_sdk/agent_memory/__init__.py | 58 +- src/sap_cloud_sdk/agent_memory/_models.py | 17 +- src/sap_cloud_sdk/agent_memory/client.py | 62 +- src/sap_cloud_sdk/agent_memory/config.py | 86 +- src/sap_cloud_sdk/agent_memory/exceptions.py | 25 + .../agent_memory/factory/__init__.py | 7 +- .../factory/langgraph_checkpoint.py | 58 +- .../agent_memory/thread_registry/__init__.py | 33 + .../agent_memory/thread_registry/_base.py | 162 ++++ .../agent_memory/thread_registry/_factory.py | 61 ++ .../thread_registry/_in_memory.py | 256 ++++++ .../agent_memory/thread_registry/_service.py | 457 ++++++++++ src/sap_cloud_sdk/agent_memory/user-guide.md | 521 ++++++++---- .../core/protocol/http/__init__.py | 2 + .../core/protocol/http/models.py | 40 + .../core/runtime_context/__init__.py | 3 + .../runtime_context/providers/__init__.py | 2 + .../core/runtime_context/providers/_ias.py | 20 +- .../core/secret_resolver/resolver.py | 53 +- src/sap_cloud_sdk/core/telemetry/operation.py | 10 + tests/agent_memory/integration/conftest.py | 93 +- .../integration/test_agentmemory_bdd.py | 69 +- .../integration/test_thread_registry_bdd.py | 246 ++++++ .../integration/thread_registry.feature | 47 ++ tests/agent_memory/unit/conftest.py | 39 + tests/agent_memory/unit/test_client.py | 481 ++++++++--- tests/agent_memory/unit/test_config.py | 276 ++++-- tests/agent_memory/unit/test_exceptions.py | 4 - .../unit/test_langgraph_checkpointer.py | 47 +- tests/agent_memory/unit/test_models.py | 93 +- tests/agent_memory/unit/test_odata.py | 30 +- .../unit/test_thread_registry_factory.py | 79 ++ .../unit/test_thread_registry_in_memory.py | 384 +++++++++ .../unit/test_thread_registry_service.py | 794 ++++++++++++++++++ tests/agent_memory/unit/test_timed_memory.py | 57 +- .../runtime_context/test_runtime_context.py | 35 +- .../unit/_future_annotations_fixture.py | 28 + .../unit/test_secret_resolver.py | 192 +++++ tests/core/unit/telemetry/test_operation.py | 6 +- uv.lock | 2 +- 41 files changed, 4418 insertions(+), 519 deletions(-) create mode 100644 src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py create mode 100644 src/sap_cloud_sdk/agent_memory/thread_registry/_base.py create mode 100644 src/sap_cloud_sdk/agent_memory/thread_registry/_factory.py create mode 100644 src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py create mode 100644 src/sap_cloud_sdk/agent_memory/thread_registry/_service.py create mode 100644 tests/agent_memory/integration/test_thread_registry_bdd.py create mode 100644 tests/agent_memory/integration/thread_registry.feature create mode 100644 tests/agent_memory/unit/conftest.py create mode 100644 tests/agent_memory/unit/test_thread_registry_factory.py create mode 100644 tests/agent_memory/unit/test_thread_registry_in_memory.py create mode 100644 tests/agent_memory/unit/test_thread_registry_service.py create mode 100644 tests/core/unit/secret_resolver/unit/_future_annotations_fixture.py diff --git a/pyproject.toml b/pyproject.toml index 841b66c0..d8344cd9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,6 +90,7 @@ dev = [ markers = [ "integration: marks tests as integration tests (requires Docker)", "aicore: marks tests that require AI Core credentials (AICORE_* env vars)", + "no_autouse_credentials: opts out of the default no-credentials fixture in checkpointer tests", ] [tool.coverage.run] @@ -148,3 +149,4 @@ exclude = [ "src/sap_cloud_sdk/core/auditlog_ng/gen", "src/buf", ] + diff --git a/src/sap_cloud_sdk/agent_memory/__init__.py b/src/sap_cloud_sdk/agent_memory/__init__.py index ee0e89b7..eb26f78a 100644 --- a/src/sap_cloud_sdk/agent_memory/__init__.py +++ b/src/sap_cloud_sdk/agent_memory/__init__.py @@ -27,6 +27,9 @@ AgentMemoryError, AgentMemoryHttpError, AgentMemoryNotFoundError, + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, + AgentMemoryThreadNotFoundError, AgentMemoryValidationError, ) from sap_cloud_sdk.agent_memory._models import ( @@ -38,19 +41,34 @@ SearchResult, ) from sap_cloud_sdk.agent_memory.utils._odata import FilterDefinition -from sap_cloud_sdk.core.protocol.http import HttpClient, XsuaaAuthProvider +from sap_cloud_sdk.agent_memory.thread_registry import ( + BaseThreadRegistry, + InMemoryThreadRegistry, + SAPAgentMemoryThreadRegistry, + ThreadPage, + ThreadRegistryEntry, + create_thread_registry, +) +from sap_cloud_sdk.core.protocol.http import ( + HttpClient, + IasAuthProvider, + XsuaaAuthProvider, +) def _build_agent_memory_http( instance: str, config: Optional[AgentMemoryConfig] -) -> HttpClient: - if config is not None: - auth = XsuaaAuthProvider(lambda: config) if config.token_url else None - return HttpClient(config.base_url, auth, timeout=config.timeout) - factory = _make_config_factory(instance) - cfg = factory() - auth = XsuaaAuthProvider(factory) if cfg.token_url else None - return HttpClient(cfg.base_url, auth, timeout=cfg.timeout) +) -> tuple[HttpClient, bool]: + """Build an :class:`HttpClient` for the given config, returning (client, is_ias).""" + # Explicit config is wrapped as a static factory; otherwise the binding is read + # via a ConfigFactory that re-reads on each call to pick up secret rotation. + config_factory = ( + (lambda: config) if config is not None else _make_config_factory(instance) + ) + cfg = config_factory() + is_ias = cfg.auth_mode == "ias" + auth = IasAuthProvider() if is_ias else XsuaaAuthProvider(config_factory) + return HttpClient(cfg.base_url, auth, timeout=cfg.timeout), is_ias def create_client( @@ -68,6 +86,10 @@ def create_client( ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*`` env vars) and derives the subscriber token URL using the ``identityzone`` field. - ``PROVIDER`` — same binding, uses provider token directly. + - **IAS binding** (URL-only, no ``token_url``) — detected from the config: + ``access_strategy`` and ``tenant`` are ignored, and the per-request Bearer + token is supplied via RuntimeContext (wired by ``bootstrap(app)`` on the + server, or set manually for scripts). - Explicit ``config`` — uses the provided configuration directly. Args: @@ -87,10 +109,11 @@ def create_client( ``tenant`` is not provided. """ try: - if config is not None: - http = _build_agent_memory_http("default", config) - else: - http = _build_agent_memory_http("default", None) + http, is_ias = _build_agent_memory_http("default", config) + if is_ias: + # IAS mode: calls are routed by the per-request token, not by an + # XSUAA access strategy — tenant/access_strategy do not apply. + return AgentMemoryClient(http, _ias_mode=True) return AgentMemoryClient( http, access_strategy=access_strategy, @@ -112,6 +135,9 @@ def create_client( "AgentMemoryConfigError", "AgentMemoryHttpError", "AgentMemoryNotFoundError", + "AgentMemoryThreadConflictError", + "AgentMemoryThreadExpiredError", + "AgentMemoryThreadNotFoundError", "AgentMemoryValidationError", "FilterDefinition", "Memory", @@ -119,5 +145,11 @@ def create_client( "MessageRole", "RetentionConfig", "SearchResult", + "BaseThreadRegistry", + "InMemoryThreadRegistry", + "SAPAgentMemoryThreadRegistry", + "ThreadPage", + "ThreadRegistryEntry", "create_client", + "create_thread_registry", ] diff --git a/src/sap_cloud_sdk/agent_memory/_models.py b/src/sap_cloud_sdk/agent_memory/_models.py index 0d9f0862..46c6f3d0 100644 --- a/src/sap_cloud_sdk/agent_memory/_models.py +++ b/src/sap_cloud_sdk/agent_memory/_models.py @@ -16,7 +16,11 @@ class AccessStrategy(str, Enum): - """Access strategy for tenant-scoped Agent Memory operations.""" + """Access strategy for XSUAA tenant-scoped Agent Memory operations. + + IAS bindings need no strategy: they are detected from a URL-only binding + (no ``token_url``) and the per-request token is supplied via the ContextVar. + """ SUBSCRIBER = "SUBSCRIBER" PROVIDER = "PROVIDER" @@ -224,6 +228,11 @@ class RetentionConfig: message_days: How long to keep messages (days). ``None`` disables cleanup. memory_days: How long to keep memories without access (days). ``None`` disables cleanup. usage_log_days: How long to keep access and search logs (days). ``None`` disables cleanup. + thread_days: Default TTL (days) applied only to registered threads (those created + via the Thread Registry API). Age-based cutoff for thread cleanup; unregistered + threads are unaffected. ``None`` disables age-based cleanup. + max_checkpoints_per_thread: Keep at most N checkpoints per ``(thread, scope)``, + deleting oldest first. ``None`` disables count-based cleanup. create_timestamp: ISO-8601 creation timestamp (read-only). update_timestamp: ISO-8601 last-update timestamp (read-only). """ @@ -232,6 +241,8 @@ class RetentionConfig: message_days: Optional[int] = None memory_days: Optional[int] = None usage_log_days: Optional[int] = None + thread_days: Optional[int] = None + max_checkpoints_per_thread: Optional[int] = None create_timestamp: Optional[str] = None update_timestamp: Optional[str] = None @@ -243,6 +254,8 @@ def from_dict(cls, data: dict[str, Any]) -> RetentionConfig: message_days=data.get("messageDays"), memory_days=data.get("memoryDays"), usage_log_days=data.get("usageLogDays"), + thread_days=data.get("threadDays"), + max_checkpoints_per_thread=data.get("maxCheckpointsPerThread"), create_timestamp=data.get("createTimestamp"), update_timestamp=data.get("updateTimestamp"), ) @@ -254,6 +267,8 @@ def to_dict(self) -> dict[str, Any]: "messageDays": self.message_days, "memoryDays": self.memory_days, "usageLogDays": self.usage_log_days, + "threadDays": self.thread_days, + "maxCheckpointsPerThread": self.max_checkpoints_per_thread, "createTimestamp": self.create_timestamp, "updateTimestamp": self.update_timestamp, } diff --git a/src/sap_cloud_sdk/agent_memory/client.py b/src/sap_cloud_sdk/agent_memory/client.py index 2ab7d4f7..629352cb 100644 --- a/src/sap_cloud_sdk/agent_memory/client.py +++ b/src/sap_cloud_sdk/agent_memory/client.py @@ -59,6 +59,7 @@ _MESSAGE_DAYS_KEY = "messageDays" _MEMORY_DAYS_KEY = "memoryDays" _USAGE_LOG_DAYS_KEY = "usageLogDays" +_THREAD_DAYS_KEY = "threadDays" def _require_non_empty(**fields: str) -> None: @@ -71,6 +72,25 @@ def _require_non_empty(**fields: str) -> None: ) +def _require_ias_token() -> None: + """Raise AgentMemoryValidationError if no IAS token is in the RuntimeContext. + + In IAS mode the per-request bearer token must be placed in the + RuntimeContext by the IAS context provider (wired via ``bootstrap(app)``). + Checking here — before the request is dispatched — surfaces a clear + ``AgentMemoryValidationError``; the HTTP layer would otherwise send an + unauthenticated request and the service would reject it with a 401. + """ + from sap_cloud_sdk.core.runtime_context import get_context + from sap_cloud_sdk.core.runtime_context.providers._ias import IAS_TOKEN + + if not get_context().get(IAS_TOKEN): + raise AgentMemoryValidationError( + "IAS token not found in RuntimeContext. " + "Ensure bootstrap(app) is configured." + ) + + def _validate_filter_clauses( clauses: list[FilterDefinition], allowed_targets: set[str] ) -> None: @@ -96,7 +116,7 @@ class AgentMemoryClient: Do not instantiate directly — use :func:`sap_cloud_sdk.agent_memory.create_client`. Args: - transport: HTTP transport loaded from the default service binding + http: HTTP client loaded from the default service binding (resolved once at construction time by :func:`sap_cloud_sdk.agent_memory.create_client`). access_strategy: Tenant access strategy for all operations. @@ -112,7 +132,15 @@ def __init__( *, access_strategy: AccessStrategy = AccessStrategy.SUBSCRIBER, tenant: Optional[str] = None, + _ias_mode: bool = False, ) -> None: + if _ias_mode: + # IAS mode: calls are routed by the per-request token, so the XSUAA + # access_strategy/tenant parameters do not apply and are not validated. + self._http = http + self._ias_mode = True + self._tenant = None + return if access_strategy is AccessStrategy.SUBSCRIBER and not tenant: raise AgentMemoryValidationError( "tenant is required when access_strategy=SUBSCRIBER" @@ -124,6 +152,7 @@ def __init__( "Only use this strategy for provider-owned operations." ) self._http = http + self._ias_mode = False self._tenant = tenant if access_strategy is AccessStrategy.SUBSCRIBER else None def close(self) -> None: @@ -146,6 +175,8 @@ def _request( **kwargs: Any, ) -> dict[str, Any]: logger.debug("%s %s (tenant=%r)", method.value, path, tenant_subdomain) + if self._ias_mode: + _require_ias_token() if params: path = f"{path}?{urlencode(params, quote_via=quote)}" try: @@ -164,6 +195,8 @@ def _request( raise AgentMemoryHttpError( f"Request failed: {method.value} {path} — {exc}" ) from exc + except AgentMemoryValidationError: + raise except Exception as exc: raise AgentMemoryHttpError(str(exc)) from exc if response.status_code == 204 or not response.content: @@ -605,6 +638,8 @@ def update_retention_config( message_days: Optional[int] = None, memory_days: Optional[int] = None, usage_log_days: Optional[int] = None, + thread_days: Optional[int] = None, + max_checkpoints_per_thread: Optional[int] = None, ) -> None: """Update the data retention configuration. @@ -616,21 +651,36 @@ def update_retention_config( message_days: How long to keep messages (days). memory_days: How long to keep memories without access (days). usage_log_days: How long to keep access and search logs (days). + thread_days: Default TTL (days) applied only to registered threads + (those created via the Thread Registry API). Age-based cutoff for + thread cleanup; unregistered threads are unaffected. ``None`` leaves + the server setting unchanged. ``0`` disables age-based cleanup. + max_checkpoints_per_thread: Keep at most N checkpoints per + ``(thread, scope)``, deleting oldest first. ``None`` leaves + the server setting unchanged. ``0`` disables count-based cleanup. Raises: AgentMemoryValidationError: If no fields are provided, any provided value is negative, or tenant is missing for ``SUBSCRIBER``. AgentMemoryHttpError: If the request fails. """ - if message_days is None and memory_days is None and usage_log_days is None: + if ( + message_days is None + and memory_days is None + and usage_log_days is None + and thread_days is None + and max_checkpoints_per_thread is None + ): raise AgentMemoryValidationError( - "At least one of 'message_days', 'memory_days', or " - "'usage_log_days' must be provided" + "At least one of 'message_days', 'memory_days', 'usage_log_days', " + "'thread_days', or 'max_checkpoints_per_thread' must be provided" ) for name, value in ( ("message_days", message_days), ("memory_days", memory_days), ("usage_log_days", usage_log_days), + ("thread_days", thread_days), + ("max_checkpoints_per_thread", max_checkpoints_per_thread), ): if value is not None and value < 0: raise AgentMemoryValidationError(f"'{name}' must be >= 0") @@ -642,6 +692,10 @@ def update_retention_config( payload[_MEMORY_DAYS_KEY] = memory_days if usage_log_days is not None: payload[_USAGE_LOG_DAYS_KEY] = usage_log_days + if thread_days is not None: + payload[_THREAD_DAYS_KEY] = thread_days + if max_checkpoints_per_thread is not None: + payload["maxCheckpointsPerThread"] = max_checkpoints_per_thread self._request( HttpMethod.PATCH, diff --git a/src/sap_cloud_sdk/agent_memory/config.py b/src/sap_cloud_sdk/agent_memory/config.py index bd454dc3..8ab4ff74 100644 --- a/src/sap_cloud_sdk/agent_memory/config.py +++ b/src/sap_cloud_sdk/agent_memory/config.py @@ -20,7 +20,7 @@ import json from dataclasses import dataclass -from typing import Optional, TYPE_CHECKING +from typing import Literal, Optional, TYPE_CHECKING from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError @@ -45,6 +45,11 @@ class AgentMemoryConfig: can be derived by replacing this value in ``token_url``. timeout: Timeout in seconds for HTTP requests. Default is 30.0. + The authentication mode is exposed as the derived :pyattr:`auth_mode` + property (``"xsuaa"`` or ``"ias"``) rather than a stored field: it is + computed from the credentials so it can never contradict them, and callers + dispatch on ``config.auth_mode`` instead of inspecting ``token_url``. + Example — deployed BTP service:: config = AgentMemoryConfig( @@ -86,6 +91,38 @@ def __post_init__(self) -> None: raise AgentMemoryConfigError( "identityzone must be a non-empty string when provided" ) + xsuaa_creds = (self.token_url, self.client_id, self.client_secret) + if any(xsuaa_creds) and not all(xsuaa_creds): + missing = [ + name + for name, value in ( + ("token_url", self.token_url), + ("client_id", self.client_id), + ("client_secret", self.client_secret), + ) + if not value + ] + raise AgentMemoryConfigError( + "Partial XSUAA credentials provided — all three (token_url, client_id, " + "client_secret) must be set together. Missing: " + ", ".join(missing) + ) + if self.identityzone and not self.token_url: + raise AgentMemoryConfigError( + "identityzone applies only to XSUAA (it derives the subscriber token URL); " + "it must not be set without token_url/client_id/client_secret." + ) + + @property + def auth_mode(self) -> Literal["ias", "xsuaa"]: + """Authentication mode derived from the configured credentials. + + ``"xsuaa"`` when OAuth2 credentials are present (``token_url`` set, which + ``__post_init__`` guarantees implies ``client_id``/``client_secret`` too), + ``"ias"`` otherwise. This is the single source of truth for auth dispatch — + callers branch on ``config.auth_mode`` instead of inspecting ``token_url``. + Computed, never stored, so it can never contradict the credentials. + """ + return "xsuaa" if self.token_url else "ias" @dataclass @@ -96,7 +133,8 @@ class BindingData: """ application_url: str = "" - uaa: str = "" + # uaa absent → IAS mode: token provided per-request via ContextVar, no OAuth2 needed. + uaa: Optional[str] = None def validate(self) -> None: """Raise ``AgentMemoryConfigError`` if any required field is empty.""" @@ -104,13 +142,18 @@ def validate(self) -> None: raise AgentMemoryConfigError( "Agent Memory binding is missing required field: application_url" ) - if not self.uaa: - raise AgentMemoryConfigError( - "Agent Memory binding is missing required field: uaa" - ) + # uaa is optional — absence means IAS mode (token provided per-request via ContextVar). def extract_config(self) -> AgentMemoryConfig: - """Parse the UAA JSON string and return an ``AgentMemoryConfig``.""" + """Parse the UAA JSON string and return an ``AgentMemoryConfig``. + + When ``uaa`` is absent (URL-only IAS binding), returns a config with + only ``base_url`` set. OAuth2 credentials are not required in IAS mode + because the token is provided per-request via the RuntimeContext. + """ + if not self.uaa: + return AgentMemoryConfig(base_url=self.application_url) + try: uaa_data = json.loads(self.uaa, strict=False) except json.JSONDecodeError as e: @@ -128,7 +171,7 @@ def extract_config(self) -> AgentMemoryConfig: raise AgentMemoryConfigError(f"Missing required field in uaa JSON: {e}") -def _load_config_from_env() -> AgentMemoryConfig: +def _load_config_from_env() -> Optional[AgentMemoryConfig]: """Load Agent Memory configuration from a mounted volume or environment variables. Uses the secret resolver with fallback order: @@ -136,13 +179,34 @@ def _load_config_from_env() -> AgentMemoryConfig: 2. Environment variables ``CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*`` Returns: - A validated ``AgentMemoryConfig``. + A validated ``AgentMemoryConfig``, or ``None`` if no binding is found. Raises: - AgentMemoryConfigError: If configuration cannot be loaded or is incomplete. + AgentMemoryConfigError: If a binding is found but is incomplete or malformed. """ + from sap_cloud_sdk.core.secret_resolver import ( + read_from_mount_and_fallback_to_env_var, + ) + + binding = BindingData() + try: + read_from_mount_and_fallback_to_env_var( + base_volume_mount="/etc/secrets/appfnd", + base_var_name="CLOUD_SDK_CFG", + module=_SERVICE_NAME, + instance="default", + target=binding, + ) + except RuntimeError: + return None + + # If neither field was filled, no binding is configured anywhere. + if not binding.application_url and not binding.uaa: + return None + + binding.validate() try: - return _make_config_factory("default")() + return binding.extract_config() except AgentMemoryConfigError: raise except Exception as exc: diff --git a/src/sap_cloud_sdk/agent_memory/exceptions.py b/src/sap_cloud_sdk/agent_memory/exceptions.py index e0ea2f42..d42aa9ca 100644 --- a/src/sap_cloud_sdk/agent_memory/exceptions.py +++ b/src/sap_cloud_sdk/agent_memory/exceptions.py @@ -37,3 +37,28 @@ class AgentMemoryNotFoundError(AgentMemoryHttpError): class AgentMemoryValidationError(AgentMemoryError): """Raised when client-side input validation fails before a request is sent.""" + + +class AgentMemoryThreadNotFoundError(AgentMemoryError): + """Raised when a thread_id does not exist in the thread registry. + + Both the in-memory and service-backed registries raise this type so callers + catch a single exception regardless of backend. + """ + + +class AgentMemoryThreadConflictError(AgentMemoryError): + """Raised when a thread_id is already registered by a different owner. + + "Owner" is the ``(agent_id, invoker_id)`` pair. The service-backed registry + maps the HTTP 409 ownership conflict onto this type. + """ + + +class AgentMemoryThreadExpiredError(AgentMemoryError): + """Raised when a thread_id belongs to an expired thread that has not yet been cleaned up. + + Raised on 409 responses whose body contains "expired", meaning the thread + row still exists in the DB but its TTL has passed. Re-registering with the + same ID is not allowed until the cleanup job removes the row. + """ diff --git a/src/sap_cloud_sdk/agent_memory/factory/__init__.py b/src/sap_cloud_sdk/agent_memory/factory/__init__.py index 525b1b2f..1269fc0a 100644 --- a/src/sap_cloud_sdk/agent_memory/factory/__init__.py +++ b/src/sap_cloud_sdk/agent_memory/factory/__init__.py @@ -6,9 +6,14 @@ Available factories: - :func:`sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint.create_checkpointer` - — returns a LangGraph ``BaseCheckpointSaver`` (short-term memory) + — returns a LangGraph ``BaseCheckpointSaver`` (short-term memory / graph state) + +- :func:`sap_cloud_sdk.agent_memory.thread_registry.create_thread_registry` + — returns a :class:`~sap_cloud_sdk.agent_memory.thread_registry.BaseThreadRegistry` + for thread lifecycle, ownership, and TTL management Usage: from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer + from sap_cloud_sdk.agent_memory.thread_registry import create_thread_registry """ diff --git a/src/sap_cloud_sdk/agent_memory/factory/langgraph_checkpoint.py b/src/sap_cloud_sdk/agent_memory/factory/langgraph_checkpoint.py index 379e6e97..cddc376c 100644 --- a/src/sap_cloud_sdk/agent_memory/factory/langgraph_checkpoint.py +++ b/src/sap_cloud_sdk/agent_memory/factory/langgraph_checkpoint.py @@ -7,8 +7,8 @@ # No TTL — plain InMemorySaver checkpointer = create_checkpointer() - # With TTL — TimedInMemorySaver evicts inactive threads automatically - checkpointer = create_checkpointer(ttl_seconds=3600) + # ttl_seconds is deprecated — use create_thread_registry(ttl_seconds=...) instead + checkpointer = create_checkpointer(ttl_seconds=3600) # deprecated app = workflow.compile(checkpointer=checkpointer) @@ -18,11 +18,31 @@ """ import logging +import warnings from typing import Optional +from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics + logger = logging.getLogger(__name__) +@record_metrics( + Module.AGENT_MEMORY, + Operation.AGENT_MEMORY_CREATE_CHECKPOINTER, + deprecated=True, +) +def _create_timed_checkpointer(ttl_seconds: int): + """Build the deprecated TTL-based checkpointer. + + Isolated into its own decorated helper so the ``deprecated=True`` metric is + recorded only for the deprecated ``ttl_seconds`` path — a bare + ``create_checkpointer()`` never reaches here and emits nothing. + """ + from sap_cloud_sdk.agent_memory.factory._timed_memory import TimedInMemorySaver + + return TimedInMemorySaver(ttl_seconds=ttl_seconds) + + def create_checkpointer(*, ttl_seconds: Optional[int] = None): """Create a LangGraph checkpointer for the current environment. @@ -34,6 +54,13 @@ def create_checkpointer(*, ttl_seconds: Optional[int] = None): ttl_seconds: Evict threads inactive for this many seconds. ``None`` (default) disables eviction. + .. deprecated:: + TTL is now managed at the thread level. Call + ``create_checkpointer()`` without ``ttl_seconds`` and + pass ``ttl_seconds`` to ``create_thread_registry()`` + instead. Threads that expire in the registry + automatically clean up their checkpoints. + Returns: BaseCheckpointSaver instance. @@ -52,24 +79,31 @@ def create_checkpointer(*, ttl_seconds: Optional[int] = None): """ try: from langgraph.checkpoint.memory import InMemorySaver - except ImportError: + except ImportError as exc: raise ImportError( "langgraph is required for create_checkpointer(). " "Install it with: pip install sap-cloud-sdk[langgraph] or pip install langgraph" - ) + ) from exc if ttl_seconds is not None: - from sap_cloud_sdk.agent_memory.factory._timed_memory import TimedInMemorySaver - + warnings.warn( + "The ttl_seconds parameter is deprecated. TTL is now managed at the thread level: " + "call create_checkpointer() without ttl_seconds, and pass ttl_seconds to " + "create_thread_registry() instead. Threads that expire in the registry will " + "automatically clean up their checkpoints.", + DeprecationWarning, + stacklevel=2, + ) logger.warning( - "create_checkpointer(): using TimedInMemorySaver(ttl_seconds=%d) — " - "session state is in-process only and will be lost on process exit.", + "create_checkpointer(ttl_seconds=%d): the ttl_seconds parameter is deprecated — " + "pass ttl_seconds to create_thread_registry() instead. " + "Threads that expire in the registry automatically clean up their checkpoints.", ttl_seconds, ) - return TimedInMemorySaver(ttl_seconds=ttl_seconds) + return _create_timed_checkpointer(ttl_seconds) - logger.warning( - "create_checkpointer(): using InMemorySaver — " - "session state is in-process only and will be lost on process exit." + logger.info( + "create_checkpointer(): using InMemorySaver — session state is " + "in-process only and will be lost on process exit." ) return InMemorySaver() diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py b/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py new file mode 100644 index 00000000..b500958d --- /dev/null +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py @@ -0,0 +1,33 @@ +"""Thread registry package for SAP Agent Memory. + +Public API:: + + from sap_cloud_sdk.agent_memory.thread_registry import ( + BaseThreadRegistry, + InMemoryThreadRegistry, + SAPAgentMemoryThreadRegistry, + ThreadPage, + ThreadRegistryEntry, + create_thread_registry, + ) +""" + +from sap_cloud_sdk.agent_memory.thread_registry._base import ( + BaseThreadRegistry, + ThreadPage, + ThreadRegistryEntry, +) +from sap_cloud_sdk.agent_memory.thread_registry._factory import create_thread_registry +from sap_cloud_sdk.agent_memory.thread_registry._in_memory import InMemoryThreadRegistry +from sap_cloud_sdk.agent_memory.thread_registry._service import ( + SAPAgentMemoryThreadRegistry, +) + +__all__ = [ + "BaseThreadRegistry", + "InMemoryThreadRegistry", + "SAPAgentMemoryThreadRegistry", + "ThreadPage", + "ThreadRegistryEntry", + "create_thread_registry", +] diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py new file mode 100644 index 00000000..40b2623a --- /dev/null +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py @@ -0,0 +1,162 @@ +"""Base interface and shared types for thread registries.""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from typing import Any, Optional + +from typing_extensions import TypedDict + + +def _validate_ttl(ttl_seconds: Optional[int]) -> None: + """Raise ValueError for any TTL that cannot represent a positive duration.""" + if ttl_seconds is not None and ( + not isinstance(ttl_seconds, int) + or isinstance(ttl_seconds, bool) + or ttl_seconds <= 0 + ): + raise ValueError(f"ttl_seconds must be a positive integer, got {ttl_seconds!r}") + + +class ThreadRegistryEntry(TypedDict): + """A single thread registry record returned by all registry operations.""" + + threadId: str + agentId: str + invokerId: str + description: Optional[str] + createdAt: Optional[str] + updatedAt: Optional[str] + expiresAt: Optional[str] + threadTtl: Optional[int] + metadata: Optional[dict[str, Any]] + + +class ThreadPage(TypedDict): + """Paginated result from :meth:`~BaseThreadRegistry.list_threads`. + + ``total`` reflects the full server-side count for the given filters + regardless of the page size:: + + page = registry.list_threads(agent_id="sales-agent") + if page["offset"] + len(page["entries"]) < page["total"]: + next_page = registry.list_threads( + agent_id="sales-agent", + limit=page["limit"], + offset=page["offset"] + page["limit"], + ) + """ + + entries: list[ThreadRegistryEntry] + total: int + limit: int + offset: int + + +class BaseThreadRegistry(ABC): + """Abstract base class for thread registry implementations. + + Analogous to ``BaseCheckpointSaver`` in the LangGraph ecosystem. + Both ``InMemoryThreadRegistry`` (local dev) and + ``SAPAgentMemoryThreadRegistry`` (service-backed) implement this interface. + + Subclasses must implement all five abstract methods. Missing implementations + raise ``TypeError`` at instantiation time. + + .. note:: Async interface + + If async support is introduced in a future version, async methods will + always arrive as **non-abstract concrete defaults** (never as + ``@abstractmethod``). Existing subclasses are therefore never required + to implement them, and will automatically benefit from a default + threadpool-backed implementation via ``asyncio.to_thread``. + """ + + @abstractmethod + def create_or_register_thread( + self, + agent_id: str, + invoker_id: str, + *, + thread_id: Optional[str] = None, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + """Create a thread (server-assigned ID) or register an existing one. + + When ``thread_id`` is omitted the server assigns a UUID and a new + thread is always created. + + When ``thread_id`` is supplied the call is idempotent for the same + ``(agent_id, invoker_id)`` owner. + + Raises: + AgentMemoryThreadConflictError: If the thread exists under a different owner. + AgentMemoryThreadExpiredError: If the thread exists but has expired. + """ + ... + + @abstractmethod + def get_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + """Fetch a single thread by ID. + + Raises: + AgentMemoryThreadNotFoundError: If the thread does not exist. + """ + ... + + @abstractmethod + def list_threads( + self, + *, + agent_id: Optional[str] = None, + invoker_id: Optional[str] = None, + limit: Optional[int] = None, + offset: Optional[int] = None, + tenant_id: Optional[str] = None, + ) -> ThreadPage: + """List threads with optional filters and pagination. + + When ``limit`` is ``None`` all matching threads are returned. + """ + ... + + @abstractmethod + def update_thread( + self, + thread_id: str, + *, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + """Update mutable fields of a thread. + + Only the fields you pass are modified; omitted fields are left + unchanged (PATCH-style merge, mirroring LangGraph's ``threads.update``). + + Pass ``ttl_seconds`` (a positive integer) to set the sliding TTL. Omit + it to leave the current TTL unchanged. + + Raises: + AgentMemoryThreadNotFoundError: If the thread does not exist. + """ + ... + + @abstractmethod + def delete_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> None: + """Delete a thread. Idempotent — a missing thread is a no-op.""" + ... diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_factory.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_factory.py new file mode 100644 index 00000000..7149c274 --- /dev/null +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_factory.py @@ -0,0 +1,61 @@ +"""Factory for creating a thread registry for the current environment.""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Optional + +if TYPE_CHECKING: + from sap_cloud_sdk.agent_memory.thread_registry._base import BaseThreadRegistry + +logger = logging.getLogger(__name__) + + +def create_thread_registry( + *, + ttl_seconds: Optional[int] = None, +) -> "BaseThreadRegistry": + """Create a thread registry for the current environment. + + Automatically detects whether Agent Memory Service credentials are available: + + - **Credentials present** — returns :class:`SAPAgentMemoryThreadRegistry` backed + by the Agent Memory Service (persistent across restarts). + - **No credentials** — falls back to :class:`InMemoryThreadRegistry`. State is + in-process only and will not survive restarts. + + Args: + ttl_seconds: Registry-level default inactivity threshold in seconds. + Threads inactive for longer than this are treated as expired. The window + slides — it resets on each write. Applied per-thread when the caller does + not pass an explicit ``ttl_seconds`` per operation. ``None`` = never + expire. + + Returns: + A :class:`~sap_cloud_sdk.agent_memory.thread_registry.BaseThreadRegistry` instance. + """ + from sap_cloud_sdk.agent_memory.config import _load_config_from_env + from sap_cloud_sdk.agent_memory.thread_registry._in_memory import ( + InMemoryThreadRegistry, + ) + from sap_cloud_sdk.agent_memory.thread_registry._service import ( + SAPAgentMemoryThreadRegistry, + ) + + config = _load_config_from_env() + + if config is not None: + logger.info("create_thread_registry(): using SAPAgentMemoryThreadRegistry.") + return SAPAgentMemoryThreadRegistry( + config, + default_ttl_seconds=ttl_seconds, + ) + + logger.warning( + "create_thread_registry(): no Agent Memory Service credentials found, " + "using InMemoryThreadRegistry — state is in-process only and will be " + "lost on process exit." + ) + return InMemoryThreadRegistry( + default_ttl_seconds=ttl_seconds, + ) diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py new file mode 100644 index 00000000..dfde4dd7 --- /dev/null +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py @@ -0,0 +1,256 @@ +"""In-memory thread registry for local development and unit testing.""" + +from __future__ import annotations + +import time +import uuid +from dataclasses import dataclass, field +from datetime import datetime, timedelta, timezone +from typing import Any, Optional + +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, + AgentMemoryThreadNotFoundError, +) +from sap_cloud_sdk.agent_memory.thread_registry._base import ( + BaseThreadRegistry, + ThreadPage, + ThreadRegistryEntry, + _validate_ttl, +) +from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics + + +@dataclass +class _StoredEntry: + thread_id: str + agent_id: str + invoker_id: str + description: Optional[str] + metadata: Optional[dict[str, Any]] + created_at: str + thread_ttl: Optional[int] = None + last_active: float = field(default_factory=time.monotonic) + last_active_wall: datetime = field( + default_factory=lambda: datetime.now(timezone.utc) + ) + + def is_expired(self, registry_ttl: Optional[int]) -> bool: + effective = self.thread_ttl if self.thread_ttl is not None else registry_ttl + if effective is None: + return False + return time.monotonic() - self.last_active > effective + + def touch(self) -> None: + self.last_active = time.monotonic() + self.last_active_wall = datetime.now(timezone.utc) + + def to_entry(self, registry_ttl: Optional[int] = None) -> ThreadRegistryEntry: + effective_ttl = self.thread_ttl if self.thread_ttl is not None else registry_ttl + if effective_ttl is not None: + expires_iso: Optional[str] = ( + self.last_active_wall + timedelta(seconds=effective_ttl) + ).strftime("%Y-%m-%dT%H:%M:%SZ") + else: + expires_iso = None + return ThreadRegistryEntry( + threadId=self.thread_id, + agentId=self.agent_id, + invokerId=self.invoker_id, + description=self.description, + createdAt=self.created_at, + updatedAt=self.last_active_wall.strftime("%Y-%m-%dT%H:%M:%SZ"), + expiresAt=expires_iso, + threadTtl=self.thread_ttl, + metadata=self.metadata, + ) + + +def _now_iso() -> str: + return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +class InMemoryThreadRegistry(BaseThreadRegistry): + """Dict-backed thread registry for local development and unit testing. + + State is in-process only and will not survive restarts. + + Implements a sliding idle TTL: expiry is computed as ``ttl_seconds`` + of inactivity. The window resets on every write. Expiry is checked lazily + at read time — no background thread is needed. + + ``expiresAt`` in returned entries is computed from the wall-clock time of + the last write plus the effective TTL, approximating the ``expiresAt`` field + returned by the Agent Memory Service. + + Irreducible differences from the service-backed registry: + + - **Best-effort timing**: expiry time advances on every register/update + (sliding); the service records server-side expiry at creation. + - **Lazy eviction**: threads are evicted at the next read, not on a sweep + schedule. A thread may appear live briefly after its true TTL. + - **No restart survival**: all state is lost on process exit. + + Args: + default_ttl_seconds: Registry-level default inactivity threshold. + Applied when a per-call ``ttl_seconds`` is not supplied. + ``None`` means threads never expire. + """ + + def __init__( + self, + *, + default_ttl_seconds: Optional[int] = None, + ) -> None: + _validate_ttl(default_ttl_seconds) + self._default_ttl = default_ttl_seconds + self._store: dict[str, _StoredEntry] = {} + + def _effective_ttl(self, ttl_seconds: Optional[int]) -> Optional[int]: + return ttl_seconds if ttl_seconds is not None else self._default_ttl + + def _get_live(self, thread_id: str) -> _StoredEntry: + entry = self._store.get(thread_id) + if entry is None or entry.is_expired(self._default_ttl): + raise AgentMemoryThreadNotFoundError(f"Thread '{thread_id}' not found") + return entry + + @record_metrics( + Module.AGENT_MEMORY, Operation.AGENT_MEMORY_CREATE_OR_REGISTER_THREAD + ) + def create_or_register_thread( + self, + agent_id: str, + invoker_id: str, + *, + thread_id: Optional[str] = None, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + _validate_ttl(ttl_seconds) + ttl = self._effective_ttl(ttl_seconds) + + if thread_id is None: + thread_id = str(uuid.uuid4()) + entry = _StoredEntry( + thread_id=thread_id, + agent_id=agent_id, + invoker_id=invoker_id, + description=description, + metadata=metadata, + created_at=_now_iso(), + thread_ttl=ttl, + ) + self._store[thread_id] = entry + return entry.to_entry(self._default_ttl) + + existing = self._store.get(thread_id) + if existing is not None: + if existing.is_expired(self._default_ttl): + raise AgentMemoryThreadExpiredError( + f"Thread '{thread_id}' has expired and cannot be re-created with the same ID" + ) + if existing.agent_id != agent_id or existing.invoker_id != invoker_id: + raise AgentMemoryThreadConflictError( + f"Thread '{thread_id}' is already registered by a different owner" + ) + existing.touch() + return existing.to_entry(self._default_ttl) + + entry = _StoredEntry( + thread_id=thread_id, + agent_id=agent_id, + invoker_id=invoker_id, + description=description, + metadata=metadata, + created_at=_now_iso(), + thread_ttl=ttl, + ) + self._store[thread_id] = entry + return entry.to_entry(self._default_ttl) + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_THREAD) + def get_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + return self._get_live(thread_id).to_entry(self._default_ttl) + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_THREADS) + def list_threads( + self, + *, + agent_id: Optional[str] = None, + invoker_id: Optional[str] = None, + limit: Optional[int] = None, + offset: Optional[int] = None, + tenant_id: Optional[str] = None, + ) -> ThreadPage: + if limit is not None: + if isinstance(limit, bool) or not isinstance(limit, int) or limit < 0: + raise ValueError(f"limit must be a non-negative integer, got {limit!r}") + resolved_offset = 0 if offset is None else offset + if ( + not isinstance(resolved_offset, int) + or isinstance(resolved_offset, bool) + or resolved_offset < 0 + ): + raise ValueError(f"offset must be a non-negative integer, got {offset!r}") + all_matching = [ + entry.to_entry(self._default_ttl) + for entry in self._store.values() + if not entry.is_expired(self._default_ttl) + and (agent_id is None or entry.agent_id == agent_id) + and (invoker_id is None or entry.invoker_id == invoker_id) + ] + total = len(all_matching) + if limit is not None and limit == 0: + return ThreadPage(entries=[], total=total, limit=0, offset=resolved_offset) + page = ( + all_matching[resolved_offset:] + if limit is None + else all_matching[resolved_offset : resolved_offset + limit] + ) + return ThreadPage( + entries=page, + total=total, + limit=len(page) if limit is None else limit, + offset=resolved_offset, + ) + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_THREAD) + def update_thread( + self, + thread_id: str, + *, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + # PATCH-style merge: only fields the caller passed are modified. An + # omitted ttl_seconds leaves the thread's existing TTL untouched. + entry = self._get_live(thread_id) + if description is not None: + entry.description = description + if metadata is not None: + entry.metadata = metadata + if ttl_seconds is not None: + _validate_ttl(ttl_seconds) + entry.thread_ttl = ttl_seconds + entry.touch() + return entry.to_entry(self._default_ttl) + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_THREAD) + def delete_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> None: + self._store.pop(thread_id, None) diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py new file mode 100644 index 00000000..12720b65 --- /dev/null +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py @@ -0,0 +1,457 @@ +"""Service-backed thread registry using the Agent Memory Service REST API.""" + +from __future__ import annotations + +import json as _json +import logging +from typing import Any, Optional +from urllib.parse import quote + +from requests.exceptions import RequestException, Timeout + +from sap_cloud_sdk.agent_memory.client import _require_ias_token +from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryHttpError, + AgentMemoryNotFoundError, + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, + AgentMemoryThreadNotFoundError, + AgentMemoryValidationError, +) +from sap_cloud_sdk.agent_memory.thread_registry._base import ( + BaseThreadRegistry, + ThreadPage, + ThreadRegistryEntry, + _validate_ttl, +) +from sap_cloud_sdk.core.protocol.http import ( + HttpClient, + HttpMethod, + IasAuthProvider, + XsuaaAuthProvider, +) +from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics + +logger = logging.getLogger(__name__) + +_THREAD_COLLECTION = "/v1/agent/threads" + + +# --------------------------------------------------------------------------- +# OData URL helpers +# --------------------------------------------------------------------------- + + +def _escape_odata_string(value: str) -> str: + """Double single quotes for OData string literals.""" + return value.replace("'", "''") + + +def _odata_entity_path(thread_id: str) -> str: + """Build OData entity-key path: /v1/agent/threads(threadId='...').""" + encoded = quote(_escape_odata_string(thread_id), safe="") + return f"{_THREAD_COLLECTION}(threadId='{encoded}')" + + +def _build_list_path( + agent_id: Optional[str], + invoker_id: Optional[str], + limit: Optional[int], + offset: int, +) -> str: + """Build the GET path for list_threads, keeping OData $ params un-encoded.""" + filter_parts: list[str] = [] + if agent_id is not None: + filter_parts.append(f"agentId eq '{_escape_odata_string(agent_id)}'") + if invoker_id is not None: + filter_parts.append(f"invokerId eq '{_escape_odata_string(invoker_id)}'") + + # Build OData query string manually — urlencode would encode $ as %24. + parts = [ + f"$orderby={quote('updatedAt desc', safe=',')}", + "$count=true", + ] + if limit is not None: + parts.append(f"$top={limit}") + if offset > 0: + parts.append(f"$skip={offset}") + if filter_parts: + parts.append(f"$filter={quote(' and '.join(filter_parts))}") + + return f"{_THREAD_COLLECTION}?{'&'.join(parts)}" + + +# --------------------------------------------------------------------------- +# Response parsing +# --------------------------------------------------------------------------- + + +def _is_expired_response(exc: AgentMemoryHttpError) -> bool: + """Return True if a 409 response indicates an expired thread (vs. conflict). + + Tries structured JSON error codes first (``error.code``, ``errorCode``), + then falls back to substring matching on both the exception message and the + raw response body. Two-layer detection means a service that upgrades from + prose-only errors to machine-readable codes is handled automatically. + """ + if exc.response_text: + try: + body = _json.loads(exc.response_text) + # Check common SAP/OData structured error shapes. + code: str = ( + (body.get("error") or {}).get("code", "") + or body.get("errorCode", "") + or body.get("code", "") + ) + if code: + return "expired" in code.lower() + # No code field — fall through to string matching below. + except (ValueError, TypeError): + pass # response body is not JSON + # String-match fallback: check both the exception message and response body. + combined = f"{exc} {exc.response_text or ''}".lower() + return "expired" in combined + + +def _parse_entry(row: dict[str, Any]) -> ThreadRegistryEntry: + # The service always returns camelCase JSON (SAP CAP/OData convention). + # Using plain .get() avoids the lossy `or` operator (which would discard + # a legitimate empty-string value by falling through to the next key). + return ThreadRegistryEntry( + threadId=row.get("threadId", ""), + agentId=row.get("agentId", ""), + invokerId=row.get("invokerId", ""), + description=row.get("description"), + createdAt=row.get("createdAt"), + updatedAt=row.get("updatedAt"), + expiresAt=row.get("expiresAt"), + threadTtl=row.get("threadTtl"), + metadata=row.get("metadata"), + ) + + +# --------------------------------------------------------------------------- +# Registry implementation +# --------------------------------------------------------------------------- + + +class SAPAgentMemoryThreadRegistry(BaseThreadRegistry): + """Service-backed thread registry for the SAP Agent Memory Service. + + Uses the SDK's :class:`~sap_cloud_sdk.core.protocol.http.HttpClient` + for OAuth2 or IAS authentication and request execution. Instantiate via + :func:`create_thread_registry` rather than directly. + + **Lifecycle**: the registry owns its :class:`~sap_cloud_sdk.core.protocol.http.HttpClient` + (OAuth sessions, underlying ``requests.Session`` objects). Call :meth:`close` or use the + registry as a context manager when you are done with it:: + + with create_thread_registry() as reg: + reg.create_or_register_thread(...) + + Args: + config: Service configuration (base URL, OAuth2 credentials, timeout). + default_ttl_seconds: Registry-level default sliding TTL in seconds. + Applied when a per-call ``ttl_seconds`` is not supplied. + """ + + def __init__( + self, + config: AgentMemoryConfig, + *, + default_ttl_seconds: Optional[int] = None, + ) -> None: + _validate_ttl(default_ttl_seconds) + is_ias = config.auth_mode == "ias" + self._ias_mode = is_ias + if is_ias: + auth = IasAuthProvider() + else: + auth = XsuaaAuthProvider(lambda: config) + self._http = HttpClient(config.base_url, auth, timeout=config.timeout) + self._base_url = config.base_url.rstrip("/") + self._default_ttl = default_ttl_seconds + + # ------------------------------------------------------------------ + # Lifecycle + # ------------------------------------------------------------------ + + def close(self) -> None: + """Close the underlying HTTP transport and release OAuth sessions.""" + self._http.close() + + def __enter__(self) -> "SAPAgentMemoryThreadRegistry": + return self + + def __exit__(self, *args: object) -> None: + self.close() + + def _ttl(self, ttl_seconds: Optional[int]) -> Optional[int]: + return ttl_seconds if ttl_seconds is not None else self._default_ttl + + def _strip_base(self, url: str) -> str: + """Strip the base URL prefix from an absolute URL, returning just the path.""" + if url.startswith(self._base_url): + return url[len(self._base_url) :] + return url + + def _request( + self, + method: HttpMethod, + path: str, + *, + tenant_subdomain: Optional[str] = None, + **kwargs: Any, + ) -> dict[str, Any]: + if self._ias_mode: + _require_ias_token() + try: + response = self._http.request( + method, + path, + tenant_subdomain=tenant_subdomain, + headers={"Content-Type": "application/json"}, + **kwargs, + ) + except Timeout as exc: + raise AgentMemoryHttpError( + f"Request timed out: {method.value} {path}" + ) from exc + except RequestException as exc: + raise AgentMemoryHttpError( + f"Request failed: {method.value} {path} — {exc}" + ) from exc + except AgentMemoryValidationError: + raise + except Exception as exc: + raise AgentMemoryHttpError(str(exc)) from exc + if response.status_code == 204 or not response.content: + return {} + if response.status_code == 404: + raise AgentMemoryNotFoundError( + f"Resource not found: {method.value} {path}", + status_code=404, + response_text=response.text, + ) + if not response.ok: + raise AgentMemoryHttpError( + f"Thread registry request failed. " + f"Method: {method.value}, Path: {path}, " + f"Status: {response.status_code}, Response: {response.text}", + status_code=response.status_code, + response_text=response.text, + ) + return response.json() + + # ------------------------------------------------------------------ + # create_or_register_thread + # ------------------------------------------------------------------ + + @record_metrics( + Module.AGENT_MEMORY, Operation.AGENT_MEMORY_CREATE_OR_REGISTER_THREAD + ) + def create_or_register_thread( + self, + agent_id: str, + invoker_id: str, + *, + thread_id: Optional[str] = None, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + ttl = self._ttl(ttl_seconds) + _validate_ttl(ttl) + + body: dict[str, Any] = {"agentId": agent_id, "invokerId": invoker_id} + if thread_id is not None: + body["threadId"] = thread_id + if description is not None: + body["description"] = description + if ttl is not None: + body["threadTtl"] = ttl + if metadata is not None: + body["metadata"] = metadata + + try: + data = self._request( + HttpMethod.POST, + _THREAD_COLLECTION, + json=body, + tenant_subdomain=tenant_id, + ) + except AgentMemoryHttpError as exc: + if thread_id is not None and exc.status_code == 409: + if _is_expired_response(exc): + raise AgentMemoryThreadExpiredError( + f"Thread '{thread_id}' has expired " + f"and cannot be re-created with the same ID" + ) from exc + raise AgentMemoryThreadConflictError( + f"Thread '{thread_id}' is already registered by a different owner" + ) from exc + raise + + return _parse_entry(data) + + # ------------------------------------------------------------------ + # get_thread + # ------------------------------------------------------------------ + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_GET_THREAD) + def get_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + return self._get_thread(thread_id, tenant_id=tenant_id) + + def _get_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + # Undecorated implementation shared by the public get_thread and by + # update_thread, so a single update_thread emits exactly one metric + # (update_thread) rather than also recording a nested get_thread. + try: + data = self._request( + HttpMethod.GET, + _odata_entity_path(thread_id), + tenant_subdomain=tenant_id, + ) + except AgentMemoryNotFoundError as exc: + raise AgentMemoryThreadNotFoundError( + f"Thread '{thread_id}' not found" + ) from exc + return _parse_entry(data) + + # ------------------------------------------------------------------ + # list_threads + # ------------------------------------------------------------------ + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_LIST_THREADS) + def list_threads( + self, + *, + agent_id: Optional[str] = None, + invoker_id: Optional[str] = None, + limit: Optional[int] = None, + offset: Optional[int] = None, + tenant_id: Optional[str] = None, + ) -> ThreadPage: + # Validate limit and offset before the limit==0 early-return. + # The bool guard must come first: False == 0 in Python, so without it + # limit=False would bypass validation and return an empty page silently. + if limit is not None: + if isinstance(limit, bool) or not isinstance(limit, int) or limit < 0: + raise ValueError(f"limit must be a non-negative integer, got {limit!r}") + resolved_offset = 0 if offset is None else offset + if ( + not isinstance(resolved_offset, int) + or isinstance(resolved_offset, bool) + or resolved_offset < 0 + ): + raise ValueError(f"offset must be a non-negative integer, got {offset!r}") + if limit is not None and limit == 0: + path = _build_list_path(agent_id, invoker_id, 0, resolved_offset) + body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + return ThreadPage( + entries=[], + total=body.get("@odata.count", 0), + limit=0, + offset=resolved_offset, + ) + + if limit is not None: + path = _build_list_path(agent_id, invoker_id, limit, resolved_offset) + body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + return ThreadPage( + entries=[_parse_entry(row) for row in body.get("value", [])], + total=body.get("@odata.count", 0), + limit=limit, + offset=resolved_offset, + ) + + # Unbounded — follow @odata.nextLink to collect all pages. + path = _build_list_path(agent_id, invoker_id, None, resolved_offset) + body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + total = body.get("@odata.count", 0) + entries = [_parse_entry(row) for row in body.get("value", [])] + next_link: Optional[str] = body.get("@odata.nextLink") + while next_link: + body = self._request( + HttpMethod.GET, self._strip_base(next_link), tenant_subdomain=tenant_id + ) + entries.extend(_parse_entry(row) for row in body.get("value", [])) + next_link = body.get("@odata.nextLink") + return ThreadPage( + entries=entries, + total=total, + limit=len(entries), + offset=resolved_offset, + ) + + # ------------------------------------------------------------------ + # update_thread + # ------------------------------------------------------------------ + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_UPDATE_THREAD) + def update_thread( + self, + thread_id: str, + *, + description: Optional[str] = None, + ttl_seconds: Optional[int] = None, + metadata: Optional[dict[str, Any]] = None, + tenant_id: Optional[str] = None, + ) -> ThreadRegistryEntry: + # PATCH-style merge: only fields the caller passed are sent. In + # particular the registry default TTL is NOT reapplied here — a + # description-only update must not silently reset the thread's TTL. + body: dict[str, Any] = {} + if description is not None: + body["description"] = description + if metadata is not None: + body["metadata"] = metadata + if ttl_seconds is not None: + _validate_ttl(ttl_seconds) + body["threadTtl"] = ttl_seconds + + try: + self._request( + HttpMethod.PATCH, + _odata_entity_path(thread_id), + json=body, + tenant_subdomain=tenant_id, + ) + except AgentMemoryNotFoundError as exc: + raise AgentMemoryThreadNotFoundError( + f"Thread '{thread_id}' not found" + ) from exc + + return self._get_thread(thread_id, tenant_id=tenant_id) + + # ------------------------------------------------------------------ + # delete_thread + # ------------------------------------------------------------------ + + @record_metrics(Module.AGENT_MEMORY, Operation.AGENT_MEMORY_DELETE_THREAD) + def delete_thread( + self, + thread_id: str, + *, + tenant_id: Optional[str] = None, + ) -> None: + try: + self._request( + HttpMethod.DELETE, + _odata_entity_path(thread_id), + tenant_subdomain=tenant_id, + ) + except AgentMemoryNotFoundError: + pass diff --git a/src/sap_cloud_sdk/agent_memory/user-guide.md b/src/sap_cloud_sdk/agent_memory/user-guide.md index 75b67a53..4632f00c 100644 --- a/src/sap_cloud_sdk/agent_memory/user-guide.md +++ b/src/sap_cloud_sdk/agent_memory/user-guide.md @@ -11,7 +11,6 @@ plain text, and the service makes it searchable by meaning. ## Table of Contents -- [Agent Memory User Guide](#agent-memory-user-guide) - [Table of Contents](#table-of-contents) - [Installation](#installation) - [Import](#import) @@ -39,6 +38,26 @@ plain text, and the service makes it searchable by meaning. - [Delete a Message](#delete-a-message) - [List Messages](#list-messages) - [Content and metadata filtering](#content-and-metadata-filtering-1) + - [Thread Registry](#thread-registry) + - [Create or Register a Thread](#create-or-register-a-thread) + - [Get a Thread](#get-a-thread) + - [List Threads](#list-threads) + - [Update a Thread](#update-a-thread) + - [Delete a Thread](#delete-a-thread) + - [Thread TTL and lifecycle](#thread-ttl-and-lifecycle) + - [Resource Management: `close()` and Context Manager](#resource-management-close-and-context-manager) + - [Exposing idle TTL with `@agent_config`](#exposing-idle-ttl-with-agent_config) + - [LangGraph Checkpointer](#langgraph-checkpointer) + - [Prerequisites](#prerequisites) + - [Import](#import-1) + - [Usage with LangGraph StateGraph](#usage-with-langgraph-stategraph) + - [Usage with LangChain create_agent](#usage-with-langchain-create_agent) + - [Configuration](#configuration) + - [XSUAA binding](#xsuaa-binding) + - [Mounted Secrets (Kubernetes)](#mounted-secrets-kubernetes) + - [Environment Variables](#environment-variables) + - [UAA JSON Schema](#uaa-json-schema) + - [IAS binding](#ias-binding) - [Data Models](#data-models) - [Enums](#enums) - [Error Handling](#error-handling) @@ -57,18 +76,8 @@ plain text, and the service makes it searchable by meaning. - [`search_memories()` returns no results](#search_memories-returns-no-results) - [`AgentMemoryNotFoundError` when fetching a resource](#agentmemorynotfounderror-when-fetching-a-resource) - [`AgentMemoryHttpError` with status 401](#agentmemoryhttperror-with-status-401) - - [Configuration](#configuration) - - [Service Binding](#service-binding) - - [Mounted Secrets (Kubernetes)](#mounted-secrets-kubernetes) - - [Environment Variables](#environment-variables) - - [UAA JSON Schema](#uaa-json-schema) - - [LangGraph Checkpointer](#langgraph-checkpointer) - - [Prerequisites](#prerequisites) - - [Import](#import-1) - - [Usage with LangGraph StateGraph](#usage-with-langgraph-stategraph) - - [Usage with LangChain create\_agent](#usage-with-langchain-create_agent) - - [Thread TTL](#thread-ttl) - - [Exposing TTL as a configurable parameter with `@agent_config`](#exposing-ttl-as-a-configurable-parameter-with-agent_config) + - [`AgentMemoryValidationError: IAS token not found in RuntimeContext`](#agentmemoryvalidationerror-ias-token-not-found-in-runtimecontext) + - [Binding Rotation](#binding-rotation) ## Installation @@ -172,6 +181,11 @@ across create, read, and search calls is the implementer's responsibility. - **Authentication:** XSUAA - **How to use:** Pass `access_strategy` and `tenant` to `create_client()`. The strategy controls whether calls use a subscriber-scoped or provider-scoped XSUAA token. Every method on the client inherits the strategy set at construction time. + > [!NOTE] + > `access_strategy` and `tenant` apply to **XSUAA bindings only**. In **IAS mode** + > (auto-detected from a URL-only binding) they are ignored — routing is driven by the + > per-request IAS token. See [IAS binding](#ias-binding) under Configuration. + | Value | Description | | ------------------------------------- | ----------------------------------------------------------------------------------------------- | | `AccessStrategy.SUBSCRIBER` (default) | Reads and writes against the subscriber tenant. Requires `tenant`. | @@ -508,6 +522,313 @@ See the [Content and metadata filtering](#content-and-metadata-filtering) note u --- +## Thread Registry + +The thread registry tracks conversation threads — their owner, idle TTL, and expiry. Unlike +the LangGraph checkpointer, it is **framework-agnostic**: it works with LangGraph, Pydantic AI, +CrewAI, or any custom agent. + +Use `create_thread_registry()` to create a registry for the current environment: + +```python +from sap_cloud_sdk.agent_memory.thread_registry import create_thread_registry + +registry = create_thread_registry(ttl_seconds=3600) # 1-hour idle TTL +``` + +`create_thread_registry()` auto-detects credentials: + +- **Credentials present** — returns `SAPAgentMemoryThreadRegistry` backed by the Agent Memory + Service. Thread state is persistent across restarts and enforced server-side. +- **No credentials** — falls back to `InMemoryThreadRegistry`. State is in-process only and + will not survive restarts. + +You can also import the concrete types and the `BaseThreadRegistry` ABC directly: + +```python +from sap_cloud_sdk.agent_memory.thread_registry import ( + BaseThreadRegistry, + InMemoryThreadRegistry, + SAPAgentMemoryThreadRegistry, +) +``` + +### Create or Register a Thread + +`create_or_register_thread()` is idempotent: calling it with the same `thread_id` a second +time touches the thread (resetting its idle TTL) rather than raising an error. + +```python +entry = registry.create_or_register_thread( + agent_id="my-agent", + invoker_id="user-123", + thread_id="session-abc", # optional — auto-generated UUID if omitted + description="Support chat", + ttl_seconds=1800, # per-thread override, or omit to use registry default +) +print(entry["threadId"], entry["expiresAt"]) +``` + +### Get a Thread + +```python +entry = registry.get_thread(thread_id="session-abc") +``` + +Raises `AgentMemoryThreadNotFoundError` if the thread does not exist or has expired. + +### List Threads + +```python +page = registry.list_threads( + agent_id="my-agent", # optional filter + invoker_id="user-123", # optional filter + limit=50, + offset=0, +) +for entry in page["entries"]: + print(entry["threadId"], entry["expiresAt"]) +print(f"{page['total']} threads total") +``` + +### Update a Thread + +```python +entry = registry.update_thread( + "session-abc", + description="Updated description", + ttl_seconds=7200, # extend TTL +) +``` + +`update_thread()` is a PATCH-style merge — only the fields you pass are modified. Omit +`ttl_seconds` to leave the thread's existing TTL untouched (mirrors LangGraph's +`threads.update`). + +### Delete a Thread + +```python +registry.delete_thread("session-abc") # idempotent — no error if already gone +``` + +### Thread TTL and lifecycle + +**Sliding idle TTL**: `ttl_seconds` is an *inactivity* window. The expiry resets on each +write (register, update). A thread inactive for longer than `ttl_seconds` is treated as +expired. `None` means threads never expire. + +TTL precedence (highest to lowest): +1. Per-call `ttl_seconds` passed to `create_or_register_thread()` / `update_thread()` +2. `create_thread_registry(ttl_seconds=...)` registry-level default +3. No expiry (`None`) + +`InMemoryThreadRegistry` evaluates expiry lazily at read time; the service-backed registry +enforces expiry server-side via the `expiresAt` field. + +### Resource Management: `close()` and Context Manager + +`SAPAgentMemoryThreadRegistry` owns an HTTP transport with an OAuth token cache and connection +pool. Call `close()` when the registry is no longer needed, or use the context manager: + +```python +# Explicit close +registry = create_thread_registry() +try: + ... +finally: + registry.close() + +# Context manager (preferred) +with create_thread_registry() as registry: + entry = registry.create_or_register_thread( + agent_id="my-agent", invoker_id="user-123" + ) +``` + +> [!NOTE] +> `InMemoryThreadRegistry.close()` is a no-op — it has no external resources to release. +> The context manager interface is available on all registry types for uniform usage. + +### Exposing idle TTL with `@agent_config` + +Use `@agent_config` from `sap_cloud_sdk.agent_decorators` to expose `ttl_seconds` as an +operator-adjustable configuration field. The key `config.registry.ttl_seconds` groups it +with other registry settings so external tooling can surface it in the low-code UI alongside +model selection and temperature. + +```python +from sap_cloud_sdk.agent_decorators import agent_config +from sap_cloud_sdk.agent_memory.thread_registry import create_thread_registry + + +@agent_config( + key="config.registry.ttl_seconds", + label="Thread idle TTL (seconds)", + description="Expire conversation threads after this period of inactivity. " + "Set to 0 to disable eviction.", +) +def thread_ttl_seconds() -> int: + return 3600 # 1 hour + + +class MyAgent: + def __init__(self): + ttl = thread_ttl_seconds() + self._registry = create_thread_registry(ttl_seconds=ttl or None) +``` + +> [!NOTE] +> `InMemoryThreadRegistry` state does not survive process restarts. When Agent Memory Service +> credentials are available, `create_thread_registry()` returns `SAPAgentMemoryThreadRegistry` +> instead, and thread expiry is enforced server-side via the `expiresAt` field on the thread +> registry row. + +## LangGraph Checkpointer + +> This section covers the LangGraph-specific +> factory for short-term memory (checkpointing). It is only relevant if your agent +> is built with LangGraph or LangChain's `create_agent()`. The core +> `AgentMemoryClient` and `create_client()` above are framework-agnostic and work +> independently of this section. + +For LangGraph agents, the `agent_memory` module provides a `create_checkpointer()` +factory in the `factory` subpackage. It returns a `BaseCheckpointSaver` that +manages short-term session memory — conversation state, thread continuity, and +HITL support — natively within LangGraph. + +`create_checkpointer()` returns an `InMemorySaver`. State is in-process only and +will not survive process restarts. + +> **Deprecated — `create_checkpointer(ttl_seconds=...)`.** Passing `ttl_seconds` +> returns a `TimedInMemorySaver` that evicts inactive threads. This is deprecated: +> thread lifecycle is owned by the **thread registry**, and configuring TTL in two +> places leads to drift. The parameter still works and will not be removed yet +> (existing code keeps running), but it now emits a `DeprecationWarning`. Configure +> TTL on `create_thread_registry()` instead: +> +> ```python +> # Deprecated — checkpointer-level TTL +> checkpointer = create_checkpointer(ttl_seconds=3600) +> +> # Preferred — TTL owned by the thread registry +> registry = create_thread_registry(ttl_seconds=3600) +> checkpointer = create_checkpointer() +> ``` + +### Prerequisites + +`langgraph` is an optional dependency. Install it via the SDK extra or directly: + +```bash +# Via SDK optional extra (recommended) +pip install "sap-cloud-sdk[langgraph]" + +# Or install langgraph directly +pip install langgraph +``` + +### Import + +```python +from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer +``` + +### Usage with LangGraph StateGraph + +```python +from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer + +checkpointer = create_checkpointer() +app = workflow.compile(checkpointer=checkpointer) + +result = app.invoke( + {"messages": [{"role": "user", "content": "hello"}]}, + {"configurable": {"thread_id": "session-1"}}, +) +``` + +### Usage with LangChain create_agent + +```python +from langchain.agents import create_agent +from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer + +agent = create_agent( + model="...", + tools=[...], + checkpointer=create_checkpointer(), +) +``` + +## Configuration + +`create_client()` auto-detects the binding type: + +- A binding whose `uaa` JSON carries a `token_url` (`url`) selects **XSUAA**. +- A binding with only a service URL and **no** `uaa`/`token_url` selects **IAS**. + +### XSUAA binding + +- **Mount path**: `$SERVICE_BINDING_ROOT/hana-agent-memory/default/` (defaults to `/etc/secrets/appfnd/hana-agent-memory/default/`) +- **Required keys**: `application_url` (Agent Memory service URL), `uaa` (JSON string with XSUAA credentials) +- **Env var fallback**: `CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_{FIELD}` (uppercased) + +> **Note:** `SERVICE_BINDING_ROOT` defaults to `/etc/secrets/appfnd` when not set. See the [Secret Resolver guide](../core/secret_resolver/user-guide.md) for details. + +#### Mounted Secrets (Kubernetes) + +``` +$SERVICE_BINDING_ROOT/hana-agent-memory/default/ +├── application_url +└── uaa +``` + +#### Environment Variables + +```bash +export CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL="https://agent-memory.example.com" +export CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA='{"clientid":"...","clientsecret":"...","url":"https://..."}' +``` + +#### UAA JSON Schema + +The `uaa` key must contain a JSON string with the XSUAA credentials: + +```json +{ + "clientid": "sb-xxx", + "clientsecret": "xxx", + "url": "https://subdomain.authentication.region.hana.ondemand.com" +} +``` + +### IAS binding + +IAS mode is **auto-detected** — a URL-only binding (`application_url`, no `uaa`/`token_url`) +selects IAS automatically. There is nothing extra to configure on `create_client()`: +`access_strategy` and `tenant` are **ignored** in IAS mode (requests are routed by the +per-request token, not by an XSUAA strategy). + +Unlike XSUAA, the client holds no credentials and fetches no token of its own. The Bearer +token travels **per request**, read from the request `RuntimeContext` at call time. + +> [!IMPORTANT] +> **The agent must wire `bootstrap(app)` / `IASContextProvider`.** This is what parses the +> inbound `Authorization` header and places the IAS token into the `RuntimeContext` that the +> client reads. Without it, **every** Agent Memory and Thread Registry call fails with: +> +> ``` +> AgentMemoryValidationError: IAS token not found in RuntimeContext. Ensure bootstrap(app) is configured. +> ``` +> +> Wire it once at startup — for example `bootstrap(app)` on your FastAPI/Flask app — so each +> inbound request populates the context before any client call runs. + +> [!NOTE] +> **Feedback / future improvement:** the cloud-sdk could auto-bootstrap the IAS context +> wiring when it detects an IAS binding at runtime, so agents would not have to call +> `bootstrap(app)` manually. Until then, wiring the context provider is a required manual step. + ## Data Models | Model | Description | @@ -515,7 +836,7 @@ See the [Content and metadata filtering](#content-and-metadata-filtering) note u | `Memory` | A persistent memory entry (`id`, `agent_id`, `invoker_id`, `content`, `metadata`, timestamps) | | `SearchResult` | Extends `Memory` with a `similarity` field (cosine score, 0–1) | | `Message` | A message (`id`, `agent_id`, `invoker_id`, `message_group`, `role`, `content`, `metadata`, timestamp) | -| `RetentionConfig` | Data retention policy (`message_days`, `memory_days`, `usage_log_days`, timestamps) | +| `RetentionConfig` | Data retention policy (`message_days`, `memory_days`, `usage_log_days`, `thread_days`, `max_checkpoints_per_thread`, timestamps) | ### Enums @@ -549,10 +870,13 @@ level of specificity: ``` AgentMemoryError -├── AgentMemoryConfigError # bad or missing configuration -├── AgentMemoryValidationError # invalid inputs caught before any network call -└── AgentMemoryHttpError # HTTP-level error (status_code, response_text) - └── AgentMemoryNotFoundError # 404 Not Found +├── AgentMemoryConfigError # bad or missing configuration +├── AgentMemoryValidationError # invalid inputs caught before any network call +├── AgentMemoryHttpError # HTTP-level error (status_code, response_text) +│ └── AgentMemoryNotFoundError # 404 Not Found (memories, messages) +├── AgentMemoryThreadNotFoundError # thread not found in the registry +├── AgentMemoryThreadConflictError # thread ID already owned by a different owner +└── AgentMemoryThreadExpiredError # thread has expired and cannot be reused ``` ```python @@ -562,6 +886,9 @@ from sap_cloud_sdk.agent_memory.exceptions import ( AgentMemoryValidationError, AgentMemoryHttpError, AgentMemoryNotFoundError, + AgentMemoryThreadNotFoundError, + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, ) # Catch invalid inputs before they reach the network @@ -605,6 +932,7 @@ rc = client.get_retention_config() print(f"Messages: {rc.message_days} days") print(f"Memories: {rc.memory_days} days") print(f"Usage logs: {rc.usage_log_days} days") +print(f"Registered threads: {rc.thread_days} days") ``` ### Update Retention Config @@ -617,9 +945,14 @@ client.update_retention_config( message_days=30, memory_days=90, usage_log_days=180, + thread_days=45, ) ``` +`thread_days` is the default TTL (in days) applied **only to registered threads** +(those created via the Thread Registry API). It is the age-based cutoff for thread +cleanup; threads not registered with the Thread Registry are unaffected. + Set a field to `0` to mark all data in that category for deletion at the next nightly scheduled cleanup. The server also accepts `null` to disable automatic cleanup for that category. @@ -799,160 +1132,20 @@ The OAuth2 token has expired and automatic refresh failed, or the configured cre (`client_id`, `client_secret`, `token_url`) are incorrect. Verify the credentials in your environment variables or service binding. ---- - -## Configuration - -### Service Binding - -- **Mount path**: `$SERVICE_BINDING_ROOT/hana-agent-memory/default/` (defaults to `/etc/secrets/appfnd/hana-agent-memory/default/`) -- **Required keys**: `application_url` (Agent Memory service URL), `uaa` (JSON string with XSUAA credentials) -- **Env var fallback**: `CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_{FIELD}` (uppercased) - -> **Note:** `SERVICE_BINDING_ROOT` defaults to `/etc/secrets/appfnd` when not set. See the [Secret Resolver guide](../core/secret_resolver/user-guide.md) for details. +### `AgentMemoryValidationError: IAS token not found in RuntimeContext` -#### Mounted Secrets (Kubernetes) - -``` -$SERVICE_BINDING_ROOT/hana-agent-memory/default/ -├── application_url -└── uaa ``` - -#### Environment Variables - -```bash -export CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL="https://agent-memory.example.com" -export CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA='{"clientid":"...","clientsecret":"...","url":"https://..."}' +AgentMemoryValidationError: IAS token not found in RuntimeContext. Ensure bootstrap(app) is configured. ``` -#### UAA JSON Schema - -The `uaa` key must contain a JSON string with the XSUAA credentials: +The client is running in **IAS mode** but no IAS token reached the request `RuntimeContext`. +The agent must wire `bootstrap(app)` / `IASContextProvider` so the inbound `Authorization` +header is parsed into the context before any client call. See [IAS binding](#ias-binding). -```json -{ - "clientid": "sb-xxx", - "clientsecret": "xxx", - "url": "https://subdomain.authentication.region.hana.ondemand.com" -} -``` +--- ## Binding Rotation The client handles BTP credential rotation automatically. See the [HTTP Client guide](../core/protocol/http/user-guide.md) for details. --- - -## LangGraph Checkpointer - -> This section covers the LangGraph-specific -> factory for short-term memory (checkpointing). It is only relevant if your agent -> is built with LangGraph or LangChain's `create_agent()`. The core -> `AgentMemoryClient` and `create_client()` above are framework-agnostic and work -> independently of this section. - -For LangGraph agents, the `agent_memory` module provides a `create_checkpointer()` -factory in the `factory` subpackage. It returns a `BaseCheckpointSaver` that -manages short-term session memory — conversation state, thread continuity, and -HITL support — natively within LangGraph. - -> [!NOTE] -> The current implementation uses LangGraph's `InMemorySaver` or -> `TimedInMemorySaver` (when `ttl_seconds` is set). State is held in-process -> and does not survive restarts. Persistent checkpointing backed by the -> Agent Memory Service is not yet supported. - -### Prerequisites - -`langgraph` is an optional dependency. Install it via the SDK extra or directly: - -```bash -# Via SDK optional extra (recommended) -pip install "sap-cloud-sdk[langgraph]" - -# Or install langgraph directly -pip install langgraph -``` - -### Import - -```python -from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer -``` - -### Usage with LangGraph StateGraph - -```python -from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer - -checkpointer = create_checkpointer() -app = workflow.compile(checkpointer=checkpointer) - -result = app.invoke( - {"messages": [{"role": "user", "content": "hello"}]}, - {"configurable": {"thread_id": "session-1"}}, -) -``` - -### Usage with LangChain create_agent - -```python -from langchain.agents import create_agent -from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer - -agent = create_agent( - model="...", - tools=[...], - checkpointer=create_checkpointer(), -) -``` - -### Thread TTL - -Pass `ttl_seconds` to evict threads that have been inactive for the given -period. This prevents unbounded memory growth in long-running processes. - -```python -# Evict threads inactive for more than 1 hour -checkpointer = create_checkpointer(ttl_seconds=3600) -``` - -When `ttl_seconds` is set, the factory returns a `TimedInMemorySaver` that -tracks last-active time per thread and evicts inactive threads via a -background daemon sweep. Eviction is best-effort — a thread may live up to -`ttl_seconds + 60` seconds before deletion. - -#### Exposing TTL as a configurable parameter with `@agent_config` - -Use `@agent_config` from `sap_cloud_sdk.agent_decorators` to expose -`ttl_seconds` as an operator-adjustable configuration field. The key -`config.checkpointer.ttl_seconds` groups it with other checkpointer settings -so external tooling can surface it in the low-code UI alongside model -selection and temperature. - -```python -from sap_cloud_sdk.agent_decorators import agent_config -from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer - - -@agent_config( - key="config.checkpointer.ttl_seconds", - label="Thread TTL (seconds)", - description="Evict inactive conversation threads after this period of " - "inactivity. Set to 0 to disable eviction.", -) -def thread_ttl_seconds() -> int: - return 3600 # 1 hour - - -class MyAgent: - def __init__(self): - ttl = thread_ttl_seconds() - self._checkpointer = create_checkpointer(ttl_seconds=ttl or None) -``` - -> [!NOTE] -> `TimedInMemorySaver` state does not survive process restarts — the TTL -> applies to in-process memory only. Persistent TTL enforcement will be -> available when the Agent Memory Service checkpointer ships. diff --git a/src/sap_cloud_sdk/core/protocol/http/__init__.py b/src/sap_cloud_sdk/core/protocol/http/__init__.py index 4a65d6f1..6d924480 100644 --- a/src/sap_cloud_sdk/core/protocol/http/__init__.py +++ b/src/sap_cloud_sdk/core/protocol/http/__init__.py @@ -3,6 +3,7 @@ from sap_cloud_sdk.core.protocol.http.models import ( AuthProvider, HttpMethod, + IasAuthProvider, XsuaaAuthProvider, ) from sap_cloud_sdk.core.protocol.http.client import HttpClient @@ -11,5 +12,6 @@ "AuthProvider", "HttpClient", "HttpMethod", + "IasAuthProvider", "XsuaaAuthProvider", ] diff --git a/src/sap_cloud_sdk/core/protocol/http/models.py b/src/sap_cloud_sdk/core/protocol/http/models.py index f5e0e09f..99e73180 100644 --- a/src/sap_cloud_sdk/core/protocol/http/models.py +++ b/src/sap_cloud_sdk/core/protocol/http/models.py @@ -176,3 +176,43 @@ def base_url(self) -> Optional[str]: expose a ``base_url`` attribute (e.g. the destination module uses ``url``). """ return getattr(self._config, "base_url", None) + + +class IasAuthProvider(AuthProvider): + """Bearer-token auth provider for IAS (Identity Authentication Service). + + Unlike :class:`XsuaaAuthProvider`, this provider holds no credentials and + fetches no token. In IAS mode the token is supplied *per request* by the + caller's runtime: the IAS context provider (wired via ``bootstrap(app)``) + parses the inbound ``Authorization`` header and stores the raw bearer token + in the request :class:`~sap_cloud_sdk.core.runtime_context.RuntimeContext`. + This provider reads that token at :meth:`get_session` time and attaches it. + + Because the token is read fresh on every call, there is nothing to cache or + invalidate — ``invalidate``/``invalidate_all``/``close`` are no-ops. + + When no token is present the session is returned without an ``Authorization`` + header; enforcing presence is the caller's responsibility (the Agent Memory + client raises a validation error before dispatching the request). + """ + + def get_session(self, tenant_subdomain: Optional[str] = None) -> requests.Session: + # Imported lazily: runtime_context pulls in sap_cloud_sdk.ias, which would + # create an import cycle if imported at module load time. + from sap_cloud_sdk.core.runtime_context import get_context + from sap_cloud_sdk.core.runtime_context.providers._ias import IAS_TOKEN + + session = requests.Session() + ias_token = get_context().get(IAS_TOKEN) + if ias_token: + session.headers["Authorization"] = f"Bearer {ias_token}" + return session + + def invalidate(self, tenant_subdomain: Optional[str] = None) -> None: + pass + + def invalidate_all(self) -> None: + pass + + def close(self) -> None: + pass diff --git a/src/sap_cloud_sdk/core/runtime_context/__init__.py b/src/sap_cloud_sdk/core/runtime_context/__init__.py index 520a237b..79317be1 100644 --- a/src/sap_cloud_sdk/core/runtime_context/__init__.py +++ b/src/sap_cloud_sdk/core/runtime_context/__init__.py @@ -41,6 +41,7 @@ from sap_cloud_sdk.core.runtime_context.providers import ( DWCContextProvider, IASContextProvider, + IAS_TOKEN, SAPTriggerContextProvider, APP_TENANT_ID, GLOBAL_TENANT_ID, @@ -63,6 +64,7 @@ "get_attached_adapters", "GLOBAL_TENANT_ID", "IASContextProvider", + "IAS_TOKEN", "RuntimeContext", "RequestEnvelope", "SAPTriggerContextProvider", @@ -71,4 +73,5 @@ "get_context", "is_feature_enabled", "register", + "set_context", ] diff --git a/src/sap_cloud_sdk/core/runtime_context/providers/__init__.py b/src/sap_cloud_sdk/core/runtime_context/providers/__init__.py index db79192a..ddfbaae6 100644 --- a/src/sap_cloud_sdk/core/runtime_context/providers/__init__.py +++ b/src/sap_cloud_sdk/core/runtime_context/providers/__init__.py @@ -5,6 +5,7 @@ APP_TENANT_ID, GLOBAL_TENANT_ID, IASContextProvider, + IAS_TOKEN, USER_ID, ) from sap_cloud_sdk.core.runtime_context.providers._sap_trigger import ( @@ -16,6 +17,7 @@ "DWCContextProvider", "GLOBAL_TENANT_ID", "IASContextProvider", + "IAS_TOKEN", "SAPTriggerContextProvider", "USER_ID", ] diff --git a/src/sap_cloud_sdk/core/runtime_context/providers/_ias.py b/src/sap_cloud_sdk/core/runtime_context/providers/_ias.py index f23b4418..2d5dbde2 100644 --- a/src/sap_cloud_sdk/core/runtime_context/providers/_ias.py +++ b/src/sap_cloud_sdk/core/runtime_context/providers/_ias.py @@ -13,6 +13,7 @@ APP_TENANT_ID = ContextKey[str]("ias.app_tid") GLOBAL_TENANT_ID = ContextKey[str]("ias.sap_gtid") USER_ID = ContextKey[str]("ias.user_uuid") +IAS_TOKEN = ContextKey[str]("ias.token") class IASContextProvider(ContextProvider): @@ -26,25 +27,28 @@ class IASContextProvider(ContextProvider): - :data:`APP_TENANT_ID` from ``app_tid`` claim - :data:`GLOBAL_TENANT_ID` from ``sap_gtid`` claim - :data:`USER_ID` from ``user_uuid`` claim + - :data:`IAS_TOKEN` raw bearer token string """ def extract(self, envelope: RequestEnvelope) -> RuntimeContext: auth = envelope.headers.get("authorization", "") + if not auth: + return RuntimeContext({}) - claims = None - if auth: - try: - claims = parse_token(auth) - except Exception as e: - logger.debug("IAS token parsing failed, context will be empty: %s", e) + raw = auth.removeprefix("Bearer ").removeprefix("bearer ").strip() + if not raw: + return RuntimeContext({}) - values = {} - if claims: + values: dict = {IAS_TOKEN: raw} + try: + claims = parse_token(raw) if claims.app_tid: values[APP_TENANT_ID] = claims.app_tid if claims.sap_gtid: values[GLOBAL_TENANT_ID] = claims.sap_gtid if claims.user_uuid: values[USER_ID] = claims.user_uuid + except Exception as e: + logger.debug("IAS token parsing failed, context will be empty: %s", e) return RuntimeContext(values) diff --git a/src/sap_cloud_sdk/core/secret_resolver/resolver.py b/src/sap_cloud_sdk/core/secret_resolver/resolver.py index c76bbc74..fc2c2308 100644 --- a/src/sap_cloud_sdk/core/secret_resolver/resolver.py +++ b/src/sap_cloud_sdk/core/secret_resolver/resolver.py @@ -3,8 +3,9 @@ from __future__ import annotations import os +import typing from dataclasses import fields, is_dataclass -from typing import Any, Dict, Tuple +from typing import Any, Dict, Tuple, Union from .constants import BASE_MOUNT_PATH @@ -47,31 +48,51 @@ def _validate_path(path: str) -> None: raise NotADirectoryError(f"path is not a directory: {path}") -def _get_field_map(target: Any) -> Dict[str, Tuple[str, type]]: +def _get_field_map(target: Any) -> Dict[str, Tuple[str, type, bool]]: """ - Build a mapping from secret key -> (attribute_name, attribute_type) for a dataclass instance. + Build a mapping from secret key -> (attribute_name, attribute_type, is_optional). Priority: 1. Use field.metadata["secret"] if present as the key 2. Fallback to the lowercase dataclass field name - Only string-typed fields are supported. + + Supported field types: + - ``str`` → required; absent secret raises an error + - ``Optional[str]`` → optional; absent secret is silently skipped and the + field keeps its dataclass default + + Optionality is declared via ``Optional[str]`` (not metadata); uses + ``get_type_hints()`` to handle PEP 563 string annotations correctly. """ if not is_dataclass(target) or isinstance(target, type): raise TypeError("target must be a dataclass instance") - mapping: Dict[str, Tuple[str, type]] = {} + # get_type_hints() resolves PEP 563 string annotations to real type objects. + hints = typing.get_type_hints(type(target)) + + mapping: Dict[str, Tuple[str, type, bool]] = {} for f in fields(target): - # Only support string fields for secrets (consistent with Go SDK) - # Allow plain 'str' annotations; reject others to keep behavior predictable - if f.type is not str: + hint = hints.get(f.name) + origin = typing.get_origin(hint) + args = typing.get_args(hint) + + # Optional[str] is Union[str, NoneType] with exactly two args. + is_optional_str = ( + origin is Union and len(args) == 2 and str in args and type(None) in args + ) + + if hint is not str and not is_optional_str: raise TypeError( - f"target field '{f.name}' is not a string (only str fields are supported)" + f"target field '{f.name}' is not a string " + "(only str and Optional[str] fields are supported)" ) + + is_optional = is_optional_str key = f.metadata.get("secret") if hasattr(f, "metadata") else None if key and isinstance(key, str) and key.strip(): - mapping[key] = (f.name, f.type) + mapping[key] = (f.name, str, is_optional) else: - mapping[f.name.lower()] = (f.name, f.type) + mapping[f.name.lower()] = (f.name, str, is_optional) return mapping @@ -82,17 +103,20 @@ def _load_from_path(secret_dir: str, target: Any) -> None: Reads each field key as a file name under secret_dir. Used for both the servicebinding.io flat layout ($ROOT//) and the legacy three-level layout via :func:`_load_from_mount`. + Fields typed ``Optional[str]`` are silently skipped when the file is absent. """ _validate_path(secret_dir) field_map = _get_field_map(target) - for key, (attr_name, _) in field_map.items(): + for key, (attr_name, _, is_optional) in field_map.items(): file_path = os.path.join(secret_dir, key) try: # Read entire file content as text; do not strip newlines to match Go behavior with open(file_path, "r", encoding="utf-8") as f: content = f.read() except FileNotFoundError as e: + if is_optional: + continue # Align with Go: surface precise file error raise FileNotFoundError( f"failed to read secret file {file_path}: {e}" @@ -120,14 +144,17 @@ def _load_from_env(base_var_name: str, module: str, instance: str, target: Any) Load secrets from environment variables with names: {base_var_name}_{module}_{instance}_{field_key} (uppercased) instance names have '-' replaced with '_' for env var compatibility. + Fields typed ``Optional[str]`` are silently skipped when the env var is absent. """ field_map = _get_field_map(target) prefix = f"{base_var_name}_{module}_{instance}".upper() - for key, (attr_name, _) in field_map.items(): + for key, (attr_name, _, is_optional) in field_map.items(): var_name = f"{prefix}_{key}".upper() value = os.environ.get(var_name) if value is None: + if is_optional: + continue # Align with Go: error if env var not found raise KeyError(f"env var not found: {var_name}") setattr(target, attr_name, value) diff --git a/src/sap_cloud_sdk/core/telemetry/operation.py b/src/sap_cloud_sdk/core/telemetry/operation.py index 4ae1893d..3b843c6f 100644 --- a/src/sap_cloud_sdk/core/telemetry/operation.py +++ b/src/sap_cloud_sdk/core/telemetry/operation.py @@ -319,6 +319,16 @@ class Operation(str, Enum): AGENT_MEMORY_GET_RETENTION_CONFIG = "get_retention_config" AGENT_MEMORY_UPDATE_RETENTION_CONFIG = "update_retention_config" + # Agent Memory — Thread Registry Operations + AGENT_MEMORY_CREATE_OR_REGISTER_THREAD = "create_or_register_thread" + AGENT_MEMORY_GET_THREAD = "get_thread" + AGENT_MEMORY_LIST_THREADS = "list_threads" + AGENT_MEMORY_UPDATE_THREAD = "update_thread" + AGENT_MEMORY_DELETE_THREAD = "delete_thread" + + # Agent Memory — Checkpointer factory + AGENT_MEMORY_CREATE_CHECKPOINTER = "create_checkpointer" + # Output Management Operations OUTPUT_MANAGEMENT_SEND_EMAIL = "send_email" OUTPUT_MANAGEMENT_SEND_EMAIL_WITH_MCP = "send_email_with_mcp" diff --git a/tests/agent_memory/integration/conftest.py b/tests/agent_memory/integration/conftest.py index 710e4699..0db9263a 100644 --- a/tests/agent_memory/integration/conftest.py +++ b/tests/agent_memory/integration/conftest.py @@ -24,6 +24,41 @@ from sap_cloud_sdk.agent_memory import AccessStrategy, create_client from sap_cloud_sdk.agent_memory.client import AgentMemoryClient from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError +from sap_cloud_sdk.agent_memory.thread_registry import SAPAgentMemoryThreadRegistry + + +def _load_env() -> None: + env_file = Path(__file__).parents[3] / ".env_integration_tests" + if env_file.exists(): + load_dotenv(env_file, override=True) + + +@pytest.fixture(scope="session", autouse=True) +def _ias_context() -> Generator[None, None, None]: + """Inject IAS_TOKEN into RuntimeContext when running in IAS mode. + + When IAS_TOKEN is set in the environment, all SDK calls made during the + test session will carry it automatically — no per-test wrapping needed. + Has no effect in XSUAA mode (IAS_TOKEN absent). + + Uses a yield fixture so the ContextVar is reset on session teardown — + prevents the IAS token leaking into unit tests when both suites share a + session (e.g. ``pytest tests/agent_memory/``). + """ + _load_env() + ias_token = os.environ.get("IAS_TOKEN") + if not ias_token: + yield + return + + from sap_cloud_sdk.core.runtime_context import IAS_TOKEN, RuntimeContext + from sap_cloud_sdk.core.runtime_context._context import _context_var + + token = _context_var.set(RuntimeContext({IAS_TOKEN: ias_token})) + try: + yield + finally: + _context_var.reset(token) @pytest.fixture(scope="session") @@ -33,14 +68,14 @@ def agent_memory_client() -> AgentMemoryClient: Uses PROVIDER as the default strategy — individual BDD steps override this per-call to exercise both PROVIDER and SUBSCRIBER scenarios. """ - env_file = Path(__file__).parents[3] / ".env_integration_tests" - if env_file.exists(): - load_dotenv(env_file, override=True) + _load_env() try: return create_client(access_strategy=AccessStrategy.PROVIDER) except AgentMemoryConfigError as e: - pytest.skip(f"Agent Memory credentials not configured — skipping integration tests: {e}") + pytest.skip( + f"Agent Memory credentials not configured — skipping integration tests: {e}" + ) except Exception as e: pytest.fail(f"Failed to create Agent Memory client for integration tests: {e}") @@ -48,11 +83,11 @@ def agent_memory_client() -> AgentMemoryClient: @pytest.fixture(scope="session") def subscriber_tenant() -> str: """Return the subscriber tenant subdomain, or skip if not configured.""" - env_file = Path(__file__).parents[3] / ".env_integration_tests" - if env_file.exists(): - load_dotenv(env_file, override=True) + _load_env() - tenant = os.environ.get("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT", "") + tenant = os.environ.get( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT", "" + ) if not tenant: pytest.skip( "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_SUBSCRIBER_TENANT not set — " @@ -79,3 +114,45 @@ def run_agent_id(agent_memory_client: AgentMemoryClient) -> Generator[str, None, agent_id = f"test-agent-{uuid.uuid4().hex[:8]}" yield agent_id _delete_all_memories(agent_memory_client, agent_id) + + +# ── Thread registry fixtures ────────────────────────────────────────────────── + + +@pytest.fixture(scope="session") +def thread_registry() -> SAPAgentMemoryThreadRegistry: + """Create a real SAPAgentMemoryThreadRegistry from environment variables. + + Skips the test session if credentials are not configured. + """ + _load_env() + + try: + from sap_cloud_sdk.agent_memory.config import _load_config_from_env + + config = _load_config_from_env() + if config is None: + pytest.skip( + "Agent Memory credentials not configured — skipping thread registry integration tests" + ) + except AgentMemoryConfigError as e: + pytest.skip( + f"Agent Memory credentials not configured — skipping thread registry integration tests: {e}" + ) + + return SAPAgentMemoryThreadRegistry(config) + + +@pytest.fixture(scope="session") +def run_thread_agent_id( + thread_registry: SAPAgentMemoryThreadRegistry, +) -> Generator[str, None, None]: + """Return a unique agent ID for thread registry tests and clean up all its threads afterwards.""" + agent_id = f"test-agent-{uuid.uuid4().hex[:8]}" + yield agent_id + page = thread_registry.list_threads(agent_id=agent_id) + for entry in page["entries"]: + try: + thread_registry.delete_thread(entry["threadId"]) + except Exception: + pass diff --git a/tests/agent_memory/integration/test_agentmemory_bdd.py b/tests/agent_memory/integration/test_agentmemory_bdd.py index ef655d2d..dbfc5c07 100644 --- a/tests/agent_memory/integration/test_agentmemory_bdd.py +++ b/tests/agent_memory/integration/test_agentmemory_bdd.py @@ -107,7 +107,9 @@ def test_get_memory_subscriber(): pass -@scenario("agentmemory.feature", "Update memory content using SUBSCRIBER access strategy") +@scenario( + "agentmemory.feature", "Update memory content using SUBSCRIBER access strategy" +) def test_update_memory_subscriber(): pass @@ -127,7 +129,9 @@ def test_search_memories_subscriber(): pass -@scenario("agentmemory.feature", "Create and get a message using SUBSCRIBER access strategy") +@scenario( + "agentmemory.feature", "Create and get a message using SUBSCRIBER access strategy" +) def test_add_message_subscriber(): pass @@ -192,7 +196,9 @@ def use_configured_subscriber_tenant(context, subscriber_tenant): def memory_exists(context, agent_memory_client, agent_id, invoker_id, content): context["client"] = agent_memory_client context["memory"] = agent_memory_client.add_memory( - context["agent_id"], context["invoker_id"], content, + context["agent_id"], + context["invoker_id"], + content, ) @@ -201,10 +207,16 @@ def memory_exists(context, agent_memory_client, agent_id, invoker_id, content): 'a message exists with agent "{agent_id}" invoker "{invoker_id}" group "{group}" role "{role}" content "{content}"' ) ) -def message_exists(context, agent_memory_client, agent_id, invoker_id, group, role, content): +def message_exists( + context, agent_memory_client, agent_id, invoker_id, group, role, content +): context["client"] = agent_memory_client context["message"] = agent_memory_client.add_message( - context["agent_id"], context["invoker_id"], group, role, content, + context["agent_id"], + context["invoker_id"], + group, + role, + content, ) @@ -213,10 +225,23 @@ def message_exists(context, agent_memory_client, agent_id, invoker_id, group, ro 'a message exists with agent "{agent_id}" invoker "{invoker_id}" group "{group}" role "{role}" content "{content}" and metadata "{metadata_value}"' ) ) -def message_exists_with_metadata(context, agent_memory_client, agent_id, invoker_id, group, role, content, metadata_value): +def message_exists_with_metadata( + context, + agent_memory_client, + agent_id, + invoker_id, + group, + role, + content, + metadata_value, +): context["client"] = agent_memory_client context["message"] = agent_memory_client.add_message( - context["agent_id"], context["invoker_id"], group, role, content, + context["agent_id"], + context["invoker_id"], + group, + role, + content, metadata={"tag": metadata_value}, ) @@ -232,7 +257,9 @@ def message_exists_with_metadata(context, agent_memory_client, agent_id, invoker def add_memory(context, agent_id, invoker_id, content): client: AgentMemoryClient = context["client"] context["memory"] = client.add_memory( - context["agent_id"], context["invoker_id"], content, + context["agent_id"], + context["invoker_id"], + content, ) @@ -248,7 +275,8 @@ def get_memory(context): def update_memory(context, content): client: AgentMemoryClient = context["client"] client.update_memory( - context["memory"].id, content=content, + context["memory"].id, + content=content, ) context["memory"] = client.get_memory( context["memory"].id, @@ -295,14 +323,16 @@ def search_memories(context, query): def add_message(context, agent_id, invoker_id, group, role, content): client: AgentMemoryClient = context["client"] context["message"] = client.add_message( - context["agent_id"], context["invoker_id"], group, MessageRole(role), content, + context["agent_id"], + context["invoker_id"], + group, + MessageRole(role), + content, ) @when( - parsers.parse( - 'I list messages filtered by agent "{agent_id}" and group "{group}"' - ) + parsers.parse('I list messages filtered by agent "{agent_id}" and group "{group}"') ) def list_messages(context, agent_id, group): client: AgentMemoryClient = context["client"] @@ -325,24 +355,21 @@ def delete_message(context): @when("I get the retention config") def get_retention_config(context): client: AgentMemoryClient = context["client"] - context["retention_config"] = client.get_retention_config( - ) + context["retention_config"] = client.get_retention_config() @when("I update the retention config with message_days 30 and memory_days 90") def update_retention_config(context): client: AgentMemoryClient = context["client"] client.update_retention_config( - message_days=30, memory_days=90, - ) - context["retention_config"] = client.get_retention_config( + message_days=30, + memory_days=90, ) + context["retention_config"] = client.get_retention_config() @when( - parsers.parse( - 'I count memories for agent "{agent_id}" and invoker "{invoker_id}"' - ) + parsers.parse('I count memories for agent "{agent_id}" and invoker "{invoker_id}"') ) def count_memories(context, agent_id, invoker_id): client: AgentMemoryClient = context["client"] diff --git a/tests/agent_memory/integration/test_thread_registry_bdd.py b/tests/agent_memory/integration/test_thread_registry_bdd.py new file mode 100644 index 00000000..51cd6a85 --- /dev/null +++ b/tests/agent_memory/integration/test_thread_registry_bdd.py @@ -0,0 +1,246 @@ +"""BDD integration tests for the Thread Registry. + +Run against a live Agent Memory service: + + pytest tests/agent_memory/integration/test_thread_registry_bdd.py + +Credentials are loaded from .env_integration_tests (same file as the memory +integration tests). All tests are automatically skipped when credentials are +absent. +""" + +import uuid + +import pytest +from pytest_bdd import given, parsers, scenario, then, when + +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryThreadConflictError, + AgentMemoryThreadNotFoundError, +) +from sap_cloud_sdk.agent_memory.thread_registry import SAPAgentMemoryThreadRegistry + +# -- Scenarios ----------------------------------------------------------------- + + +@scenario("thread_registry.feature", "Register and get a thread") +def test_register_and_get_thread(): + pass + + +@scenario("thread_registry.feature", "Create a thread with server-assigned ID") +def test_create_thread(): + pass + + +@scenario("thread_registry.feature", "List threads filtered by agent_id") +def test_list_threads(): + pass + + +@scenario("thread_registry.feature", "Update thread description") +def test_update_thread(): + pass + + +@scenario("thread_registry.feature", "Delete a thread") +def test_delete_thread(): + pass + + +@scenario( + "thread_registry.feature", + "Re-registering same thread with same owner is idempotent", +) +def test_register_idempotent(): + pass + + +@scenario( + "thread_registry.feature", + "Registering same thread with different owner raises a conflict", +) +def test_register_conflict(): + pass + + +# -- Fixtures / state ---------------------------------------------------------- + + +@pytest.fixture +def context(run_thread_agent_id): + return { + "agent_id": run_thread_agent_id, + "invoker_id": "test-user", + "thread": None, + "fetched_thread": None, + "threads": None, + "error": None, + } + + +# -- Given steps --------------------------------------------------------------- + + +@given("a configured Thread Registry") +def configured_thread_registry(context, thread_registry): + context["registry"] = thread_registry + + +@given( + parsers.parse( + 'a thread is registered with agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def thread_is_registered(context, thread_registry, agent_id, invoker_id): + context["registry"] = thread_registry + thread_id = str(uuid.uuid4()) + context["thread"] = thread_registry.create_or_register_thread( + context["agent_id"], context["invoker_id"], thread_id=thread_id + ) + + +# -- When steps ---------------------------------------------------------------- + + +@when( + parsers.parse( + 'I register a thread with agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def register_thread(context, agent_id, invoker_id): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + thread_id = str(uuid.uuid4()) + context["thread"] = registry.create_or_register_thread( + context["agent_id"], context["invoker_id"], thread_id=thread_id + ) + + +@when( + parsers.parse( + 'I register the same thread again with agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def register_same_thread_again(context, agent_id, invoker_id): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + try: + context["thread"] = registry.create_or_register_thread( + context["agent_id"], + context["invoker_id"], + thread_id=context["thread"]["threadId"], + ) + context["error"] = None + except Exception as exc: + context["error"] = exc + + +@when( + parsers.parse( + 'I register the same thread with agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def register_same_thread_different_owner(context, agent_id, invoker_id): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + try: + registry.create_or_register_thread( + agent_id, invoker_id, thread_id=context["thread"]["threadId"] + ) + context["error"] = None + except AgentMemoryThreadConflictError as exc: + context["error"] = exc + + +@when( + parsers.parse( + 'I create a thread with agent "{agent_id}" and invoker "{invoker_id}"' + ) +) +def create_thread(context, agent_id, invoker_id): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + context["thread"] = registry.create_or_register_thread( + context["agent_id"], context["invoker_id"] + ) + + +@when("I get the thread by id") +def get_thread(context): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + context["fetched_thread"] = registry.get_thread(context["thread"]["threadId"]) + + +@when(parsers.parse('I list threads filtered by agent "{agent_id}"')) +def list_threads(context, agent_id): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + context["threads"] = registry.list_threads(agent_id=context["agent_id"]) + + +@when(parsers.parse('I update the thread description to "{description}"')) +def update_thread(context, description): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + context["thread"] = registry.update_thread( + context["thread"]["threadId"], description=description + ) + + +@when("I delete the thread") +def delete_thread(context): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + registry.delete_thread(context["thread"]["threadId"]) + + +# -- Then steps ---------------------------------------------------------------- + + +@then("the thread should have a non-empty threadId") +def check_thread_id(context): + assert context["thread"]["threadId"] != "" + + +@then(parsers.parse('the thread should have agentId "{agent_id}"')) +def check_thread_agent_id(context, agent_id): + assert context["thread"]["agentId"] == context["agent_id"] + + +@then(parsers.parse('the thread should have invokerId "{invoker_id}"')) +def check_thread_invoker_id(context, invoker_id): + assert context["thread"]["invokerId"] == context["invoker_id"] + + +@then("the fetched thread should match the registered thread") +def check_fetched_thread(context): + assert context["fetched_thread"]["threadId"] == context["thread"]["threadId"] + assert context["fetched_thread"]["agentId"] == context["thread"]["agentId"] + + +@then("the result should contain at least one thread") +def check_threads_not_empty(context): + assert len(context["threads"]["entries"]) >= 1 + + +@then(parsers.parse('all returned threads should have agentId "{agent_id}"')) +def check_all_threads_agent_id(context, agent_id): + assert all( + t["agentId"] == context["agent_id"] for t in context["threads"]["entries"] + ) + + +@then(parsers.parse('the thread should have description "{description}"')) +def check_thread_description(context, description): + assert context["thread"]["description"] == description + + +@then("getting the thread should raise AgentMemoryThreadNotFoundError") +def check_thread_deleted(context): + registry: SAPAgentMemoryThreadRegistry = context["registry"] + with pytest.raises(AgentMemoryThreadNotFoundError): + registry.get_thread(context["thread"]["threadId"]) + + +@then("no error should be raised") +def check_no_error(context): + assert context["error"] is None + + +@then("a AgentMemoryThreadConflictError should be raised") +def check_ownership_conflict(context): + assert isinstance(context["error"], AgentMemoryThreadConflictError) diff --git a/tests/agent_memory/integration/thread_registry.feature b/tests/agent_memory/integration/thread_registry.feature new file mode 100644 index 00000000..2ebc1bbb --- /dev/null +++ b/tests/agent_memory/integration/thread_registry.feature @@ -0,0 +1,47 @@ +Feature: Thread Registry Integration + + Background: + Given a configured Thread Registry + + # ── Sync CRUD ───────────────────────────────────────────────────────────────── + + Scenario: Register and get a thread + When I register a thread with agent "test-agent" and invoker "test-user" + Then the thread should have a non-empty threadId + And the thread should have agentId "test-agent" + And the thread should have invokerId "test-user" + When I get the thread by id + Then the fetched thread should match the registered thread + + Scenario: Create a thread with server-assigned ID + When I create a thread with agent "test-agent" and invoker "test-user" + Then the thread should have a non-empty threadId + And the thread should have agentId "test-agent" + And the thread should have invokerId "test-user" + + Scenario: List threads filtered by agent_id + Given a thread is registered with agent "test-agent" and invoker "test-user" + When I list threads filtered by agent "test-agent" + Then the result should contain at least one thread + And all returned threads should have agentId "test-agent" + + Scenario: Update thread description + Given a thread is registered with agent "test-agent" and invoker "test-user" + When I update the thread description to "updated description" + Then the thread should have description "updated description" + + Scenario: Delete a thread + Given a thread is registered with agent "test-agent" and invoker "test-user" + When I delete the thread + Then getting the thread should raise AgentMemoryThreadNotFoundError + + Scenario: Re-registering same thread with same owner is idempotent + Given a thread is registered with agent "test-agent" and invoker "test-user" + When I register the same thread again with agent "test-agent" and invoker "test-user" + Then no error should be raised + And the thread should have agentId "test-agent" + + Scenario: Registering same thread with different owner raises a conflict + Given a thread is registered with agent "test-agent" and invoker "test-user" + When I register the same thread with agent "other-agent" and invoker "other-user" + Then a AgentMemoryThreadConflictError should be raised diff --git a/tests/agent_memory/unit/conftest.py b/tests/agent_memory/unit/conftest.py new file mode 100644 index 00000000..abd03693 --- /dev/null +++ b/tests/agent_memory/unit/conftest.py @@ -0,0 +1,39 @@ +"""Shared fixtures for agent_memory unit tests.""" + +from unittest.mock import patch + +import pytest + +_NO_CREDENTIALS = "sap_cloud_sdk.agent_memory.config._load_config_from_env" + + +@pytest.fixture(autouse=True) +def clean_runtime_context(): + """Give every unit test a clean, empty RuntimeContext. + + Unit tests must not depend on ambient process-global context. In a combined + run (e.g. ``pytest tests/agent_memory/``) the session-scoped integration + fixture sets an IAS token in the global ContextVar for the whole session, + which would otherwise leak into unit tests — breaking those that assert the + IAS guard raises when no token is present. Wrapping each test in an empty + ``sdk_context`` isolates it from any such leak, from any source. + """ + from sap_cloud_sdk.core.runtime_context import RuntimeContext + from sap_cloud_sdk.core.runtime_context._context import sdk_context + + with sdk_context(RuntimeContext({})): + yield + + +@pytest.fixture(autouse=True) +def no_credentials(request): + """Default: simulate no Agent Memory credentials so tests use in-memory path. + + Tests that need the credentials path must opt out via the + 'no_autouse_credentials' marker. + """ + if request.node.get_closest_marker("no_autouse_credentials"): + yield + return + with patch(_NO_CREDENTIALS, return_value=None): + yield diff --git a/tests/agent_memory/unit/test_client.py b/tests/agent_memory/unit/test_client.py index 9d44fe98..203a0a6a 100644 --- a/tests/agent_memory/unit/test_client.py +++ b/tests/agent_memory/unit/test_client.py @@ -75,30 +75,38 @@ def _make_subscriber_client( class TestCreateClient: - def test_uses_provided_config(self): """Factory with explicit config creates a client successfully.""" config = AgentMemoryConfig(base_url="http://localhost:3000") with patch("sap_cloud_sdk.agent_memory._build_agent_memory_http") as mock_build: - mock_build.return_value = Mock(spec=HttpClient) - client = create_client(config=config, access_strategy=AccessStrategy.PROVIDER) + mock_build.return_value = (Mock(spec=HttpClient), False) + client = create_client( + config=config, access_strategy=AccessStrategy.PROVIDER + ) assert isinstance(client, AgentMemoryClient) assert client._http is not None def test_subscriber_strategy_loads_default_binding(self, monkeypatch): """Factory with SUBSCRIBER loads the default binding (native implementation).""" import json + monkeypatch.setenv( "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", "http://memory.example.com", ) monkeypatch.setenv( "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", - json.dumps({"url": "http://auth.example.com", "clientid": "c", "clientsecret": "s", - "identityzone": "provider-zone"}), + json.dumps( + { + "url": "http://auth.example.com", + "clientid": "c", + "clientsecret": "s", + "identityzone": "provider-zone", + } + ), ) with patch("sap_cloud_sdk.agent_memory._build_agent_memory_http") as mock_build: - mock_build.return_value = Mock(spec=HttpClient) + mock_build.return_value = (Mock(spec=HttpClient), False) client = create_client( access_strategy=AccessStrategy.SUBSCRIBER, tenant="acme-corp", @@ -109,28 +117,57 @@ def test_subscriber_strategy_loads_default_binding(self, monkeypatch): def test_provider_strategy_loads_default_binding(self, monkeypatch): """Factory with PROVIDER loads the default binding.""" import json + monkeypatch.setenv( "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", "http://memory.example.com", ) monkeypatch.setenv( "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", - json.dumps({"url": "http://auth.example.com", "clientid": "c", "clientsecret": "s"}), + json.dumps( + {"url": "http://auth.example.com", "clientid": "c", "clientsecret": "s"} + ), ) with patch("sap_cloud_sdk.agent_memory._build_agent_memory_http") as mock_build: - mock_build.return_value = Mock(spec=HttpClient) + mock_build.return_value = (Mock(spec=HttpClient), False) client = create_client(access_strategy=AccessStrategy.PROVIDER) assert isinstance(client, AgentMemoryClient) assert client._http is not None + def test_url_only_binding_auto_detects_ias(self): + """URL-only binding (no token_url) auto-selects IAS: no tenant/strategy needed.""" + config = AgentMemoryConfig(base_url="http://memory.example.com") + with patch("sap_cloud_sdk.agent_memory._build_agent_memory_http") as mock_build: + mock_build.return_value = (Mock(spec=HttpClient), True) # is_ias=True + client = create_client(config=config) # defaults: SUBSCRIBER, no tenant + # IAS auto-detect: no raise despite no tenant, and no subscriber tenant stored. + assert isinstance(client, AgentMemoryClient) + assert client._tenant is None -# ── Access strategy and per-tenant transport routing ───────────────────────── + def test_url_only_binding_does_not_warn_as_provider(self, caplog): + """IAS auto-detect must not emit the PROVIDER warning.""" + config = AgentMemoryConfig(base_url="http://memory.example.com") + with patch("sap_cloud_sdk.agent_memory._build_agent_memory_http") as mock_build: + mock_build.return_value = (Mock(spec=HttpClient), True) # is_ias=True + create_client(config=config) + assert "PROVIDER" not in caplog.text + + def test_xsuaa_binding_still_requires_tenant_for_subscriber(self): + """A binding with token_url keeps the SUBSCRIBER tenant requirement.""" + config = AgentMemoryConfig( + base_url="http://memory.example.com", + token_url="http://auth.example.com/oauth/token", + client_id="c", + client_secret="s", + ) + with pytest.raises(AgentMemoryConfigError, match="tenant"): + create_client(config=config, access_strategy=AccessStrategy.SUBSCRIBER) -class TestAccessStrategy: +# ── Access strategy and per-tenant transport routing ───────────────────────── - # ── Construction-time validation ─────────────────────────────────────────── +class TestAccessStrategyTransportRouting: def test_subscriber_without_tenant_raises_at_construction(self): """SUBSCRIBER without tenant raises AgentMemoryValidationError at construction.""" http = Mock(spec=HttpClient) @@ -152,18 +189,30 @@ def test_provider_constructs_without_tenant(self): def test_client_default_subscriber_uses_subscriber_transport(self): """Client with SUBSCRIBER default uses the provided HttpClient.""" client, mock_http = _make_subscriber_client("acme") - mock_http.request.return_value = _make_response(200, json_data={ - "id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") mock_http.request.assert_called_once() def test_provider_only_uses_provider_transport(self): """PROVIDER uses the provided HttpClient.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") mock_http.request.assert_called_once() @@ -206,9 +255,15 @@ def test_subscriber_with_invalid_tenant_raises_value_error(self): def test_subscriber_passes_tenant_to_transport(self): """SUBSCRIBER client passes tenant_subdomain to the HTTP layer on every call.""" client, mock_http = _make_subscriber_client("acme") - mock_http.request.return_value = _make_response(200, json_data={ - "id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") @@ -217,9 +272,15 @@ def test_subscriber_passes_tenant_to_transport(self): def test_provider_passes_none_tenant_to_transport(self): """PROVIDER client passes tenant_subdomain=None to the HTTP layer.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") @@ -242,13 +303,16 @@ class TestMemoryCRUD: def test_add_memory_posts_correct_payload(self): """add_memory sends required and optional fields in the POST body.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "mem-1", - "agentID": "agent-a", - "invokerID": "user-b", - "content": "some memory", - "createType": "DIRECT", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "mem-1", + "agentID": "agent-a", + "invokerID": "user-b", + "content": "some memory", + "createType": "DIRECT", + }, + ) memory = client.add_memory("agent-a", "user-b", "some memory") @@ -263,9 +327,15 @@ def test_add_memory_posts_correct_payload(self): def test_add_memory_with_metadata(self): """Optional metadata is included in the POST body when provided.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "mem-1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x", metadata={"key": "val"}) @@ -274,9 +344,15 @@ def test_add_memory_with_metadata(self): def test_add_memory_excludes_none_optionals(self): """None-valued optional fields are omitted from the POST body.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "mem-1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") @@ -287,9 +363,15 @@ def test_add_memory_excludes_none_optionals(self): def test_add_memory_posts_to_memories_endpoint(self): """add_memory sends the POST to the MEMORIES endpoint.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "x", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "mem-1", + "agentID": "a", + "invokerID": "u", + "content": "x", + }, + ) client.add_memory("a", "u", "x") @@ -299,9 +381,15 @@ def test_add_memory_posts_to_memories_endpoint(self): def test_get_memory_calls_get_with_memory_id(self): """get_memory constructs the correct path with the memory ID.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "mem-1", "agentID": "a", "invokerID": "u", "content": "hello", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "mem-1", + "agentID": "a", + "invokerID": "u", + "content": "hello", + }, + ) memory = client.get_memory("mem-1") @@ -357,11 +445,19 @@ class TestListMemories: def test_returns_list_of_memories(self): """list_memories returns a list of Memory objects.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "value": [ - {"id": "m1", "agentID": "a", "invokerID": "u", "content": "memory 1"}, - ], - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "value": [ + { + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "memory 1", + }, + ], + }, + ) memories = client.list_memories(agent_id="a", invoker_id="u") @@ -502,7 +598,9 @@ class TestCountMemories: def test_returns_count_from_response(self): """count_memories returns the @odata.count value.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={"value": [], "@odata.count": 42}) + mock_http.request.return_value = _make_response( + 200, json_data={"value": [], "@odata.count": 42} + ) total = client.count_memories(agent_id="a", invoker_id="u") @@ -511,7 +609,9 @@ def test_returns_count_from_response(self): def test_sends_top_0_and_count_true(self): """count_memories uses $top=0 and $count=true.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={"value": [], "@odata.count": 0}) + mock_http.request.return_value = _make_response( + 200, json_data={"value": [], "@odata.count": 0} + ) client.count_memories() @@ -522,7 +622,9 @@ def test_sends_top_0_and_count_true(self): def test_passes_filter_when_agent_and_invoker_provided(self): """count_memories forwards agent_id and invoker_id as $filter.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={"value": [], "@odata.count": 3}) + mock_http.request.return_value = _make_response( + 200, json_data={"value": [], "@odata.count": 3} + ) client.count_memories(agent_id="agt", invoker_id="usr") @@ -547,24 +649,27 @@ class TestSearchMemories: def test_returns_results_in_api_order(self): """search_memories returns results in the order returned by the API.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "value": [ - { - "id": "m1", - "agentID": "a", - "invokerID": "u", - "content": "first", - "similarity": 0.5, - }, - { - "id": "m2", - "agentID": "a", - "invokerID": "u", - "content": "second", - "similarity": 0.9, - }, - ] - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "value": [ + { + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "first", + "similarity": 0.5, + }, + { + "id": "m2", + "agentID": "a", + "invokerID": "u", + "content": "second", + "similarity": 0.9, + }, + ] + }, + ) results = client.search_memories("a", "u", "test query") @@ -618,14 +723,17 @@ class TestMessageCRUD: def test_add_message_posts_correct_payload(self): """add_message sends required fields in the POST body.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "msg-1", - "agentID": "agent-a", - "invokerID": "user-b", - "messageGroup": "conv-1", - "role": "USER", - "content": "Hello!", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "msg-1", + "agentID": "agent-a", + "invokerID": "user-b", + "messageGroup": "conv-1", + "role": "USER", + "content": "Hello!", + }, + ) message = client.add_message( "agent-a", @@ -648,10 +756,17 @@ def test_add_message_posts_correct_payload(self): def test_add_message_posts_to_messages_endpoint(self): """add_message sends the POST to the MESSAGES endpoint.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", + }, + ) client.add_message("a", "u", "g", MessageRole.USER, "hi") @@ -661,11 +776,18 @@ def test_add_message_posts_to_messages_endpoint(self): def test_add_message_with_metadata(self): """Optional metadata is included when provided.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", - "metadata": {"key": "val"}, - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", + "metadata": {"key": "val"}, + }, + ) client.add_message( "a", "u", "g", MessageRole.USER, "hi", metadata={"key": "val"} @@ -676,10 +798,17 @@ def test_add_message_with_metadata(self): def test_add_message_excludes_none_metadata(self): """None-valued metadata is omitted from the POST body.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", + }, + ) client.add_message("a", "u", "g", MessageRole.USER, "hi") @@ -688,10 +817,17 @@ def test_add_message_excludes_none_metadata(self): def test_get_message_calls_get_with_message_id(self): """get_message constructs the correct path with the message ID.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", + }, + ) message = client.get_message("msg-1") @@ -717,18 +853,21 @@ class TestListMessages: def test_returns_list_of_messages(self): """list_messages returns a list of Message objects.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "value": [ - { - "id": "msg-1", - "agentID": "a", - "invokerID": "u", - "messageGroup": "g", - "role": "USER", - "content": "hi", - }, - ], - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "value": [ + { + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", + }, + ], + }, + ) messages = client.list_messages(agent_id="a", invoker_id="u") @@ -884,12 +1023,17 @@ class TestRetentionConfig: def test_get_retention_config(self): """get_retention_config sends GET to the retentionConfig endpoint.""" client, mock_http = _make_client() - mock_http.request.return_value = _make_response(200, json_data={ - "id": 1, "messageDays": 30, "memoryDays": 90, - "usageLogDays": 180, - "createTimestamp": "2025-01-01T00:00:00Z", - "updateTimestamp": "2025-01-02T00:00:00Z", - }) + mock_http.request.return_value = _make_response( + 200, + json_data={ + "id": 1, + "messageDays": 30, + "memoryDays": 90, + "usageLogDays": 180, + "createTimestamp": "2025-01-01T00:00:00Z", + "updateTimestamp": "2025-01-02T00:00:00Z", + }, + ) rc = client.get_retention_config() @@ -926,12 +1070,22 @@ def test_update_retention_config_excludes_none_fields(self): assert payload["memoryDays"] == 90 assert payload["usageLogDays"] == 180 + def test_update_retention_config_thread_days(self): + """update_retention_config sends threadDays when provided.""" + client, mock_http = _make_client() + mock_http.request.return_value = _make_response(204, content=b"") + + client.update_retention_config(thread_days=45) + + payload = mock_http.request.call_args[1]["json"] + assert payload["threadDays"] == 45 + assert "messageDays" not in payload + # ── Context manager ─────────────────────────────────────────────────────────── class TestContextManager: - def test_close_delegates_to_http(self): """close() delegates to the HttpClient's close method.""" client, mock_http = _make_client() @@ -1158,3 +1312,106 @@ def test_list_messages_raises_for_empty_contains(self): client.list_messages( filters=[FilterDefinition(target="metadata", contains="")], ) + + +# ── IAS mode: per-request token guard ────────────────────────────────────────── + + +class TestIASMode: + """IAS-mode client raises before dispatch when no token is in context.""" + + def _make_ias_client(self) -> tuple[AgentMemoryClient, Mock]: + http = Mock(spec=HttpClient) + http.request.return_value = _make_response(200) + return AgentMemoryClient(http, _ias_mode=True), http + + def test_request_raises_without_ias_token(self): + """No IAS_TOKEN in context → AgentMemoryValidationError, no HTTP call.""" + from sap_cloud_sdk.core.runtime_context._context import sdk_context + from sap_cloud_sdk.core.runtime_context import RuntimeContext + + client, http = self._make_ias_client() + with sdk_context(RuntimeContext({})): + with pytest.raises(AgentMemoryValidationError, match=r"bootstrap\(app\)"): + client.list_memories() + http.request.assert_not_called() + + def test_request_succeeds_with_ias_token(self): + """IAS_TOKEN present → guard passes, request is dispatched.""" + from sap_cloud_sdk.core.runtime_context._context import sdk_context + from sap_cloud_sdk.core.runtime_context import RuntimeContext + from sap_cloud_sdk.core.runtime_context.providers._ias import IAS_TOKEN + + client, http = self._make_ias_client() + http.request.return_value = _make_response(200, json_data={"value": []}) + with sdk_context(RuntimeContext({IAS_TOKEN: "ias-token-xyz"})): + client.list_memories() + http.request.assert_called_once() + + +# --------------------------------------------------------------------------- +# XSUAA config validation +# --------------------------------------------------------------------------- + + +class TestXsuaaConfigValidation: + """Construction-time XSUAA credential and access strategy validation.""" + + def test_partial_xsuaa_config_raises_at_construction(self): + """Partial XSUAA creds (token_url only) must raise AgentMemoryConfigError.""" + from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig + from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError + + with pytest.raises(AgentMemoryConfigError, match="Partial XSUAA"): + AgentMemoryConfig( + base_url="http://memory.example.com", + token_url="http://auth.example.com/oauth/token", + # client_id and client_secret deliberately omitted + ) + + def test_full_xsuaa_config_constructs_successfully(self): + """Full XSUAA creds must not raise.""" + from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig + + cfg = AgentMemoryConfig( + base_url="http://memory.example.com", + token_url="http://auth.example.com/oauth/token", + client_id="cid", + client_secret="csecret", + ) + assert cfg.token_url is not None + + def test_ias_only_config_constructs_successfully(self): + """URL-only IAS config (no XSUAA creds) must not raise.""" + from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig + + cfg = AgentMemoryConfig(base_url="http://memory.example.com") + assert cfg.token_url is None + assert cfg.client_id is None + assert cfg.client_secret is None + + def test_xsuaa_subscriber_with_tenant_emits_no_provider_warning(self, caplog): + """XSUAA SUBSCRIBER with tenant must not emit the PROVIDER warning.""" + import logging + + http = Mock(spec=HttpClient) + with caplog.at_level(logging.WARNING): + AgentMemoryClient( + http, + access_strategy=AccessStrategy.SUBSCRIBER, + tenant="my-tenant", + ) + assert not any("PROVIDER" in r.message for r in caplog.records) + + def test_xsuaa_provider_without_ias_mode_emits_warning(self, caplog): + """XSUAA PROVIDER strategy (non-IAS) warns once at construction.""" + import logging + + http = Mock(spec=HttpClient) + with caplog.at_level(logging.WARNING): + AgentMemoryClient( + http, + access_strategy=AccessStrategy.PROVIDER, + _ias_mode=False, + ) + assert any("PROVIDER is active" in r.message for r in caplog.records) diff --git a/tests/agent_memory/unit/test_config.py b/tests/agent_memory/unit/test_config.py index 40e9b2ea..825d039b 100644 --- a/tests/agent_memory/unit/test_config.py +++ b/tests/agent_memory/unit/test_config.py @@ -12,11 +12,13 @@ ) from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError -_VALID_UAA = json.dumps({ - "url": "https://auth.example.com", - "clientid": "my-client", - "clientsecret": "my-secret", -}) +_VALID_UAA = json.dumps( + { + "url": "https://auth.example.com", + "clientid": "my-client", + "clientsecret": "my-secret", + } +) _RESOLVER = "sap_cloud_sdk.core.secret_resolver.read_from_mount_and_fallback_to_env_var" @@ -59,6 +61,23 @@ def test_identityzone_defaults_to_none(self): config = AgentMemoryConfig(base_url="http://localhost:8080") assert config.identityzone is None + def test_raises_when_identityzone_set_without_token_url(self): + # identityzone is XSUAA-only (derives the subscriber token URL); it must + # not be set in IAS mode (no token_url/client_id/client_secret). + with pytest.raises(AgentMemoryConfigError, match="identityzone applies only to XSUAA"): + AgentMemoryConfig(base_url="http://localhost", identityzone="my-zone") + + def test_identityzone_allowed_with_full_xsuaa_credentials(self): + config = AgentMemoryConfig( + base_url="https://memory.example.com", + token_url="https://auth.example.com/oauth/token", + client_id="my-client", + client_secret="my-secret", + identityzone="my-zone", + ) + assert config.identityzone == "my-zone" + assert config.auth_mode == "xsuaa" + def test_valid_config_with_all_fields_does_not_raise(self): AgentMemoryConfig( base_url="https://memory.example.com", @@ -67,6 +86,28 @@ def test_valid_config_with_all_fields_does_not_raise(self): client_secret="my-secret", ) + def test_auth_mode_derived_as_ias_when_no_token_url(self): + config = AgentMemoryConfig(base_url="http://localhost:3000") + assert config.auth_mode == "ias" + + def test_auth_mode_derived_as_xsuaa_when_token_url_present(self): + config = AgentMemoryConfig( + base_url="https://memory.example.com", + token_url="https://auth.example.com/oauth/token", + client_id="my-client", + client_secret="my-secret", + ) + assert config.auth_mode == "xsuaa" + + def test_auth_mode_is_read_only(self): + # auth_mode is a computed property, never a settable/stored field, so it + # can never contradict the credentials. + config = AgentMemoryConfig(base_url="http://localhost:3000") + with pytest.raises(AttributeError): + config.auth_mode = "xsuaa" # type: ignore[misc] + with pytest.raises(TypeError): + AgentMemoryConfig(base_url="http://localhost:3000", auth_mode="xsuaa") # type: ignore[call-arg] + # ── BindingData ─────────────────────────────────────────────────────────────── @@ -76,67 +117,112 @@ def test_validate_raises_when_application_url_missing(self): with pytest.raises(AgentMemoryConfigError, match="application_url"): BindingData(application_url="", uaa=_VALID_UAA).validate() - def test_validate_raises_when_uaa_missing(self): - with pytest.raises(AgentMemoryConfigError, match="uaa"): - BindingData(application_url="https://memory.example.com", uaa="").validate() + def test_validate_passes_when_uaa_absent(self): + # URL-only IAS binding: no uaa required + BindingData(application_url="https://memory.example.com", uaa="").validate() + + def test_validate_passes_when_uaa_none(self): + # None is the canonical "absent" value for the Optional[str] field + BindingData(application_url="https://memory.example.com", uaa=None).validate() + + def test_uaa_defaults_to_none(self): + # Resolver leaves uaa=None when only APPLICATION_URL is present (IAS mode) + binding = BindingData(application_url="https://memory.example.com") + assert binding.uaa is None def test_validate_passes_when_all_fields_set(self): - BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).validate() + BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).validate() def test_extract_config_maps_url(self): - config = BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).extract_config() + config = BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).extract_config() assert config.base_url == "https://memory.example.com" def test_extract_config_derives_token_url(self): - config = BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).extract_config() + config = BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).extract_config() assert config.token_url == "https://auth.example.com/oauth/token" + def test_extract_config_sets_xsuaa_auth_mode_when_uaa_present(self): + config = BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).extract_config() + assert config.auth_mode == "xsuaa" + def test_extract_config_strips_trailing_slash_from_uaa_url(self): - uaa = json.dumps({"url": "https://auth.example.com/", "clientid": "c", "clientsecret": "s"}) - config = BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + uaa = json.dumps( + {"url": "https://auth.example.com/", "clientid": "c", "clientsecret": "s"} + ) + config = BindingData( + application_url="https://memory.example.com", uaa=uaa + ).extract_config() assert config.token_url == "https://auth.example.com/oauth/token" def test_extract_config_maps_client_credentials(self): - config = BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).extract_config() + config = BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).extract_config() assert config.client_id == "my-client" assert config.client_secret == "my-secret" def test_extract_config_raises_on_invalid_json(self): with pytest.raises(AgentMemoryConfigError, match="Failed to parse uaa JSON"): - BindingData(application_url="https://memory.example.com", uaa="not-json").extract_config() + BindingData( + application_url="https://memory.example.com", uaa="not-json" + ).extract_config() def test_extract_config_raises_on_missing_json_key(self): - uaa = json.dumps({"url": "https://auth.example.com"}) # missing clientid/clientsecret - with pytest.raises(AgentMemoryConfigError, match="Missing required field in uaa JSON"): - BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + uaa = json.dumps( + {"url": "https://auth.example.com"} + ) # missing clientid/clientsecret + with pytest.raises( + AgentMemoryConfigError, match="Missing required field in uaa JSON" + ): + BindingData( + application_url="https://memory.example.com", uaa=uaa + ).extract_config() def test_extract_config_maps_identityzone_when_present(self): - uaa = json.dumps({ - "url": "https://my-zone.authentication.eu12.hana.ondemand.com", - "clientid": "c", - "clientsecret": "s", - "identityzone": "my-zone", - }) - config = BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + uaa = json.dumps( + { + "url": "https://my-zone.authentication.eu12.hana.ondemand.com", + "clientid": "c", + "clientsecret": "s", + "identityzone": "my-zone", + } + ) + config = BindingData( + application_url="https://memory.example.com", uaa=uaa + ).extract_config() assert config.identityzone == "my-zone" def test_extract_config_identityzone_is_none_when_absent(self): - config = BindingData(application_url="https://memory.example.com", uaa=_VALID_UAA).extract_config() + config = BindingData( + application_url="https://memory.example.com", uaa=_VALID_UAA + ).extract_config() assert config.identityzone is None def test_extract_config_ignores_extra_uaa_fields(self): - uaa = json.dumps({ - "apiurl": "https://api.authentication.eu12.hana.ondemand.com", - "clientid": "my-client", - "clientsecret": "my-secret", - "credential-type": "binding-secret", - "identityzone": "my-zone", - "tenantid": "tenant-123", - "url": "https://auth.example.com", - "xsappname": "my-app", - "zoneid": "1acb547d-6df6-40a6-abb6-e41dd7d079d1", - }) - config = BindingData(application_url="https://memory.example.com", uaa=uaa).extract_config() + uaa = json.dumps( + { + "apiurl": "https://api.authentication.eu12.hana.ondemand.com", + "clientid": "my-client", + "clientsecret": "my-secret", + "credential-type": "binding-secret", + "identityzone": "my-zone", + "tenantid": "tenant-123", + "url": "https://auth.example.com", + "xsappname": "my-app", + "zoneid": "1acb547d-6df6-40a6-abb6-e41dd7d079d1", + } + ) + config = BindingData( + application_url="https://memory.example.com", uaa=uaa + ).extract_config() assert config.base_url == "https://memory.example.com" assert config.token_url == "https://auth.example.com/oauth/token" assert config.client_id == "my-client" @@ -144,8 +230,44 @@ def test_extract_config_ignores_extra_uaa_fields(self): assert config.identityzone == "my-zone" def test_extract_config_raises_on_empty_uaa_object(self): - with pytest.raises(AgentMemoryConfigError, match="Missing required field in uaa JSON"): - BindingData(application_url="https://memory.example.com", uaa="{}").extract_config() + with pytest.raises( + AgentMemoryConfigError, match="Missing required field in uaa JSON" + ): + BindingData( + application_url="https://memory.example.com", uaa="{}" + ).extract_config() + + def test_extract_config_url_only_when_uaa_absent(self): + # URL-only IAS binding: returns base_url-only config, OAuth2 fields None + config = BindingData( + application_url="https://memory.example.com", uaa="" + ).extract_config() + assert config.base_url == "https://memory.example.com" + assert config.token_url is None + assert config.client_id is None + assert config.client_secret is None + + def test_extract_config_sets_ias_auth_mode_when_uaa_absent(self): + config = BindingData( + application_url="https://memory.example.com", uaa="" + ).extract_config() + assert config.auth_mode == "ias" + # None is the canonical absent value after the Optional[str] migration + config = BindingData( + application_url="https://memory.example.com", uaa=None + ).extract_config() + assert config.base_url == "https://memory.example.com" + assert config.token_url is None + assert config.client_id is None + assert config.client_secret is None + + def test_extract_config_url_only_when_uaa_uses_default(self): + # No uaa argument at all — field defaults to None + config = BindingData( + application_url="https://memory.example.com" + ).extract_config() + assert config.base_url == "https://memory.example.com" + assert config.token_url is None # ── _load_config_from_env ───────────────────────────────────────────────────── @@ -162,6 +284,7 @@ def test_success_via_resolver(self): with patch(_RESOLVER, side_effect=_fill_binding): config = _load_config_from_env() + assert config is not None assert config.base_url == "https://memory.example.com" assert config.token_url == "https://auth.example.com/oauth/token" assert config.client_id == "my-client" @@ -179,34 +302,87 @@ def test_calls_resolver_with_correct_arguments(self): assert kwargs["instance"] == "default" def test_falls_back_to_env_vars(self, monkeypatch): - monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", "https://memory.example.com") + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", + "https://memory.example.com", + ) monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", _VALID_UAA) # Let the real resolver run — mount will fail, env vars will succeed with patch("os.stat", side_effect=FileNotFoundError("no mount")): config = _load_config_from_env() + assert config is not None assert config.base_url == "https://memory.example.com" assert config.client_id == "my-client" - def test_raises_config_error_when_resolver_fails(self): + def test_returns_none_when_no_binding_found(self): + with patch( + _RESOLVER, return_value=None + ): # resolver fills nothing → both fields stay "" + result = _load_config_from_env() + assert result is None + + def test_returns_none_when_resolver_finds_no_credentials(self): + # RuntimeError from the resolver means "no credentials configured anywhere". + # _load_config_from_env must return None so the factory falls back to + # InMemory — the factory checks `if config is not None` and has no + # try/except around _load_config_from_env(), so raising here would crash it. with patch(_RESOLVER, side_effect=RuntimeError("both sources failed")): - with pytest.raises(AgentMemoryConfigError, match="Failed to load Agent Memory configuration"): - _load_config_from_env() + result = _load_config_from_env() + assert result is None def test_raises_config_error_when_binding_incomplete(self): def partial_fill(**kwargs): - kwargs["target"].application_url = "https://memory.example.com" - # uaa remains empty → validate() raises + # application_url missing → validate() raises + kwargs["target"].uaa = _VALID_UAA with patch(_RESOLVER, side_effect=partial_fill): - with pytest.raises(AgentMemoryConfigError, match="uaa"): + with pytest.raises(AgentMemoryConfigError, match="application_url"): _load_config_from_env() + def test_load_config_from_env_url_only_binding(self): + def fill_url_only(**kwargs): + kwargs["target"].application_url = "https://memory.example.com" + # uaa intentionally absent — IAS mode + + with patch(_RESOLVER, side_effect=fill_url_only): + config = _load_config_from_env() + + assert config is not None + assert config.base_url == "https://memory.example.com" + assert config.token_url is None + assert config.client_id is None + def test_raises_config_error_when_uaa_json_invalid(self, monkeypatch): - monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", "https://memory.example.com") - monkeypatch.setenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", "not-valid-json") + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", + "https://memory.example.com", + ) + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", "not-valid-json" + ) with patch("os.stat", side_effect=FileNotFoundError("no mount")): - with pytest.raises(AgentMemoryConfigError, match="Failed to parse uaa JSON"): + with pytest.raises( + AgentMemoryConfigError, match="Failed to parse uaa JSON" + ): _load_config_from_env() + + def test_ias_url_only_from_env_vars(self, monkeypatch): + # IAS binding: only APPLICATION_URL env var present, no UAA — real resolver path + monkeypatch.setenv( + "CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_APPLICATION_URL", + "https://memory.example.com", + ) + # Ensure UAA is absent even if set in the outer environment + monkeypatch.delenv("CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_UAA", raising=False) + + with patch("os.stat", side_effect=FileNotFoundError("no mount")): + config = _load_config_from_env() + + assert config is not None + assert config.base_url == "https://memory.example.com" + assert config.token_url is None + assert config.client_id is None + assert config.client_secret is None diff --git a/tests/agent_memory/unit/test_exceptions.py b/tests/agent_memory/unit/test_exceptions.py index b2a8a230..4353b71e 100644 --- a/tests/agent_memory/unit/test_exceptions.py +++ b/tests/agent_memory/unit/test_exceptions.py @@ -9,7 +9,6 @@ class TestAgentMemoryError: - def test_is_base_exception(self): assert issubclass(AgentMemoryError, Exception) @@ -23,7 +22,6 @@ def test_can_be_raised_and_caught(self): class TestAgentMemoryConfigError: - def test_is_agent_memory_error(self): assert issubclass(AgentMemoryConfigError, AgentMemoryError) @@ -37,7 +35,6 @@ def test_caught_as_base_error(self): class TestAgentMemoryHttpError: - def test_is_agent_memory_error(self): assert issubclass(AgentMemoryHttpError, AgentMemoryError) @@ -72,7 +69,6 @@ def test_caught_as_base_error(self): class TestAgentMemoryNotFoundError: - def test_is_http_error(self): assert issubclass(AgentMemoryNotFoundError, AgentMemoryHttpError) diff --git a/tests/agent_memory/unit/test_langgraph_checkpointer.py b/tests/agent_memory/unit/test_langgraph_checkpointer.py index 5885961c..0830e540 100644 --- a/tests/agent_memory/unit/test_langgraph_checkpointer.py +++ b/tests/agent_memory/unit/test_langgraph_checkpointer.py @@ -10,7 +10,9 @@ from langgraph.graph import END, START, StateGraph from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer +from sap_cloud_sdk.core.telemetry import Module, Operation +_REQUEST_METRIC = "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" class TestCreateCheckpointer: """Tests for create_checkpointer() factory.""" @@ -22,21 +24,41 @@ def test_returns_in_memory_saver(self): result = create_checkpointer() assert isinstance(result, InMemorySaver) - def test_ttl_seconds_still_returns_in_memory_saver(self): - """ttl_seconds returns TimedInMemorySaver.""" + def test_ttl_returns_timed_in_memory_saver(self): + """Passing ttl_seconds returns a TimedInMemorySaver (deprecated path).""" from sap_cloud_sdk.agent_memory.factory._timed_memory import TimedInMemorySaver - result = create_checkpointer(ttl_seconds=3600) + + with pytest.warns(DeprecationWarning, match="ttl_seconds parameter is deprecated"): + result = create_checkpointer(ttl_seconds=3600) assert isinstance(result, TimedInMemorySaver) + assert result.ttl_seconds == 3600 + + def test_no_ttl_does_not_warn(self): + """The default (no ttl_seconds) path is not deprecated and does not warn.""" + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + create_checkpointer() + + # ── Deprecation telemetry ───────────────────────────────────────────────── - def test_ttl_seconds_logs_warning(self, caplog): - """ttl_seconds logs a warning about in-process state.""" - import logging - with caplog.at_level( - logging.WARNING, - logger="sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint", - ): + def test_ttl_path_records_deprecated_metric(self): + """The deprecated ttl_seconds path records a single deprecated=True metric.""" + with patch(_REQUEST_METRIC) as metric: create_checkpointer(ttl_seconds=3600) - assert "TimedInMemorySaver" in caplog.text + metric.assert_called_once_with( + Module.AGENT_MEMORY, + None, + Operation.AGENT_MEMORY_CREATE_CHECKPOINTER, + True, + ) + + def test_no_ttl_records_no_metric(self): + """A bare create_checkpointer() is not deprecated and emits no metric.""" + with patch(_REQUEST_METRIC) as metric: + create_checkpointer() + metric.assert_not_called() # ── Missing langgraph ───────────────────────────────────────────────────── @@ -57,6 +79,7 @@ def mock_import(name, *args, **kwargs): def test_checkpointer_compiles_with_langgraph_graph(self): """Returned checkpointer can compile a LangGraph StateGraph.""" + class SimpleState(TypedDict): value: str @@ -73,6 +96,7 @@ def noop(state: SimpleState) -> SimpleState: def test_checkpointer_persists_state_across_invocations(self): """State is preserved across invocations on the same thread_id.""" + class TickState(TypedDict): values: list @@ -95,6 +119,7 @@ def append_node(state: TickState) -> TickState: def test_different_thread_ids_are_isolated(self): """Two thread IDs maintain independent state.""" + class NameState(TypedDict): name: str diff --git a/tests/agent_memory/unit/test_models.py b/tests/agent_memory/unit/test_models.py index aa159e99..2fd9d6fc 100644 --- a/tests/agent_memory/unit/test_models.py +++ b/tests/agent_memory/unit/test_models.py @@ -13,7 +13,6 @@ class TestMemory: - def test_from_dict_maps_known_fields(self): """All known API fields are mapped to model attributes.""" data = { @@ -45,15 +44,25 @@ def test_from_dict_optional_fields_default_to_none(self): def test_from_dict_parses_string_metadata(self): """String metadata is parsed as JSON.""" - data = {"id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - "metadata": '{"key": "val"}'} + data = { + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + "metadata": '{"key": "val"}', + } memory = Memory.from_dict(data) assert memory.metadata == {"key": "val"} def test_from_dict_handles_invalid_json_metadata(self): """Invalid JSON metadata is wrapped in a raw dict.""" - data = {"id": "m1", "agentID": "a", "invokerID": "u", "content": "x", - "metadata": "not-json"} + data = { + "id": "m1", + "agentID": "a", + "invokerID": "u", + "content": "x", + "metadata": "not-json", + } memory = Memory.from_dict(data) assert memory.metadata == {"raw": "not-json"} @@ -67,7 +76,10 @@ def test_from_dict_empty_dict_uses_defaults(self): def test_to_dict_shape(self): """to_dict outputs all fields with camelCase keys.""" memory = Memory( - id="m1", agent_id="a", invoker_id="u", content="hello", + id="m1", + agent_id="a", + invoker_id="u", + content="hello", create_timestamp="2025-01-01T00:00:00Z", update_timestamp="2025-01-02T00:00:00Z", ) @@ -84,7 +96,10 @@ def test_to_dict_shape(self): def test_to_dict_includes_metadata_when_set(self): """to_dict includes metadata when set.""" memory = Memory( - id="m1", agent_id="a", invoker_id="u", content="hello", + id="m1", + agent_id="a", + invoker_id="u", + content="hello", metadata={"k": "v"}, ) d = memory.to_dict() @@ -115,7 +130,6 @@ def test_round_trip(self): class TestSearchResult: - def test_from_dict_maps_all_fields(self): """All known fields including similarity score are mapped correctly.""" data = { @@ -146,7 +160,10 @@ def test_from_dict_similarity_defaults_to_none(self): def test_to_dict_includes_similarity(self): """to_dict includes the similarity score.""" result = SearchResult( - id="m1", agent_id="a", invoker_id="u", content="x", + id="m1", + agent_id="a", + invoker_id="u", + content="x", similarity=0.85, ) d = result.to_dict() @@ -172,7 +189,6 @@ def test_round_trip(self): class TestMessage: - def test_from_dict_maps_known_fields(self): """All known message fields are mapped correctly.""" data = { @@ -199,8 +215,12 @@ def test_from_dict_maps_known_fields(self): def test_from_dict_parses_string_metadata(self): """String metadata is parsed as JSON.""" data = { - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", "metadata": '{"key": "val"}', } msg = Message.from_dict(data) @@ -209,8 +229,12 @@ def test_from_dict_parses_string_metadata(self): def test_from_dict_handles_invalid_json_metadata(self): """Invalid JSON metadata is wrapped in a raw dict.""" data = { - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", "metadata": "not-json", } msg = Message.from_dict(data) @@ -219,8 +243,12 @@ def test_from_dict_handles_invalid_json_metadata(self): def test_from_dict_handles_missing_metadata(self): """Missing metadata defaults to None.""" data = { - "id": "msg-1", "agentID": "a", "invokerID": "u", - "messageGroup": "g", "role": "USER", "content": "hi", + "id": "msg-1", + "agentID": "a", + "invokerID": "u", + "messageGroup": "g", + "role": "USER", + "content": "hi", } msg = Message.from_dict(data) assert msg.metadata is None @@ -236,8 +264,12 @@ def test_from_dict_empty_dict_uses_defaults(self): def test_to_dict_shape(self): """to_dict outputs all fields with camelCase keys.""" msg = Message( - id="msg-1", agent_id="a", invoker_id="u", - message_group="g", role=MessageRole.USER, content="hi", + id="msg-1", + agent_id="a", + invoker_id="u", + message_group="g", + role=MessageRole.USER, + content="hi", create_timestamp="2025-01-01T00:00:00Z", ) d = msg.to_dict() @@ -254,8 +286,12 @@ def test_to_dict_shape(self): def test_to_dict_includes_metadata_when_set(self): """to_dict includes metadata when set.""" msg = Message( - id="msg-1", agent_id="a", invoker_id="u", - message_group="g", role=MessageRole.USER, content="hi", + id="msg-1", + agent_id="a", + invoker_id="u", + message_group="g", + role=MessageRole.USER, + content="hi", metadata={"key": "val"}, ) d = msg.to_dict() @@ -264,8 +300,11 @@ def test_to_dict_includes_metadata_when_set(self): def test_to_dict_omits_none_role_and_metadata(self): """to_dict omits role and metadata when None.""" msg = Message( - id="msg-1", agent_id="a", invoker_id="u", - message_group="g", content="hi", + id="msg-1", + agent_id="a", + invoker_id="u", + message_group="g", + content="hi", ) d = msg.to_dict() assert "role" not in d @@ -291,7 +330,6 @@ def test_round_trip(self): class TestEnums: - def test_message_role_values(self): """MessageRole enum has the expected members.""" assert MessageRole.USER == "USER" @@ -309,7 +347,6 @@ def test_message_role_is_str(self): class TestRetentionConfig: - def test_from_dict_maps_known_fields(self): """All known retention config fields are mapped correctly.""" data = { @@ -317,6 +354,8 @@ def test_from_dict_maps_known_fields(self): "messageDays": 30, "memoryDays": 90, "usageLogDays": 180, + "threadDays": 45, + "maxCheckpointsPerThread": 10, "createTimestamp": "2025-01-01T00:00:00Z", "updateTimestamp": "2025-01-02T00:00:00Z", } @@ -326,6 +365,8 @@ def test_from_dict_maps_known_fields(self): assert rc.message_days == 30 assert rc.memory_days == 90 assert rc.usage_log_days == 180 + assert rc.thread_days == 45 + assert rc.max_checkpoints_per_thread == 10 assert rc.create_timestamp == "2025-01-01T00:00:00Z" assert rc.update_timestamp == "2025-01-02T00:00:00Z" @@ -336,6 +377,8 @@ def test_from_dict_optional_fields_default_to_none(self): assert rc.message_days is None assert rc.memory_days is None assert rc.usage_log_days is None + assert rc.thread_days is None + assert rc.max_checkpoints_per_thread is None assert rc.create_timestamp is None assert rc.update_timestamp is None @@ -352,6 +395,8 @@ def test_round_trip(self): "messageDays": 30, "memoryDays": 90, "usageLogDays": 180, + "threadDays": 45, + "maxCheckpointsPerThread": 10, "createTimestamp": "2025-01-01T00:00:00Z", "updateTimestamp": "2025-01-02T00:00:00Z", } diff --git a/tests/agent_memory/unit/test_odata.py b/tests/agent_memory/unit/test_odata.py index deda8f05..b34387e1 100644 --- a/tests/agent_memory/unit/test_odata.py +++ b/tests/agent_memory/unit/test_odata.py @@ -15,7 +15,6 @@ class TestEscapeOdataString: - def test_single_quote_is_doubled(self): """Single quotes are escaped as '' per OData 4.01.""" assert _escape_odata_string("it's") == "it''s" @@ -37,18 +36,21 @@ def test_empty_string(self): class TestBuildContainsClauses: - def test_single_clause(self): """A single clause produces one contains() expression.""" - result = build_contains_clauses([FilterDefinition(target="metadata", contains="john")]) + result = build_contains_clauses( + [FilterDefinition(target="metadata", contains="john")] + ) assert result == ["contains(metadata, 'john')"] def test_multiple_clauses(self): """Multiple clauses produce one expression per clause.""" - result = build_contains_clauses([ - FilterDefinition(target="metadata", contains="john"), - FilterDefinition(target="content", contains="user prefers"), - ]) + result = build_contains_clauses( + [ + FilterDefinition(target="metadata", contains="john"), + FilterDefinition(target="content", contains="user prefers"), + ] + ) assert result == [ "contains(metadata, 'john')", "contains(content, 'user prefers')", @@ -56,7 +58,9 @@ def test_multiple_clauses(self): def test_single_quote_in_value_is_escaped(self): """Single quotes inside the contains value are escaped.""" - result = build_contains_clauses([FilterDefinition(target="content", contains="it's")]) + result = build_contains_clauses( + [FilterDefinition(target="content", contains="it's")] + ) assert result == ["contains(content, 'it''s')"] def test_empty_clause_list_returns_empty_list(self): @@ -68,7 +72,6 @@ def test_empty_clause_list_returns_empty_list(self): class TestBuildMemoryFilter: - def test_agent_id_only(self): """Single agent_id produces a simple eq filter.""" result = build_memory_filter(agent_id="agent-a") @@ -118,12 +121,13 @@ def test_invoker_id_with_single_quote_is_escaped(self): class TestBuildMessageFilter: - def test_all_filters(self): """All four message filters are combined.""" result = build_message_filter( - agent_id="a", invoker_id="u", - message_group="g", role="USER", + agent_id="a", + invoker_id="u", + message_group="g", + role="USER", ) assert result is not None assert "agentID eq 'a'" in result @@ -177,7 +181,6 @@ def test_role_with_single_quote_is_escaped(self): class TestBuildListParams: - def test_filter_param(self): """filter_expr is output as $filter.""" params = build_list_params(filter_expr="agentID eq 'a'") @@ -258,7 +261,6 @@ def test_all_params(self): class TestExtractValueAndCount: - def test_data_format(self): """Parses the 'data' + 'count' response format.""" response = { diff --git a/tests/agent_memory/unit/test_thread_registry_factory.py b/tests/agent_memory/unit/test_thread_registry_factory.py new file mode 100644 index 00000000..253f3371 --- /dev/null +++ b/tests/agent_memory/unit/test_thread_registry_factory.py @@ -0,0 +1,79 @@ +"""Unit tests for create_thread_registry() factory.""" + +import logging +from unittest.mock import patch + +import pytest + +from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig +from sap_cloud_sdk.agent_memory.exceptions import AgentMemoryConfigError +from sap_cloud_sdk.agent_memory.thread_registry import ( + InMemoryThreadRegistry, + SAPAgentMemoryThreadRegistry, +) +from sap_cloud_sdk.agent_memory.thread_registry._factory import create_thread_registry + +_NO_CREDENTIALS = "sap_cloud_sdk.agent_memory.config._load_config_from_env" +_VALID_CONFIG = AgentMemoryConfig( + base_url="https://agent-memory.example.com", + token_url="https://tenant.authentication.region.hana.ondemand.com/oauth/token", + client_id="client-id", + client_secret="client-secret", +) + + +class TestCreateThreadRegistryNoCredentials: + """create_thread_registry() with no credentials returns InMemoryThreadRegistry.""" + + def test_returns_in_memory_when_no_credentials(self): + result = create_thread_registry() + assert isinstance(result, InMemoryThreadRegistry) + + def test_ttl_seconds_stored_on_in_memory_registry(self): + result = create_thread_registry(ttl_seconds=3600) + assert isinstance(result, InMemoryThreadRegistry) + assert result._default_ttl == 3600 + + def test_no_ttl_default_is_none(self): + result = create_thread_registry() + assert isinstance(result, InMemoryThreadRegistry) + assert result._default_ttl is None + + def test_no_credentials_logs_warning(self, caplog): + with caplog.at_level( + logging.WARNING, + logger="sap_cloud_sdk.agent_memory.thread_registry._factory", + ): + create_thread_registry() + assert "InMemoryThreadRegistry" in caplog.text + + def test_falls_back_to_in_memory_on_no_binding(self): + with patch(_NO_CREDENTIALS, return_value=None): + result = create_thread_registry() + assert isinstance(result, InMemoryThreadRegistry) + + def test_raises_config_error_for_malformed_binding(self): + with patch(_NO_CREDENTIALS, side_effect=AgentMemoryConfigError("malformed")): + with pytest.raises(AgentMemoryConfigError, match="malformed"): + create_thread_registry() + + +class TestCreateThreadRegistryServicePath: + """create_thread_registry() with credentials returns SAPAgentMemoryThreadRegistry.""" + + def _with_config(self, fn, config=_VALID_CONFIG): + with patch(_NO_CREDENTIALS, return_value=config): + return fn() + + def test_returns_sap_registry_when_credentials_present(self): + result = self._with_config(create_thread_registry) + assert isinstance(result, SAPAgentMemoryThreadRegistry) + + def test_idle_ttl_forwarded_as_default_on_registry(self): + result = self._with_config(lambda: create_thread_registry(ttl_seconds=7200)) + assert result._default_ttl == 7200 + + def test_returns_sap_registry_for_ias_config(self): + ias_config = AgentMemoryConfig(base_url="https://agent-memory.example.com") + result = self._with_config(create_thread_registry, config=ias_config) + assert isinstance(result, SAPAgentMemoryThreadRegistry) diff --git a/tests/agent_memory/unit/test_thread_registry_in_memory.py b/tests/agent_memory/unit/test_thread_registry_in_memory.py new file mode 100644 index 00000000..3c0e05e2 --- /dev/null +++ b/tests/agent_memory/unit/test_thread_registry_in_memory.py @@ -0,0 +1,384 @@ +"""Unit tests for InMemoryThreadRegistry.""" + +import time +from unittest.mock import patch + +import pytest + +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, + AgentMemoryThreadNotFoundError, +) +from sap_cloud_sdk.agent_memory.thread_registry import InMemoryThreadRegistry +from sap_cloud_sdk.core.telemetry import Module, Operation + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def reg(): + return InMemoryThreadRegistry() + + +# --------------------------------------------------------------------------- +# CRUD +# --------------------------------------------------------------------------- + + +class TestInMemoryThreadRegistryCRUD: + def test_create_or_register_and_get(self, reg): + e = reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert e["threadId"] == "t-1" + assert e["agentId"] == "agent-a" + assert e["invokerId"] == "user-1" + + got = reg.get_thread("t-1") + assert got["threadId"] == "t-1" + + def test_get_missing_raises(self, reg): + with pytest.raises(AgentMemoryThreadNotFoundError, match="t-missing"): + reg.get_thread("t-missing") + + def test_idempotent_create_or_register_same_owner(self, reg): + e1 = reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + e2 = reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert e1["threadId"] == e2["threadId"] + + def test_ownership_conflict(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + with pytest.raises(AgentMemoryThreadConflictError): + reg.create_or_register_thread("agent-b", "user-2", thread_id="t-1") + + def test_create_without_thread_id_assigns_uuid(self, reg): + e = reg.create_or_register_thread("agent-a", "user-1") + assert len(e["threadId"]) == 36 # UUID format + + def test_list_threads(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + reg.create_or_register_thread("agent-b", "user-2", thread_id="t-2") + page = reg.list_threads() + assert len(page["entries"]) == 2 + assert page["total"] == 2 + + def test_list_threads_filter_agent_id(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + reg.create_or_register_thread("agent-b", "user-2", thread_id="t-2") + page = reg.list_threads(agent_id="agent-a") + assert len(page["entries"]) == 1 + assert page["entries"][0]["threadId"] == "t-1" + + def test_list_threads_filter_invoker_id(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + reg.create_or_register_thread("agent-a", "user-2", thread_id="t-2") + page = reg.list_threads(invoker_id="user-2") + assert len(page["entries"]) == 1 + assert page["entries"][0]["threadId"] == "t-2" + + def test_list_threads_returns_thread_page_fields(self, reg): + for i in range(5): + reg.create_or_register_thread("agent-a", "user-1", thread_id=f"t-{i}") + page = reg.list_threads(agent_id="agent-a", limit=3, offset=1) + assert page["total"] == 5 + assert page["limit"] == 3 + assert page["offset"] == 1 + assert len(page["entries"]) == 3 + + def test_list_threads_no_limit_returns_all(self): + r = InMemoryThreadRegistry() + for i in range(5): + r.create_or_register_thread("agent-a", "user-1", thread_id=f"t-{i}") + page = r.list_threads() + assert page["limit"] == 5 + assert len(page["entries"]) == 5 + assert page["total"] == 5 + + @pytest.mark.parametrize("bad_limit", [-1, -100, False, True, 1.5]) + def test_list_threads_invalid_limit_raises(self, bad_limit): + r = InMemoryThreadRegistry() + with pytest.raises(ValueError, match="limit"): + r.list_threads(limit=bad_limit) + + @pytest.mark.parametrize("bad_offset", [-1, -100, False, True, 2.5]) + def test_list_threads_invalid_offset_raises(self, bad_offset): + r = InMemoryThreadRegistry() + with pytest.raises(ValueError, match="offset"): + r.list_threads(offset=bad_offset) + + def test_list_threads_limit_zero_returns_empty(self): + r = InMemoryThreadRegistry() + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + page = r.list_threads(limit=0) + assert page["entries"] == [] + assert page["limit"] == 0 + + def test_update_thread(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + updated = reg.update_thread("t-1", description="new desc", metadata={"k": "v"}) + assert updated["description"] == "new desc" + assert updated["metadata"] == {"k": "v"} + + def test_update_missing_raises(self, reg): + with pytest.raises(AgentMemoryThreadNotFoundError): + reg.update_thread("t-missing", description="x") + + def test_delete_thread(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + reg.delete_thread("t-1") + with pytest.raises(AgentMemoryThreadNotFoundError): + reg.get_thread("t-1") + + def test_delete_idempotent(self, reg): + reg.delete_thread("t-nonexistent") # must not raise + + +# --------------------------------------------------------------------------- +# Sliding idle TTL +# --------------------------------------------------------------------------- + + +class TestInMemoryThreadRegistrySlidingTTL: + def test_no_ttl_thread_never_expires(self): + r = InMemoryThreadRegistry() + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert r.get_thread("t-1")["threadId"] == "t-1" + + def test_thread_expires_after_idle_ttl(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=10) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 11) + with pytest.raises(AgentMemoryThreadNotFoundError): + r.get_thread("t-1") + + def test_list_omits_expired(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=10) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + r.create_or_register_thread("agent-a", "user-1", thread_id="t-2") + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 11) + page = r.list_threads() + assert len(page["entries"]) == 1 + assert page["entries"][0]["threadId"] == "t-2" + + def test_update_slides_window(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=10) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 9) + r.update_thread("t-1", description="touched") + assert r.get_thread("t-1")["threadId"] == "t-1" + + def test_none_ttl_never_expires(self): + r = InMemoryThreadRegistry(default_ttl_seconds=None) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert r.get_thread("t-1")["threadId"] == "t-1" + + +# --------------------------------------------------------------------------- +# ThreadExpired on re-creation +# --------------------------------------------------------------------------- + + +class TestInMemoryThreadRegistryExpired: + def test_create_or_register_expired_raises_thread_expired(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=10) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 11) + with pytest.raises(AgentMemoryThreadExpiredError, match="t-1"): + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + + def test_expired_check_takes_priority_over_ownership(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=10) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 11) + with pytest.raises(AgentMemoryThreadExpiredError): + r.create_or_register_thread("agent-b", "user-2", thread_id="t-1") + + +# --------------------------------------------------------------------------- +# Per-thread TTL override +# --------------------------------------------------------------------------- + + +class TestInMemoryThreadRegistryPerThreadTTL: + def test_create_stores_per_thread_ttl(self): + r = InMemoryThreadRegistry() + r.create_or_register_thread( + "agent-a", "user-1", thread_id="t-1", ttl_seconds=300 + ) + assert r._store["t-1"].thread_ttl == 300 + + def test_per_thread_ttl_overrides_registry_default(self, monkeypatch): + r = InMemoryThreadRegistry(default_ttl_seconds=5) + r.create_or_register_thread( + "agent-a", "user-1", thread_id="t-1", ttl_seconds=30 + ) + entry = r._store["t-1"] + monkeypatch.setattr(entry, "last_active", time.monotonic() - 6) + assert r.get_thread("t-1")["threadId"] == "t-1" + + def test_to_entry_includes_thread_ttl(self): + r = InMemoryThreadRegistry() + e = r.create_or_register_thread( + "agent-a", "user-1", thread_id="t-1", ttl_seconds=120 + ) + assert e["threadTtl"] == 120 + + def test_to_entry_thread_ttl_none_when_not_set(self): + r = InMemoryThreadRegistry() + e = r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert e["threadTtl"] is None + + def test_update_thread_sets_per_thread_ttl(self): + r = InMemoryThreadRegistry() + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + e = r.update_thread("t-1", ttl_seconds=600) + assert e["threadTtl"] == 600 + assert r._store["t-1"].thread_ttl == 600 + + def test_update_omits_ttl_leaves_per_thread_ttl_unchanged(self): + r = InMemoryThreadRegistry() + r.create_or_register_thread( + "agent-a", "user-1", thread_id="t-1", ttl_seconds=300 + ) + e = r.update_thread("t-1", description="x") + assert e["threadTtl"] == 300 + assert r._store["t-1"].thread_ttl == 300 + + # ── expiresAt field ──────────────────────────────────────────────────────── + + def test_expires_at_none_when_no_ttl(self): + r = InMemoryThreadRegistry() + e = r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert e["expiresAt"] is None + + def test_expires_at_set_when_per_thread_ttl(self): + r = InMemoryThreadRegistry() + e = r.create_or_register_thread( + "agent-a", "user-1", thread_id="t-1", ttl_seconds=300 + ) + assert e["expiresAt"] is not None + + def test_expires_at_set_when_registry_default_ttl(self): + r = InMemoryThreadRegistry(default_ttl_seconds=600) + e = r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + assert e["expiresAt"] is not None + + def test_expires_at_slides_on_touch(self, monkeypatch): + from datetime import datetime, timedelta, timezone + + r = InMemoryThreadRegistry(default_ttl_seconds=3600) + r.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + entry = r._store["t-1"] + # Wind wall clock back so the update will produce a strictly later expiresAt + past_wall = datetime.now(timezone.utc) - timedelta(seconds=60) + monkeypatch.setattr(entry, "last_active_wall", past_wall) + before_touch = entry.to_entry(r._default_ttl)["expiresAt"] + r.update_thread("t-1", description="touched") + after_touch = r.get_thread("t-1")["expiresAt"] + assert before_touch is not None + assert after_touch is not None + assert after_touch > before_touch + + +class TestInMemoryThreadRegistryTTLValidation: + """TTL inputs that are invalid for the service must also be rejected in-memory.""" + + @pytest.mark.parametrize("bad", [-1, 0, -100, False, True, 1.5]) + def test_init_rejects_invalid_default_ttl(self, bad): + with pytest.raises(ValueError, match="ttl_seconds"): + InMemoryThreadRegistry(default_ttl_seconds=bad) + + @pytest.mark.parametrize("bad", [-1, 0, False, True, 2.0]) + def test_create_rejects_invalid_per_call_ttl(self, bad): + reg = InMemoryThreadRegistry() + with pytest.raises(ValueError, match="ttl_seconds"): + reg.create_or_register_thread("agent", "user", ttl_seconds=bad) + + @pytest.mark.parametrize("bad", [-1, 0, False, True, 3.5]) + def test_update_rejects_invalid_per_call_ttl(self, bad): + reg = InMemoryThreadRegistry() + reg.create_or_register_thread("agent", "user", thread_id="t-1") + with pytest.raises(ValueError, match="ttl_seconds"): + reg.update_thread("t-1", ttl_seconds=bad) + + def test_valid_positive_integer_accepted(self): + reg = InMemoryThreadRegistry(default_ttl_seconds=3600) + entry = reg.create_or_register_thread("agent", "user", ttl_seconds=1800) + assert entry["threadTtl"] == 1800 + + def test_none_is_accepted(self): + reg = InMemoryThreadRegistry(default_ttl_seconds=None) + entry = reg.create_or_register_thread("agent", "user", ttl_seconds=None) + assert entry["expiresAt"] is None + + +# --------------------------------------------------------------------------- +# Telemetry metrics +# --------------------------------------------------------------------------- + +_REQUEST_METRIC = "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" +_ERROR_METRIC = "sap_cloud_sdk.core.telemetry.metrics_decorator.record_error_metric" + + +class TestInMemoryTelemetry: + """The in-memory registry emits the same metrics as the service-backed one, + so operation counts are consistent regardless of backend.""" + + def test_create_or_register_thread_records_request_metric(self, reg): + with patch(_REQUEST_METRIC) as metric: + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, + None, + Operation.AGENT_MEMORY_CREATE_OR_REGISTER_THREAD, + False, + ) + + def test_get_thread_records_request_metric(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + with patch(_REQUEST_METRIC) as metric: + reg.get_thread("t-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_GET_THREAD, False + ) + + def test_list_threads_records_request_metric(self, reg): + with patch(_REQUEST_METRIC) as metric: + reg.list_threads() + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_LIST_THREADS, False + ) + + def test_update_thread_records_single_request_metric(self, reg): + """update_thread uses the private _get_live helper, not the decorated + get_thread, so exactly one metric fires.""" + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + with patch(_REQUEST_METRIC) as metric: + reg.update_thread("t-1", description="new") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_UPDATE_THREAD, False + ) + + def test_delete_thread_records_request_metric(self, reg): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + with patch(_REQUEST_METRIC) as metric: + reg.delete_thread("t-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_DELETE_THREAD, False + ) + + def test_get_missing_thread_records_error_metric(self, reg): + with patch(_REQUEST_METRIC) as req, patch(_ERROR_METRIC) as err: + with pytest.raises(AgentMemoryThreadNotFoundError): + reg.get_thread("missing") + err.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_GET_THREAD, False + ) + req.assert_not_called() diff --git a/tests/agent_memory/unit/test_thread_registry_service.py b/tests/agent_memory/unit/test_thread_registry_service.py new file mode 100644 index 00000000..afaf62fc --- /dev/null +++ b/tests/agent_memory/unit/test_thread_registry_service.py @@ -0,0 +1,794 @@ +"""Unit tests for the service-backed thread registry.""" + +from unittest.mock import MagicMock, patch + +import pytest + +from sap_cloud_sdk.agent_memory.config import AgentMemoryConfig +from sap_cloud_sdk.core.protocol.http import HttpClient, HttpMethod +from sap_cloud_sdk.core.runtime_context._context import sdk_context +from sap_cloud_sdk.core.runtime_context import RuntimeContext +from sap_cloud_sdk.core.runtime_context.providers._ias import IAS_TOKEN +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryHttpError, + AgentMemoryNotFoundError, + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, + AgentMemoryThreadNotFoundError, + AgentMemoryValidationError, +) +from sap_cloud_sdk.agent_memory.thread_registry import SAPAgentMemoryThreadRegistry +from sap_cloud_sdk.agent_memory.thread_registry._service import ( + _build_list_path, + _escape_odata_string, + _is_expired_response, + _odata_entity_path, +) +from sap_cloud_sdk.core.telemetry import Module, Operation + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_BASE = "https://agent-memory.example.com" + + +def _make_registry(base_url=_BASE, default_ttl=None): + reg = SAPAgentMemoryThreadRegistry.__new__(SAPAgentMemoryThreadRegistry) + reg._http = MagicMock(spec=HttpClient) + reg._request = MagicMock() + reg._base_url = base_url.rstrip("/") + reg._default_ttl = default_ttl + return reg + + +_ENTRY = { + "threadId": "t-1", + "agentId": "agent-a", + "invokerId": "user-1", + "description": None, + "createdAt": "2024-01-01T00:00:00Z", + "updatedAt": "2024-01-01T00:00:00Z", + "expiresAt": None, + "threadTtl": None, + "metadata": None, +} + + +# --------------------------------------------------------------------------- +# OData URL helpers +# --------------------------------------------------------------------------- + + +class TestODataHelpers: + def test_escape_odata_string_no_quotes(self): + assert _escape_odata_string("hello") == "hello" + + def test_escape_odata_string_single_quotes_doubled(self): + assert _escape_odata_string("it's") == "it''s" + + def test_escape_odata_string_multiple_quotes(self): + assert _escape_odata_string("a'b'c") == "a''b''c" + + def test_odata_entity_path_basic(self): + path = _odata_entity_path("t-1") + assert path == "/v1/agent/threads(threadId='t-1')" + + def test_odata_entity_path_escapes_single_quote(self): + path = _odata_entity_path("t'1") + # Single quotes are doubled (OData) then percent-encoded (%27%27). + assert "%27%27" in path + + def test_build_list_path_includes_orderby(self): + path = _build_list_path(None, None, None, 0) + assert "$orderby=" in path + assert "$count=true" in path + + def test_build_list_path_adds_top(self): + path = _build_list_path(None, None, 10, 0) + assert "$top=10" in path + + def test_build_list_path_adds_skip(self): + path = _build_list_path(None, None, None, 5) + assert "$skip=5" in path + + def test_build_list_path_omits_skip_when_zero(self): + path = _build_list_path(None, None, None, 0) + assert "$skip" not in path + + def test_build_list_path_filter_agent_id(self): + path = _build_list_path("my-agent", None, None, 0) + assert "agentId" in path + assert "my-agent" in path + + def test_build_list_path_filter_invoker_id(self): + path = _build_list_path(None, "user-1", None, 0) + assert "invokerId" in path + assert "user-1" in path + + def test_build_list_path_no_dollar_sign_encoding(self): + path = _build_list_path(None, None, None, 0) + assert "%24" not in path # $ must not be percent-encoded + + +# --------------------------------------------------------------------------- +# _is_expired_response (issue 3: structured 409 detection) +# --------------------------------------------------------------------------- + + +class TestIsExpiredResponse: + def _exc(self, msg: str = "", response_text: str = "") -> AgentMemoryHttpError: + return AgentMemoryHttpError(msg, status_code=409, response_text=response_text) + + def test_plain_text_expired_in_response_body(self): + assert ( + _is_expired_response(self._exc(response_text="Thread has expired")) is True + ) + + def test_plain_text_conflict_returns_false(self): + assert ( + _is_expired_response(self._exc(response_text="Thread already owned")) + is False + ) + + def test_expired_in_exception_message_not_body(self): + assert _is_expired_response(self._exc(msg="expired thread")) is True + + def test_json_error_code_expired(self): + import json + + body = json.dumps({"error": {"code": "THREAD_EXPIRED", "message": "gone"}}) + assert _is_expired_response(self._exc(response_text=body)) is True + + def test_json_error_code_conflict_returns_false(self): + import json + + body = json.dumps( + {"error": {"code": "OWNERSHIP_CONFLICT", "message": "conflict"}} + ) + assert _is_expired_response(self._exc(response_text=body)) is False + + def test_json_top_level_errorCode_expired(self): + import json + + body = json.dumps({"errorCode": "expired", "message": "gone"}) + assert _is_expired_response(self._exc(response_text=body)) is True + + def test_json_code_takes_priority_over_body_text(self): + import json + + # Structured code says CONFLICT even though the message text says "expired" — + # the code field must win. + body = json.dumps({"error": {"code": "CONFLICT"}, "message": "thread expired"}) + assert _is_expired_response(self._exc(response_text=body)) is False + + def test_malformed_json_falls_back_to_string_match(self): + assert _is_expired_response(self._exc(response_text='{"bad json')) is False + + def test_empty_response_text_and_non_expired_message(self): + assert ( + _is_expired_response(self._exc(msg="some conflict", response_text="")) + is False + ) + + +# --------------------------------------------------------------------------- +# Transport lifecycle +# --------------------------------------------------------------------------- + + +class TestTransportLifecycle: + def test_close_delegates_to_transport(self): + reg = _make_registry() + reg.close() + reg._http.close.assert_called_once() + + def test_context_manager_returns_self(self): + reg = _make_registry() + with reg as r: + assert r is reg + + def test_context_manager_closes_on_exit(self): + reg = _make_registry() + with reg: + pass + reg._http.close.assert_called_once() + + def test_context_manager_closes_even_on_exception(self): + reg = _make_registry() + with pytest.raises(RuntimeError): + with reg: + raise RuntimeError("boom") + reg._http.close.assert_called_once() + + +# --------------------------------------------------------------------------- +# create_or_register_thread +# --------------------------------------------------------------------------- + + +class TestCreateOrRegisterThread: + def test_server_assigned_id_posts_body(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + + reg.create_or_register_thread("agent-a", "user-1") + + reg._request.assert_called_once() + call_args = reg._request.call_args + assert call_args[0][0] == HttpMethod.POST + assert call_args[0][1] == "/v1/agent/threads" + body = call_args[1]["json"] + assert body["agentId"] == "agent-a" + assert body["invokerId"] == "user-1" + assert "threadId" not in body + + def test_returns_parsed_entry(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + e = reg.create_or_register_thread("agent-a", "user-1") + assert e["threadId"] == "t-1" + assert e["agentId"] == "agent-a" + + def test_with_explicit_thread_id_included_in_body(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + body = reg._request.call_args[1]["json"] + assert body["threadId"] == "t-1" + + def test_description_included_when_provided(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", description="my desc") + body = reg._request.call_args[1]["json"] + assert body["description"] == "my desc" + + def test_metadata_included_when_provided(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", metadata={"x": 1}) + body = reg._request.call_args[1]["json"] + assert body["metadata"] == {"x": 1} + + def test_idle_ttl_sent_as_threadTtl(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", ttl_seconds=3600) + body = reg._request.call_args[1]["json"] + assert body["threadTtl"] == 3600 + + def test_default_ttl_used_when_no_per_call_ttl(self): + reg = _make_registry(default_ttl=1800) + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1") + body = reg._request.call_args[1]["json"] + assert body["threadTtl"] == 1800 + + def test_per_call_ttl_overrides_default(self): + reg = _make_registry(default_ttl=1800) + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", ttl_seconds=300) + body = reg._request.call_args[1]["json"] + assert body["threadTtl"] == 300 + + def test_tenant_id_forwarded(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1", tenant_id="acme") + assert reg._request.call_args[1]["tenant_subdomain"] == "acme" + + def test_no_tenant_id_passes_none(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.create_or_register_thread("agent-a", "user-1") + assert reg._request.call_args[1]["tenant_subdomain"] is None + + def test_409_without_expired_raises_conflict_when_thread_id_given(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryHttpError( + "conflict", status_code=409, response_text="Thread already owned" + ) + with pytest.raises(AgentMemoryThreadConflictError): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + + def test_409_with_expired_raises_thread_expired(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryHttpError( + "expired", status_code=409, response_text="Thread has expired" + ) + with pytest.raises(AgentMemoryThreadExpiredError): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + + def test_409_expired_detected_from_json_error_code(self): + import json + + reg = _make_registry() + reg._request.side_effect = AgentMemoryHttpError( + "conflict", + status_code=409, + response_text=json.dumps( + {"error": {"code": "THREAD_EXPIRED", "message": "gone"}} + ), + ) + with pytest.raises(AgentMemoryThreadExpiredError): + reg.create_or_register_thread("agent-a", "user-1", thread_id="t-1") + + def test_409_without_thread_id_re_raised_as_http_error(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryHttpError( + "conflict", status_code=409, response_text="conflict" + ) + with pytest.raises(AgentMemoryHttpError): + reg.create_or_register_thread("agent-a", "user-1") + + def test_invalid_ttl_raises_value_error(self): + reg = _make_registry() + with pytest.raises(ValueError, match="ttl_seconds"): + reg.create_or_register_thread("agent-a", "user-1", ttl_seconds=-1) + + def test_boolean_ttl_raises_value_error(self): + reg = _make_registry() + with pytest.raises(ValueError, match="ttl_seconds"): + reg.create_or_register_thread("agent-a", "user-1", ttl_seconds=True) + + +# --------------------------------------------------------------------------- +# get_thread +# --------------------------------------------------------------------------- + + +class TestGetThread: + def test_uses_odata_entity_path(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.get_thread("t-1") + call_args = reg._request.call_args + assert call_args[0][0] == HttpMethod.GET + path = call_args[0][1] + assert "t-1" in path + assert path.startswith("/v1/agent/threads(") + + def test_returns_parsed_entry(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + e = reg.get_thread("t-1") + assert e["threadId"] == "t-1" + + def test_not_found_raises_thread_not_found(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryNotFoundError("not found") + with pytest.raises(AgentMemoryThreadNotFoundError, match="t-1"): + reg.get_thread("t-1") + + def test_tenant_id_forwarded(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + reg.get_thread("t-1", tenant_id="acme") + assert reg._request.call_args[1]["tenant_subdomain"] == "acme" + + +# --------------------------------------------------------------------------- +# list_threads +# --------------------------------------------------------------------------- + + +class TestListThreads: + def _page_response(self, entries, total=None, next_link=None): + body: dict = {"value": entries, "@odata.count": total or len(entries)} + if next_link: + body["@odata.nextLink"] = next_link + return body + + def test_returns_thread_page(self): + reg = _make_registry() + reg._request.return_value = self._page_response([_ENTRY.copy()]) + page = reg.list_threads() + assert len(page["entries"]) == 1 + assert page["total"] == 1 + + def test_with_limit_uses_top(self): + reg = _make_registry() + reg._request.return_value = self._page_response([_ENTRY.copy()]) + reg.list_threads(limit=5) + path = reg._request.call_args[0][1] + assert "$top=5" in path + + def test_with_offset_uses_skip(self): + reg = _make_registry() + reg._request.return_value = self._page_response([]) + reg.list_threads(limit=5, offset=10) + path = reg._request.call_args[0][1] + assert "$skip=10" in path + + def test_limit_zero_calls_transport_for_count(self): + reg = _make_registry() + reg._request.return_value = self._page_response([], total=7) + page = reg.list_threads(limit=0) + path = reg._request.call_args[0][1] + assert "$top=0" in path + assert "$count=true" in path + assert page["entries"] == [] + assert page["total"] == 7 + + def test_follows_next_link(self): + reg = _make_registry() + next_link = f"{_BASE}/v1/agent/threads?$top=1&$skip=1" + first_page = self._page_response([_ENTRY.copy()], total=2, next_link=next_link) + second_page = self._page_response([{**_ENTRY, "threadId": "t-2"}], total=2) + reg._request.side_effect = [first_page, second_page] + + page = reg.list_threads() # no limit = follow all pages + + assert len(page["entries"]) == 2 + assert reg._request.call_count == 2 + + def test_next_link_strips_base_url(self): + reg = _make_registry() + next_link = f"{_BASE}/v1/agent/threads?$skip=1" + first = self._page_response([_ENTRY.copy()], total=2, next_link=next_link) + second = self._page_response([{**_ENTRY, "threadId": "t-2"}], total=2) + reg._request.side_effect = [first, second] + reg.list_threads() + second_call_path = reg._request.call_args_list[1][0][1] + assert not second_call_path.startswith("http") + assert second_call_path.startswith("/") + + def test_filter_agent_id_in_path(self): + reg = _make_registry() + reg._request.return_value = self._page_response([]) + reg.list_threads(agent_id="my-agent") + path = reg._request.call_args[0][1] + assert "my-agent" in path + + def test_filter_invoker_id_in_path(self): + reg = _make_registry() + reg._request.return_value = self._page_response([]) + reg.list_threads(invoker_id="user-1") + path = reg._request.call_args[0][1] + assert "user-1" in path + + def test_invalid_limit_raises_value_error(self): + reg = _make_registry() + with pytest.raises(ValueError, match="limit"): + reg.list_threads(limit=-1) + + def test_boolean_limit_raises_value_error(self): + """Finding 8: limit=False must not bypass the limit==0 early-return via False==0.""" + reg = _make_registry() + with pytest.raises(ValueError, match="limit"): + reg.list_threads(limit=False) + + def test_boolean_true_limit_raises_value_error(self): + """Finding 8: limit=True (== 1 numerically) should also raise, not silently fetch 1 item.""" + reg = _make_registry() + with pytest.raises(ValueError, match="limit"): + reg.list_threads(limit=True) + + def test_invalid_offset_raises_value_error(self): + reg = _make_registry() + with pytest.raises(ValueError, match="offset"): + reg.list_threads(offset=-1) + + def test_tenant_id_forwarded(self): + reg = _make_registry() + reg._request.return_value = self._page_response([]) + reg.list_threads(tenant_id="acme") + assert reg._request.call_args[1]["tenant_subdomain"] == "acme" + + +# --------------------------------------------------------------------------- +# update_thread +# --------------------------------------------------------------------------- + + +class TestUpdateThread: + def test_patches_entity_path_then_gets(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + + reg.update_thread("t-1", description="new desc") + + assert reg._request.call_count == 2 + patch_call = reg._request.call_args_list[0] + assert patch_call[0][0] == HttpMethod.PATCH + patch_path = patch_call[0][1] + assert "t-1" in patch_path + assert patch_path.startswith("/v1/agent/threads(") + + get_call = reg._request.call_args_list[1] + assert get_call[0][0] == HttpMethod.GET + + def test_body_excludes_thread_id(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1") + body = reg._request.call_args_list[0][1]["json"] + assert "threadId" not in body + + def test_description_included_in_body(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1", description="new") + body = reg._request.call_args_list[0][1]["json"] + assert body["description"] == "new" + + def test_idle_ttl_sent_as_threadTtl(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1", ttl_seconds=600) + body = reg._request.call_args_list[0][1]["json"] + assert body["threadTtl"] == 600 + + def test_ttl_none_omits_threadTtl_leaving_it_unchanged(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1", description="x") + body = reg._request.call_args_list[0][1]["json"] + assert "threadTtl" not in body + + def test_default_ttl_not_reapplied_on_update(self): + # A description-only update must not resend the registry default TTL, + # otherwise it would silently reset a per-thread TTL override. + reg = _make_registry(default_ttl=7200) + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1", description="x") + body = reg._request.call_args_list[0][1]["json"] + assert "threadTtl" not in body + + def test_not_found_raises_thread_not_found(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryNotFoundError("not found") + with pytest.raises(AgentMemoryThreadNotFoundError, match="t-1"): + reg.update_thread("t-1", description="x") + + def test_returns_refreshed_entry(self): + reg = _make_registry() + refreshed = {**_ENTRY, "description": "updated"} + reg._request.side_effect = [{}, refreshed] + e = reg.update_thread("t-1", description="updated") + assert e["description"] == "updated" + + def test_tenant_id_forwarded_to_both_calls(self): + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + reg.update_thread("t-1", tenant_id="acme") + assert reg._request.call_args_list[0][1]["tenant_subdomain"] == "acme" + assert reg._request.call_args_list[1][1]["tenant_subdomain"] == "acme" + + def test_invalid_ttl_raises_value_error(self): + reg = _make_registry() + with pytest.raises(ValueError, match="ttl_seconds"): + reg.update_thread("t-1", ttl_seconds=0) + + +# --------------------------------------------------------------------------- +# delete_thread +# --------------------------------------------------------------------------- + + +class TestDeleteThread: + def test_calls_delete_with_entity_path(self): + reg = _make_registry() + reg._request.return_value = {} + reg.delete_thread("t-1") + call_args = reg._request.call_args + assert call_args[0][0] == HttpMethod.DELETE + path = call_args[0][1] + assert "t-1" in path + assert path.startswith("/v1/agent/threads(") + + def test_idempotent_swallows_not_found(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryNotFoundError("not found") + reg.delete_thread("t-1") # must not raise + + def test_tenant_id_forwarded(self): + reg = _make_registry() + reg._request.return_value = {} + reg.delete_thread("t-1", tenant_id="acme") + assert reg._request.call_args[1]["tenant_subdomain"] == "acme" + + def test_other_http_errors_propagate(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryHttpError("server error", status_code=500) + with pytest.raises(AgentMemoryHttpError): + reg.delete_thread("t-1") + + +# --------------------------------------------------------------------------- +# End-to-end IAS flow +# --------------------------------------------------------------------------- + + +class TestIASEndToEnd: + """Full chain: IAS ContextVar set → registry method → Bearer token on wire. + + Uses a real SAPAgentMemoryThreadRegistry (no mock _request) with a patched + requests.Session so the HTTP call is intercepted at the network boundary. + """ + + def _make_registry_with_real_transport(self): + config = AgentMemoryConfig( + base_url=_BASE + ) # IAS-mode: URL only, no OAuth2 creds + return SAPAgentMemoryThreadRegistry(config) + + def test_create_thread_sends_ias_bearer_token(self): + """create_or_register_thread() sends Authorization: Bearer .""" + mock_response = MagicMock() + mock_response.status_code = 201 + mock_response.ok = True + mock_response.content = b"{}" + mock_response.json.return_value = { + "threadId": "t-1", + "agentId": "agent-a", + "invokerId": "user-1", + } + + with sdk_context(RuntimeContext({IAS_TOKEN: "ias-dummy-token-xyz"})): + with patch( + "sap_cloud_sdk.core.protocol.http.models.requests" + ) as mock_requests: + mock_session = MagicMock() + mock_session.headers = {} + mock_requests.Session.return_value = mock_session + mock_session.request.return_value = mock_response + + reg = self._make_registry_with_real_transport() + reg.create_or_register_thread(agent_id="agent-a", invoker_id="user-1") + + assert mock_session.headers["Authorization"] == "Bearer ias-dummy-token-xyz" + + def test_get_thread_sends_ias_bearer_token(self): + """get_thread() passes the IAS token through HttpClient.""" + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.ok = True + mock_response.content = b"{}" + mock_response.json.return_value = { + "threadId": "t-1", + "agentId": "agent-a", + "invokerId": "user-1", + } + + with sdk_context(RuntimeContext({IAS_TOKEN: "ias-dummy-token-xyz"})): + with patch( + "sap_cloud_sdk.core.protocol.http.models.requests" + ) as mock_requests: + mock_session = MagicMock() + mock_session.headers = {} + mock_requests.Session.return_value = mock_session + mock_session.request.return_value = mock_response + + reg = self._make_registry_with_real_transport() + reg.get_thread("t-1") + + assert mock_session.headers["Authorization"] == "Bearer ias-dummy-token-xyz" + + def test_no_ias_token_raises_validation_error(self): + """Without the IAS ContextVar, IAS-mode calls raise before any request. + + The missing-token guard runs in the client layer, so no HTTP request is + ever dispatched when the token is absent. + + sdk_context(RuntimeContext({})) explicitly clears any ambient IAS token + that a session-scoped integration fixture may have injected, making the + test deterministic regardless of run order. + """ + with patch("sap_cloud_sdk.core.protocol.http.models.requests") as mock_requests: + mock_session = MagicMock() + mock_requests.Session.return_value = mock_session + + reg = self._make_registry_with_real_transport() + with sdk_context(RuntimeContext({})): + with pytest.raises( + AgentMemoryValidationError, match="bootstrap\\(app\\)" + ): + reg.get_thread("t-1") + + mock_session.request.assert_not_called() + + +# --------------------------------------------------------------------------- +# Telemetry metrics +# --------------------------------------------------------------------------- + +_REQUEST_METRIC = "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" +_ERROR_METRIC = "sap_cloud_sdk.core.telemetry.metrics_decorator.record_error_metric" + + +class TestTelemetry: + """Each operation records a request metric on success and an error metric on failure. + + Operations are distinguished only by the Operation enum (the + ``sap.cloud_sdk.functionality`` attribute); all share Module.AGENT_MEMORY. + ``source`` is None here because the registry never sets ``_telemetry_source``. + """ + + def test_create_or_register_thread_records_request_metric(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + with patch(_REQUEST_METRIC) as metric: + reg.create_or_register_thread("agent-a", "user-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, + None, + Operation.AGENT_MEMORY_CREATE_OR_REGISTER_THREAD, + False, + ) + + def test_get_thread_records_request_metric(self): + reg = _make_registry() + reg._request.return_value = _ENTRY.copy() + with patch(_REQUEST_METRIC) as metric: + reg.get_thread("t-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_GET_THREAD, False + ) + + def test_list_threads_records_request_metric(self): + reg = _make_registry() + reg._request.return_value = {"value": [], "@odata.count": 0} + with patch(_REQUEST_METRIC) as metric: + reg.list_threads(limit=10) + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_LIST_THREADS, False + ) + + def test_update_thread_records_single_request_metric(self): + """update_thread must emit exactly one metric — the internal refreshing + read goes through the undecorated _get_thread, so no stray get_thread.""" + reg = _make_registry() + reg._request.side_effect = [{}, _ENTRY.copy()] + with patch(_REQUEST_METRIC) as metric: + reg.update_thread("t-1", description="new") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_UPDATE_THREAD, False + ) + + def test_delete_thread_records_request_metric(self): + reg = _make_registry() + reg._request.return_value = {} + with patch(_REQUEST_METRIC) as metric: + reg.delete_thread("t-1") + metric.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_DELETE_THREAD, False + ) + + def test_delete_thread_swallowed_404_counts_as_success(self): + """An idempotent delete of a missing thread returns normally, so the + request (not error) counter fires.""" + reg = _make_registry() + reg._request.side_effect = AgentMemoryNotFoundError("not found") + with patch(_REQUEST_METRIC) as req, patch(_ERROR_METRIC) as err: + reg.delete_thread("t-1") + req.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_DELETE_THREAD, False + ) + err.assert_not_called() + + def test_get_thread_not_found_records_error_metric(self): + reg = _make_registry() + reg._request.side_effect = AgentMemoryNotFoundError("not found") + with patch(_REQUEST_METRIC) as req, patch(_ERROR_METRIC) as err: + with pytest.raises(AgentMemoryThreadNotFoundError): + reg.get_thread("t-1") + err.assert_called_once_with( + Module.AGENT_MEMORY, None, Operation.AGENT_MEMORY_GET_THREAD, False + ) + req.assert_not_called() + + def test_create_validation_error_records_error_metric(self): + reg = _make_registry() + with patch(_REQUEST_METRIC) as req, patch(_ERROR_METRIC) as err: + with pytest.raises(ValueError): + reg.create_or_register_thread("agent-a", "user-1", ttl_seconds=-1) + err.assert_called_once_with( + Module.AGENT_MEMORY, + None, + Operation.AGENT_MEMORY_CREATE_OR_REGISTER_THREAD, + False, + ) + req.assert_not_called() diff --git a/tests/agent_memory/unit/test_timed_memory.py b/tests/agent_memory/unit/test_timed_memory.py index 9aa6b2b3..6c01318b 100644 --- a/tests/agent_memory/unit/test_timed_memory.py +++ b/tests/agent_memory/unit/test_timed_memory.py @@ -1,4 +1,4 @@ -"""Unit tests for TimedInMemorySaver and create_checkpointer(ttl_seconds=...).""" +"""Unit tests for TimedInMemorySaver.""" import time from typing import Any, TypedDict @@ -7,36 +7,29 @@ from langgraph.graph import END, START, StateGraph from sap_cloud_sdk.agent_memory.factory._timed_memory import TimedInMemorySaver -from sap_cloud_sdk.agent_memory.factory.langgraph_checkpoint import create_checkpointer -class TestCreateCheckpointerWithTTL: - """Tests for create_checkpointer(ttl_seconds=...) factory.""" +class TestTimedInMemorySaverDirect: + """Tests for TimedInMemorySaver via direct instantiation.""" - def test_no_ttl_returns_in_memory_saver(self): - """Default returns plain InMemorySaver when ttl_seconds is None.""" - result = create_checkpointer() - assert isinstance(result, InMemorySaver) - assert not isinstance(result, TimedInMemorySaver) - - def test_ttl_returns_timed_in_memory_saver(self): - """ttl_seconds returns TimedInMemorySaver.""" - result = create_checkpointer(ttl_seconds=3600) - assert isinstance(result, TimedInMemorySaver) + def test_default_ttl(self): + """Default TTL is 3600 seconds.""" + saver = TimedInMemorySaver() + assert saver.ttl_seconds == 3600 - def test_ttl_value_is_set(self): - """ttl_seconds value is stored on the returned saver.""" - result = create_checkpointer(ttl_seconds=7200) - assert isinstance(result, TimedInMemorySaver) - assert result.ttl_seconds == 7200 + def test_custom_ttl_stored(self): + """ttl_seconds value is stored on the saver.""" + saver = TimedInMemorySaver(ttl_seconds=7200) + assert saver.ttl_seconds == 7200 - def test_timed_saver_is_also_in_memory_saver(self): - """TimedInMemorySaver is a valid BaseCheckpointSaver subtype.""" - result = create_checkpointer(ttl_seconds=3600) - assert isinstance(result, InMemorySaver) + def test_is_also_in_memory_saver(self): + """TimedInMemorySaver is a valid InMemorySaver subtype.""" + saver = TimedInMemorySaver(ttl_seconds=3600) + assert isinstance(saver, InMemorySaver) def test_compiles_with_langgraph_graph(self): - """Timed checkpointer compiles a StateGraph correctly.""" + """TimedInMemorySaver compiles a StateGraph correctly.""" + class State(TypedDict): value: str @@ -48,23 +41,13 @@ def noop(state: State) -> State: builder.add_edge(START, "noop") builder.add_edge("noop", END) - app: Any = builder.compile(checkpointer=create_checkpointer(ttl_seconds=3600)) + app: Any = builder.compile(checkpointer=TimedInMemorySaver(ttl_seconds=3600)) assert app is not None class TestTimedInMemorySaver: """Tests for TimedInMemorySaver behaviour.""" - def test_default_ttl(self): - """Default TTL is 3600 seconds.""" - saver = TimedInMemorySaver() - assert saver.ttl_seconds == 3600 - - def test_custom_ttl(self): - """Custom TTL is stored correctly.""" - saver = TimedInMemorySaver(ttl_seconds=1800) - assert saver.ttl_seconds == 1800 - def test_sweeper_thread_is_daemon(self): """Background sweep thread is a daemon — dies with the process.""" saver = TimedInMemorySaver() @@ -73,6 +56,7 @@ def test_sweeper_thread_is_daemon(self): def test_put_updates_last_active(self): """put() updates last-active timestamp for the thread.""" + class State(TypedDict): x: int @@ -94,6 +78,7 @@ def noop(state: State) -> State: def test_expired_thread_evicted(self): """Threads inactive beyond ttl_seconds are evicted by _evict_expired.""" + class State(TypedDict): x: int @@ -123,6 +108,7 @@ def noop(state: State) -> State: def test_active_thread_not_evicted(self): """Threads active within ttl_seconds are not evicted.""" + class State(TypedDict): x: int @@ -146,6 +132,7 @@ def noop(state: State) -> State: def test_state_preserved_across_invocations(self): """TimedInMemorySaver preserves state across invocations.""" + class State(TypedDict): values: list diff --git a/tests/core/unit/runtime_context/test_runtime_context.py b/tests/core/unit/runtime_context/test_runtime_context.py index bbb40bc5..402a026e 100644 --- a/tests/core/unit/runtime_context/test_runtime_context.py +++ b/tests/core/unit/runtime_context/test_runtime_context.py @@ -33,6 +33,7 @@ from sap_cloud_sdk.core.runtime_context.providers._ias import ( APP_TENANT_ID, GLOBAL_TENANT_ID, + IAS_TOKEN, USER_ID, ) from sap_cloud_sdk.core.runtime_context.starlette import _merge @@ -270,13 +271,6 @@ def test_extracts_global_tenant_id(self): ctx = IASContextProvider().extract(envelope) assert ctx.get(GLOBAL_TENANT_ID) == "g-1" - def test_returns_empty_context_when_no_auth_header(self): - envelope = _make_envelope({}) - ctx = IASContextProvider().extract(envelope) - assert ctx.get(APP_TENANT_ID) is None - assert ctx.get(USER_ID) is None - assert ctx.get(GLOBAL_TENANT_ID) is None - def test_returns_empty_context_on_parse_error(self): envelope = _make_envelope({"authorization": "Bearer bad"}) with patch(_PATCH_PARSE, side_effect=ValueError("bad")): @@ -295,9 +289,32 @@ def test_tenant_id_none_when_claim_absent(self): def test_satisfies_context_provider_protocol(self): assert isinstance(IASContextProvider(), ContextProvider) + def test_stores_raw_bearer_token(self): + claims = _make_claims(app_tid="t-1", user_uuid="u-1") + envelope = _make_envelope({"authorization": "Bearer my-token-123"}) + with patch(_PATCH_PARSE, return_value=claims): + ctx = IASContextProvider().extract(envelope) + assert ctx.get(IAS_TOKEN) == "my-token-123" -# --------------------------------------------------------------------------- -# SAPTriggerContextProvider + def test_stores_raw_bearer_token_on_parse_error(self): + envelope = _make_envelope({"authorization": "Bearer bad-jwt"}) + with patch(_PATCH_PARSE, side_effect=ValueError("bad")): + ctx = IASContextProvider().extract(envelope) + assert ctx.get(IAS_TOKEN) == "bad-jwt" + assert ctx.get(APP_TENANT_ID) is None + + def test_no_ias_token_when_no_auth_header(self): + envelope = _make_envelope({}) + ctx = IASContextProvider().extract(envelope) + assert ctx.get(IAS_TOKEN) is None + + def test_returns_empty_context_when_no_auth_header(self): + envelope = _make_envelope({}) + ctx = IASContextProvider().extract(envelope) + assert ctx.get(APP_TENANT_ID) is None + assert ctx.get(USER_ID) is None + assert ctx.get(GLOBAL_TENANT_ID) is None + assert ctx.get(IAS_TOKEN) is None # --------------------------------------------------------------------------- diff --git a/tests/core/unit/secret_resolver/unit/_future_annotations_fixture.py b/tests/core/unit/secret_resolver/unit/_future_annotations_fixture.py new file mode 100644 index 00000000..a3ee249d --- /dev/null +++ b/tests/core/unit/secret_resolver/unit/_future_annotations_fixture.py @@ -0,0 +1,28 @@ +"""Fixture module with ``from __future__ import annotations`` active. + +``from __future__ import annotations`` turns all annotations in THIS module into +strings at runtime. Importing the dataclass defined here lets tests verify that +``_get_field_map()`` uses ``typing.get_type_hints()`` (which resolves string +annotations) rather than ``f.type`` directly (which would return a string and +break Optional[str] detection). +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Optional + + +@dataclass +class PEP563Config: + """Dataclass whose annotations are stored as strings due to PEP 563. + + At runtime (without get_type_hints): + PEP563Config.__annotations__ == {"required": "str", "optional": "Optional[str]"} + + After get_type_hints(): + {"required": str, "optional": typing.Optional[str]} + """ + + required: str = "" + optional: Optional[str] = None diff --git a/tests/core/unit/secret_resolver/unit/test_secret_resolver.py b/tests/core/unit/secret_resolver/unit/test_secret_resolver.py index f02445ff..ce564a4b 100644 --- a/tests/core/unit/secret_resolver/unit/test_secret_resolver.py +++ b/tests/core/unit/secret_resolver/unit/test_secret_resolver.py @@ -2,10 +2,12 @@ import os from dataclasses import dataclass, field +from typing import Optional from unittest.mock import patch, mock_open import pytest from sap_cloud_sdk.core.secret_resolver import read_from_mount_and_fallback_to_env_var +from sap_cloud_sdk.core.secret_resolver.resolver import _get_field_map @dataclass @@ -15,6 +17,12 @@ class SampleConfig: endpoint: str = "default" +@dataclass +class OptionalFieldConfig: + required_field: str = "" + optional_field: Optional[str] = None + + @dataclass class NonStringConfig: count: int = 0 @@ -255,3 +263,187 @@ def test_service_binding_root_flat_fails_falls_back_to_module_instance(self, moc assert config.username == "legacy_user" assert config.password == "legacy_pass" assert config.endpoint == "legacy_ep" + + +class TestOptionalFields: + """Tests for Optional[str] field support — optionality declared via type annotation.""" + + @patch.dict(os.environ, {"VAR_MODULE_INSTANCE_REQUIRED_FIELD": "req_value"}) + def test_optional_env_var_absent_keeps_default(self): + """Optional env var missing is silently skipped; field keeps its default (None).""" + config = OptionalFieldConfig() + with ( + patch("os.path.isdir", return_value=False), + patch("os.stat", side_effect=FileNotFoundError()), + ): + read_from_mount_and_fallback_to_env_var( + "/nonexistent", "VAR", "module", "instance", config + ) + + assert config.required_field == "req_value" + assert config.optional_field is None # default preserved + + @patch.dict(os.environ, { + "VAR_MODULE_INSTANCE_REQUIRED_FIELD": "req_value", + "VAR_MODULE_INSTANCE_OPTIONAL_FIELD": "opt_value", + }) + def test_optional_env_var_present_is_loaded(self): + """Optional env var that IS present is loaded normally.""" + config = OptionalFieldConfig() + with ( + patch("os.path.isdir", return_value=False), + patch("os.stat", side_effect=FileNotFoundError()), + ): + read_from_mount_and_fallback_to_env_var( + "/nonexistent", "VAR", "module", "instance", config + ) + + assert config.required_field == "req_value" + assert config.optional_field == "opt_value" + + @patch.dict(os.environ, {}, clear=True) + def test_required_env_var_absent_still_raises(self): + """Missing required field still raises RuntimeError even when optional field is absent.""" + config = OptionalFieldConfig() + with ( + patch("os.path.isdir", return_value=False), + patch("os.stat", side_effect=FileNotFoundError()), + ): + with pytest.raises(RuntimeError, match="env var failed"): + read_from_mount_and_fallback_to_env_var( + "/nonexistent", "VAR", "module", "instance", config + ) + + @patch("os.path.isdir", return_value=True) + @patch("os.stat") + def test_optional_mount_file_absent_keeps_default(self, mock_stat, mock_isdir): + """Optional secret file missing from mount is silently skipped.""" + file_side_effects = [ + mock_open(read_data="req_value").return_value, # required_field file found + FileNotFoundError("optional file not found"), # optional_field file missing + ] + config = OptionalFieldConfig() + with patch("builtins.open", side_effect=file_side_effects): + read_from_mount_and_fallback_to_env_var( + "/secrets", "VAR", "module", "instance", config + ) + + assert config.required_field == "req_value" + assert config.optional_field is None # default preserved + + @patch("os.path.isdir", return_value=True) + @patch("os.stat") + def test_required_mount_file_absent_raises(self, mock_stat, mock_isdir): + """Missing required secret file still raises FileNotFoundError.""" + config = OptionalFieldConfig() + with patch("builtins.open", side_effect=FileNotFoundError("file not found")): + with pytest.raises( + RuntimeError, + match="failed to read secrets.*failed to read secret file", + ): + read_from_mount_and_fallback_to_env_var( + "/secrets", "VAR", "module", "instance", config + ) + + +# ── TestGetFieldMap ─────────────────────────────────────────────────────────── + + +class TestGetFieldMap: + """Direct unit tests for _get_field_map() — the internal that drives optionality detection. + + These pin down the exact contract so a refactor can't silently change behaviour: + - ``str`` fields are required (is_optional=False) + - ``Optional[str]`` fields are optional (is_optional=True) + - Non-string fields raise TypeError + - Optionality comes from the type annotation, NOT from field.metadata + """ + + def test_str_field_is_required(self): + @dataclass + class Config: + host: str = "" + + result = _get_field_map(Config()) + assert result["host"][2] is False # (name, type, is_optional) + + def test_optional_str_field_is_optional(self): + @dataclass + class Config: + token: Optional[str] = None + + result = _get_field_map(Config()) + assert result["token"][2] is True + + def test_mixed_fields_correct_flags(self): + @dataclass + class Config: + required: str = "" + optional: Optional[str] = None + + result = _get_field_map(Config()) + assert result["required"][2] is False + assert result["optional"][2] is True + + def test_non_string_field_raises_type_error(self): + @dataclass + class Config: + count: int = 0 + + with pytest.raises(TypeError, match="is not a string"): + _get_field_map(Config()) + + def test_metadata_secret_key_still_respected(self): + """field(metadata={"secret": "..."}) renames the lookup key; Optional still works.""" + @dataclass + class Config: + api_token: Optional[str] = field(default=None, metadata={"secret": "token"}) + + result = _get_field_map(Config()) + # Key is the metadata "secret" value, not the field name + assert "token" in result + assert result["token"][0] == "api_token" # attribute name + assert result["token"][2] is True # is_optional + + def test_non_dataclass_raises_type_error(self): + with pytest.raises(TypeError, match="target must be a dataclass instance"): + _get_field_map("not_a_dataclass") + + def test_dataclass_class_not_instance_raises(self): + with pytest.raises(TypeError, match="target must be a dataclass instance"): + _get_field_map(OptionalFieldConfig) # class, not instance + + def test_metadata_optional_true_without_optional_type_is_required(self): + """Confirms the old metadata={"optional": True} pattern is no longer honoured. + + After the migration to Optional[str], a plain ``str`` field annotated with + ``field(metadata={"optional": True})`` is treated as REQUIRED — the type + annotation is the source of truth, not the metadata. Any pre-migration + usage of the metadata pattern must be updated to ``Optional[str]``. + """ + @dataclass + class OldStyleConfig: + legacy: str = field(default="", metadata={"optional": True}) + + result = _get_field_map(OldStyleConfig()) + # is_optional is False because the annotation is str, not Optional[str] + assert result["legacy"][2] is False + + def test_pep563_annotations_resolved_correctly(self): + """_get_field_map() works when the dataclass module uses PEP 563 (from __future__ import annotations). + + Under PEP 563, class __annotations__ stores raw strings like "Optional[str]" + instead of type objects. f.type would return a string, making ``f.type is str`` + and ``get_origin(f.type)`` both return wrong results. Using get_type_hints() + resolves the strings to real objects so Optional[str] detection stays correct. + """ + from tests.core.unit.secret_resolver.unit._future_annotations_fixture import PEP563Config + + # Confirm PEP 563 effect: annotations are strings in the class dict. + raw = PEP563Config.__annotations__ + assert isinstance(raw["required"], str), "PEP 563 not active — test invalid" + assert isinstance(raw["optional"], str), "PEP 563 not active — test invalid" + + result = _get_field_map(PEP563Config()) + assert result["required"][2] is False, "required str field should be required" + assert result["optional"][2] is True, "Optional[str] field should be optional" diff --git a/tests/core/unit/telemetry/test_operation.py b/tests/core/unit/telemetry/test_operation.py index cc42e94f..fe9e6cbb 100644 --- a/tests/core/unit/telemetry/test_operation.py +++ b/tests/core/unit/telemetry/test_operation.py @@ -316,7 +316,7 @@ def test_operation_count(self): """Test that we have the expected number of operations.""" all_operations = list(Operation) # 3 auditlog + 12 destination + 10 certificate + 10 fragment + 8 objectstore - # + 2 extensibility + 7 aicore + 23 dms + 7 agentgateway + 13 agent_memory + # + 2 extensibility + 7 aicore + 23 dms + 7 agentgateway + 19 agent_memory # + 5 data_anonymization + 52 adms + 6 print + 94 dpi_ng + 1 bootstrap + 3 output_management - # + 3 cbc = 259 - assert len(all_operations) == 259 + # + 3 cbc = 265 + assert len(all_operations) == 265 diff --git a/uv.lock b/uv.lock index 74fc68ad..496f80db 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 5 +revision = 3 requires-python = ">=3.11" resolution-markers = [ "python_full_version >= '3.15' and sys_platform == 'win32'", From 8f5fc883ec1bb3ac0c6c858065f0d74cc5a89e07 Mon Sep 17 00:00:00 2001 From: Paras Garg Date: Wed, 21 Oct 2026 10:39:20 +0530 Subject: [PATCH 2/2] refactor(agent-memory): abstract thread-registry rules, finish tenant_subdomain rename, add IAS no-token guard --- .../agent_memory/thread_registry/__init__.py | 2 +- .../agent_memory/thread_registry/_base.py | 40 +++- .../thread_registry/_in_memory.py | 27 +-- .../agent_memory/thread_registry/_service.py | 68 +++--- src/sap_cloud_sdk/agent_memory/user-guide.md | 209 +++++------------- .../core/protocol/http/models.py | 5 + .../unit/test_thread_registry_service.py | 17 +- 7 files changed, 159 insertions(+), 209 deletions(-) diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py b/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py index b500958d..ce2c1cf0 100644 --- a/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/__init__.py @@ -1,6 +1,6 @@ """Thread registry package for SAP Agent Memory. -Public API:: +Public API: from sap_cloud_sdk.agent_memory.thread_registry import ( BaseThreadRegistry, diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py index 40b2623a..0c67f154 100644 --- a/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_base.py @@ -3,10 +3,15 @@ from __future__ import annotations from abc import ABC, abstractmethod -from typing import Any, Optional +from typing import Any, NoReturn, Optional from typing_extensions import TypedDict +from sap_cloud_sdk.agent_memory.exceptions import ( + AgentMemoryThreadConflictError, + AgentMemoryThreadExpiredError, +) + def _validate_ttl(ttl_seconds: Optional[int]) -> None: """Raise ValueError for any TTL that cannot represent a positive duration.""" @@ -18,6 +23,29 @@ def _validate_ttl(ttl_seconds: Optional[int]) -> None: raise ValueError(f"ttl_seconds must be a positive integer, got {ttl_seconds!r}") +def _raise_thread_expired( + thread_id: str, *, from_exc: Optional[BaseException] = None +) -> NoReturn: + """Raise the canonical "re-create an expired thread" error. + + Both registries reject re-creating a thread whose ID has expired; this + keeps the message identical whether the check is local (in-memory) or + derived from a service 409 response. + """ + raise AgentMemoryThreadExpiredError( + f"Thread '{thread_id}' has expired and cannot be re-created with the same ID" + ) from from_exc + + +def _raise_thread_conflict( + thread_id: str, *, from_exc: Optional[BaseException] = None +) -> NoReturn: + """Raise the canonical "thread owned by someone else" error.""" + raise AgentMemoryThreadConflictError( + f"Thread '{thread_id}' is already registered by a different owner" + ) from from_exc + + class ThreadRegistryEntry(TypedDict): """A single thread registry record returned by all registry operations.""" @@ -82,7 +110,7 @@ def create_or_register_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: """Create a thread (server-assigned ID) or register an existing one. @@ -103,7 +131,7 @@ def get_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: """Fetch a single thread by ID. @@ -120,7 +148,7 @@ def list_threads( invoker_id: Optional[str] = None, limit: Optional[int] = None, offset: Optional[int] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadPage: """List threads with optional filters and pagination. @@ -136,7 +164,7 @@ def update_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: """Update mutable fields of a thread. @@ -156,7 +184,7 @@ def delete_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> None: """Delete a thread. Idempotent — a missing thread is a no-op.""" ... diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py index dfde4dd7..a918df8f 100644 --- a/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py @@ -9,14 +9,14 @@ from typing import Any, Optional from sap_cloud_sdk.agent_memory.exceptions import ( - AgentMemoryThreadConflictError, - AgentMemoryThreadExpiredError, AgentMemoryThreadNotFoundError, ) from sap_cloud_sdk.agent_memory.thread_registry._base import ( BaseThreadRegistry, ThreadPage, ThreadRegistryEntry, + _raise_thread_conflict, + _raise_thread_expired, _validate_ttl, ) from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics @@ -37,6 +37,8 @@ class _StoredEntry: ) def is_expired(self, registry_ttl: Optional[int]) -> bool: + # Per-thread thread_ttl wins; registry_ttl is only the fallback for + # entries that never set one. None at both levels means "never expires". effective = self.thread_ttl if self.thread_ttl is not None else registry_ttl if effective is None: return False @@ -91,6 +93,9 @@ class InMemoryThreadRegistry(BaseThreadRegistry): - **Lazy eviction**: threads are evicted at the next read, not on a sweep schedule. A thread may appear live briefly after its true TTL. - **No restart survival**: all state is lost on process exit. + - **No tenant isolation**: ``tenant_subdomain`` is accepted for signature + parity with the service-backed registry but ignored — all threads share a + single store regardless of tenant. Args: default_ttl_seconds: Registry-level default inactivity threshold. @@ -128,7 +133,7 @@ def create_or_register_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: _validate_ttl(ttl_seconds) ttl = self._effective_ttl(ttl_seconds) @@ -150,13 +155,9 @@ def create_or_register_thread( existing = self._store.get(thread_id) if existing is not None: if existing.is_expired(self._default_ttl): - raise AgentMemoryThreadExpiredError( - f"Thread '{thread_id}' has expired and cannot be re-created with the same ID" - ) + _raise_thread_expired(thread_id) if existing.agent_id != agent_id or existing.invoker_id != invoker_id: - raise AgentMemoryThreadConflictError( - f"Thread '{thread_id}' is already registered by a different owner" - ) + _raise_thread_conflict(thread_id) existing.touch() return existing.to_entry(self._default_ttl) @@ -177,7 +178,7 @@ def get_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: return self._get_live(thread_id).to_entry(self._default_ttl) @@ -189,7 +190,7 @@ def list_threads( invoker_id: Optional[str] = None, limit: Optional[int] = None, offset: Optional[int] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadPage: if limit is not None: if isinstance(limit, bool) or not isinstance(limit, int) or limit < 0: @@ -231,7 +232,7 @@ def update_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: # PATCH-style merge: only fields the caller passed are modified. An # omitted ttl_seconds leaves the thread's existing TTL untouched. @@ -251,6 +252,6 @@ def delete_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> None: self._store.pop(thread_id, None) diff --git a/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py b/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py index 12720b65..b70f201a 100644 --- a/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py +++ b/src/sap_cloud_sdk/agent_memory/thread_registry/_service.py @@ -14,8 +14,6 @@ from sap_cloud_sdk.agent_memory.exceptions import ( AgentMemoryHttpError, AgentMemoryNotFoundError, - AgentMemoryThreadConflictError, - AgentMemoryThreadExpiredError, AgentMemoryThreadNotFoundError, AgentMemoryValidationError, ) @@ -23,6 +21,8 @@ BaseThreadRegistry, ThreadPage, ThreadRegistryEntry, + _raise_thread_conflict, + _raise_thread_expired, _validate_ttl, ) from sap_cloud_sdk.core.protocol.http import ( @@ -91,9 +91,9 @@ def _is_expired_response(exc: AgentMemoryHttpError) -> bool: """Return True if a 409 response indicates an expired thread (vs. conflict). Tries structured JSON error codes first (``error.code``, ``errorCode``), - then falls back to substring matching on both the exception message and the - raw response body. Two-layer detection means a service that upgrades from - prose-only errors to machine-readable codes is handled automatically. + then falls back to substring matching on the raw response body. Two-layer + detection means a service that upgrades from prose-only errors to + machine-readable codes is handled automatically. """ if exc.response_text: try: @@ -109,9 +109,10 @@ def _is_expired_response(exc: AgentMemoryHttpError) -> bool: # No code field — fall through to string matching below. except (ValueError, TypeError): pass # response body is not JSON - # String-match fallback: check both the exception message and response body. - combined = f"{exc} {exc.response_text or ''}".lower() - return "expired" in combined + # String-match fallback: search only the raw response body. The exception + # message embeds method/path/status context that could contain "expired" + # from an unrelated error (e.g. a 409 whose body says "OAuth token expired"). + return "expired" in (exc.response_text or "").lower() def _parse_entry(row: dict[str, Any]) -> ThreadRegistryEntry: @@ -260,7 +261,7 @@ def create_or_register_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: ttl = self._ttl(ttl_seconds) _validate_ttl(ttl) @@ -280,18 +281,13 @@ def create_or_register_thread( HttpMethod.POST, _THREAD_COLLECTION, json=body, - tenant_subdomain=tenant_id, + tenant_subdomain=tenant_subdomain, ) except AgentMemoryHttpError as exc: if thread_id is not None and exc.status_code == 409: if _is_expired_response(exc): - raise AgentMemoryThreadExpiredError( - f"Thread '{thread_id}' has expired " - f"and cannot be re-created with the same ID" - ) from exc - raise AgentMemoryThreadConflictError( - f"Thread '{thread_id}' is already registered by a different owner" - ) from exc + _raise_thread_expired(thread_id, from_exc=exc) + _raise_thread_conflict(thread_id, from_exc=exc) raise return _parse_entry(data) @@ -305,15 +301,15 @@ def get_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: - return self._get_thread(thread_id, tenant_id=tenant_id) + return self._get_thread(thread_id, tenant_subdomain=tenant_subdomain) def _get_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: # Undecorated implementation shared by the public get_thread and by # update_thread, so a single update_thread emits exactly one metric @@ -322,7 +318,7 @@ def _get_thread( data = self._request( HttpMethod.GET, _odata_entity_path(thread_id), - tenant_subdomain=tenant_id, + tenant_subdomain=tenant_subdomain, ) except AgentMemoryNotFoundError as exc: raise AgentMemoryThreadNotFoundError( @@ -342,7 +338,7 @@ def list_threads( invoker_id: Optional[str] = None, limit: Optional[int] = None, offset: Optional[int] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadPage: # Validate limit and offset before the limit==0 early-return. # The bool guard must come first: False == 0 in Python, so without it @@ -359,7 +355,9 @@ def list_threads( raise ValueError(f"offset must be a non-negative integer, got {offset!r}") if limit is not None and limit == 0: path = _build_list_path(agent_id, invoker_id, 0, resolved_offset) - body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + body = self._request( + HttpMethod.GET, path, tenant_subdomain=tenant_subdomain + ) return ThreadPage( entries=[], total=body.get("@odata.count", 0), @@ -369,7 +367,9 @@ def list_threads( if limit is not None: path = _build_list_path(agent_id, invoker_id, limit, resolved_offset) - body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + body = self._request( + HttpMethod.GET, path, tenant_subdomain=tenant_subdomain + ) return ThreadPage( entries=[_parse_entry(row) for row in body.get("value", [])], total=body.get("@odata.count", 0), @@ -379,13 +379,15 @@ def list_threads( # Unbounded — follow @odata.nextLink to collect all pages. path = _build_list_path(agent_id, invoker_id, None, resolved_offset) - body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_id) + body = self._request(HttpMethod.GET, path, tenant_subdomain=tenant_subdomain) total = body.get("@odata.count", 0) entries = [_parse_entry(row) for row in body.get("value", [])] next_link: Optional[str] = body.get("@odata.nextLink") while next_link: body = self._request( - HttpMethod.GET, self._strip_base(next_link), tenant_subdomain=tenant_id + HttpMethod.GET, + self._strip_base(next_link), + tenant_subdomain=tenant_subdomain, ) entries.extend(_parse_entry(row) for row in body.get("value", [])) next_link = body.get("@odata.nextLink") @@ -408,7 +410,7 @@ def update_thread( description: Optional[str] = None, ttl_seconds: Optional[int] = None, metadata: Optional[dict[str, Any]] = None, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> ThreadRegistryEntry: # PATCH-style merge: only fields the caller passed are sent. In # particular the registry default TTL is NOT reapplied here — a @@ -427,14 +429,14 @@ def update_thread( HttpMethod.PATCH, _odata_entity_path(thread_id), json=body, - tenant_subdomain=tenant_id, + tenant_subdomain=tenant_subdomain, ) except AgentMemoryNotFoundError as exc: raise AgentMemoryThreadNotFoundError( f"Thread '{thread_id}' not found" ) from exc - return self._get_thread(thread_id, tenant_id=tenant_id) + return self._get_thread(thread_id, tenant_subdomain=tenant_subdomain) # ------------------------------------------------------------------ # delete_thread @@ -445,13 +447,15 @@ def delete_thread( self, thread_id: str, *, - tenant_id: Optional[str] = None, + tenant_subdomain: Optional[str] = None, ) -> None: try: self._request( HttpMethod.DELETE, _odata_entity_path(thread_id), - tenant_subdomain=tenant_id, + tenant_subdomain=tenant_subdomain, ) except AgentMemoryNotFoundError: - pass + logger.debug( + "delete_thread: thread %r not found, treating as no-op", thread_id + ) diff --git a/src/sap_cloud_sdk/agent_memory/user-guide.md b/src/sap_cloud_sdk/agent_memory/user-guide.md index 4632f00c..6c8c8b3b 100644 --- a/src/sap_cloud_sdk/agent_memory/user-guide.md +++ b/src/sap_cloud_sdk/agent_memory/user-guide.md @@ -1,13 +1,9 @@ # Agent Memory User Guide -This module provides a Python client for the SAP Agent Memory service (v1 API). It lets agents -store, retrieve, and semantically search persistent memories, and record conversation messages -grouped into logical message groups. The service handles vector embeddings automatically for memories — you store -plain text, and the service makes it searchable by meaning. +This module provides a Python client for the SAP Agent Memory service (v1 API). It lets agents store, retrieve, and semantically search persistent memories, and record conversation messages grouped into logical message groups. The service handles vector embeddings automatically for memories — you store plain text, and the service makes it searchable by meaning. > [!NOTE] -> Memory extraction is the caller's responsibility. This client stores whatever text you pass -> as `content`; it does not extract or summarize memories from conversation text on its own. +> Memory extraction is the caller's responsibility. This client stores whatever text you pass as `content`; it does not extract or summarize memories from conversation text on its own. ## Table of Contents @@ -81,8 +77,7 @@ plain text, and the service makes it searchable by meaning. ## Installation -See the [SAP Cloud SDK for Python installation guide](https://github.com/SAP/cloud-sdk-python#installation) -for setup instructions. The agent memory module is included automatically. +See the [SAP Cloud SDK for Python installation guide](https://github.com/SAP/cloud-sdk-python#installation) for setup instructions. The agent memory module is included automatically. ## Import @@ -122,9 +117,7 @@ memories = client.list_memories(agent_id="my-agent", invoker_id="user-123") print(f"Found {len(memories)} memories") ``` -`create_client()` reads credentials from the `CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*` -environment variables (or a mounted volume on BTP). See the -[Configuration](#configuration) section for the full variable table. +`create_client()` reads credentials from the `CLOUD_SDK_CFG_HANA_AGENT_MEMORY_DEFAULT_*` environment variables (or a mounted volume on BTP). See the [Configuration](#configuration) section for the full variable table. ### Custom Configuration @@ -144,8 +137,7 @@ client = create_client(config=config) ### Using the Context Manager -The context manager is optional, but it is the easiest way to ensure the client is -closed even if an exception is raised: +The context manager is optional, but it is the easiest way to ensure the client is closed even if an exception is raised: ```python with create_client() as client: @@ -154,8 +146,7 @@ with create_client() as client: To close the client manually, call `client.close()`. -`close()` is only for local cleanup. It does **not** commit, flush, or roll back data. -Each API call is independent and final once accepted by the service. +`close()` is only for local cleanup. It does **not** commit, flush, or roll back data. Each API call is independent and final once accepted by the service. Calling methods after `close()` is supported. @@ -163,28 +154,22 @@ Calling methods after `close()` is supported. ### `agent_id` -A stable identifier for the agent that owns the data — for example `"hr-assistant"` or -`"support-bot"`. Chosen by the implementer; typically the name or ID of the AI agent. +A stable identifier for the agent that owns the data — for example `"hr-assistant"` or `"support-bot"`. Chosen by the implementer; typically the name or ID of the AI agent. ### `invoker_id` -Identifies the user or caller associated with the data — for example a user ID from -the application's auth system. Memories and messages are scoped to the combination of -`agent_id` and `invoker_id`. +Identifies the user or caller associated with the data — for example a user ID from the application's auth system. Memories and messages are scoped to the combination of `agent_id` and `invoker_id`. -Neither value is validated by the service — they are free-form strings. Consistent use -across create, read, and search calls is the implementer's responsibility. +Neither value is validated by the service — they are free-form strings. Consistent use across create, read, and search calls is the implementer's responsibility. ## Multitenancy - **Supported:** Yes -- **Authentication:** XSUAA -- **How to use:** Pass `access_strategy` and `tenant` to `create_client()`. The strategy controls whether calls use a subscriber-scoped or provider-scoped XSUAA token. Every method on the client inherits the strategy set at construction time. +- **Authentication:** XSUAA or IAS +- **How to use (XSUAA):** Pass `access_strategy` and `tenant` to `create_client()`. The strategy controls whether calls use a subscriber-scoped or provider-scoped XSUAA token. Every method on the client inherits the strategy set at construction time. +- **How to use (IAS):** Nothing to configure — tenant routing is driven by the per-request IAS token, not by `access_strategy`/`tenant` (which are ignored in IAS mode). The subscriber identity comes from the inbound IAS token placed into the request `RuntimeContext`. See [IAS binding](#ias-binding) under Configuration. - > [!NOTE] - > `access_strategy` and `tenant` apply to **XSUAA bindings only**. In **IAS mode** - > (auto-detected from a URL-only binding) they are ignored — routing is driven by the - > per-request IAS token. See [IAS binding](#ias-binding) under Configuration. + The `access_strategy` values below apply to **XSUAA bindings only**: | Value | Description | | ------------------------------------- | ----------------------------------------------------------------------------------------------- | @@ -198,15 +183,9 @@ across create, read, and search calls is the implementer's responsibility. ## Semantic Search: A Brief Primer -Texts with different words — or even different languages — can have the same meaning. -"How to make pizza dough?" and "Italian flatbread preparation steps" are semantically similar -despite sharing no words. To search a large corpus by meaning rather than exact keywords, the -service uses vector embeddings. +Texts with different words — or even different languages — can have the same meaning. "How to make pizza dough?" and "Italian flatbread preparation steps" are semantically similar despite sharing no words. To search a large corpus by meaning rather than exact keywords, the service uses vector embeddings. -An embedding model translates a text into a high-dimensional numeric vector. Texts with similar -meaning produce vectors that point in a similar direction. The cosine similarity between two -vectors measures that directional closeness: a value near 1.0 means the texts are semantically -similar. +An embedding model translates a text into a high-dimensional numeric vector. Texts with similar meaning produce vectors that point in a similar direction. The cosine similarity between two vectors measures that directional closeness: a value near 1.0 means the texts are semantically similar. Example corpus: @@ -216,16 +195,13 @@ Example corpus: - "Wolves hunt deer" - "Engines power ships" -A search for "Sky illumination" returns "Clouds block sunlight" — closest in meaning, with the -highest cosine similarity — even though the query shares no words with the result. +A search for "Sky illumination" returns "Clouds block sunlight" — closest in meaning, with the highest cosine similarity — even though the query shares no words with the result. -`search_memories()` uses this mechanism: you pass a natural-language query and a similarity -threshold, and the service returns the most semantically relevant stored memories. +`search_memories()` uses this mechanism: you pass a natural-language query and a similarity threshold, and the service returns the most semantically relevant stored memories. ## Memories -Memories are persistent knowledge entries scoped to an `agent_id` + `invoker_id` pair. -The service generates a vector embedding for each memory automatically, enabling semantic search. +Memories are persistent knowledge entries scoped to an `agent_id` + `invoker_id` pair. The service generates a vector embedding for each memory automatically, enabling semantic search. ### Create a Memory @@ -260,8 +236,7 @@ print(memory.content) ### Update a Memory -`update_memory` performs a partial update; -omitted fields remain untouched. +`update_memory` performs a partial update; omitted fields remain untouched. > [!NOTE] > `content` and `metadata` are the only editable fields; `memory_id` identifies which memory to update and cannot be modified @@ -334,9 +309,7 @@ memories = client.list_memories( `target` must be `"content"` or `"metadata"`. Multiple clauses are combined with AND. > [!WARNING] -> Defining two clauses with the **same target** produces an AND predicate that requires -> both substrings to be present in the same field simultaneously. This is rarely -> intentional — for example: +> Defining two clauses with the **same target** produces an AND predicate that requires both substrings to be present in the same field simultaneously. This is rarely intentional — for example: > > ```python > filters=[ @@ -345,14 +318,10 @@ memories = client.list_memories( > ] > ``` > -> Only memories whose content contains _both_ substrings will be returned, which is -> typically an empty result set. OR combining across clauses is not yet supported. +> Only memories whose content contains _both_ substrings will be returned, which is typically an empty result set. OR combining across clauses is not yet supported. > [!NOTE] -> Metadata is stored as a JSON string. Filtering on `"metadata"` performs a free-text -> substring match on the raw JSON — for example `contains="preferences"` matches any -> metadata whose serialized form includes that word. Structured key-value filtering -> (e.g. `metadata.source == "preferences"`) is not supported. +> Metadata is stored as a JSON string. Filtering on `"metadata"` performs a free-text substring match on the raw JSON — for example `contains="preferences"` matches any metadata whose serialized form includes that word. Structured key-value filtering (e.g. `metadata.source == "preferences"`) is not supported. ### Count Memories @@ -375,8 +344,7 @@ print(f"Total memories: {total}") ### Semantic Search -Search for memories whose meaning is similar to a natural-language query. The service returns -results ordered by relevance (highest similarity first). +Search for memories whose meaning is similar to a natural-language query. The service returns results ordered by relevance (highest similarity first). ```python results = client.search_memories( @@ -408,9 +376,7 @@ for r in results: ## Messages -Messages represent individual turns in a conversation. Messages sharing the same `message_group` -form a logical message group. The service does not enforce a session concept — grouping is done -entirely via the `message_group` value you choose. +Messages represent individual turns in a conversation. Messages sharing the same `message_group` form a logical message group. The service does not enforce a session concept — grouping is done entirely via the `message_group` value you choose. ### Create a Message @@ -517,16 +483,13 @@ messages = client.list_messages( ) ``` -See the [Content and metadata filtering](#content-and-metadata-filtering) note under -[List Memories](#list-memories) for details on metadata free-text limitations. +See the [Content and metadata filtering](#content-and-metadata-filtering) note under [List Memories](#list-memories) for details on metadata free-text limitations. --- ## Thread Registry -The thread registry tracks conversation threads — their owner, idle TTL, and expiry. Unlike -the LangGraph checkpointer, it is **framework-agnostic**: it works with LangGraph, Pydantic AI, -CrewAI, or any custom agent. +The thread registry tracks conversation threads — their owner, idle TTL, and expiry. Unlike the LangGraph checkpointer, it is **framework-agnostic**: it works with LangGraph, Pydantic AI, CrewAI, or any custom agent. Use `create_thread_registry()` to create a registry for the current environment: @@ -541,7 +504,7 @@ registry = create_thread_registry(ttl_seconds=3600) # 1-hour idle TTL - **Credentials present** — returns `SAPAgentMemoryThreadRegistry` backed by the Agent Memory Service. Thread state is persistent across restarts and enforced server-side. - **No credentials** — falls back to `InMemoryThreadRegistry`. State is in-process only and - will not survive restarts. + will not survive restarts. It accepts `tenant_subdomain` for signature parity with the service-backed registry but **ignores it** — all threads share a single store, so tenant isolation is not available in this mode. You can also import the concrete types and the `BaseThreadRegistry` ABC directly: @@ -555,8 +518,7 @@ from sap_cloud_sdk.agent_memory.thread_registry import ( ### Create or Register a Thread -`create_or_register_thread()` is idempotent: calling it with the same `thread_id` a second -time touches the thread (resetting its idle TTL) rather than raising an error. +`create_or_register_thread()` is idempotent: calling it with the same `thread_id` a second time touches the thread (resetting its idle TTL) rather than raising an error. ```python entry = registry.create_or_register_thread( @@ -601,9 +563,7 @@ entry = registry.update_thread( ) ``` -`update_thread()` is a PATCH-style merge — only the fields you pass are modified. Omit -`ttl_seconds` to leave the thread's existing TTL untouched (mirrors LangGraph's -`threads.update`). +`update_thread()` is a PATCH-style merge — only the fields you pass are modified. Omit `ttl_seconds` to leave the thread's existing TTL untouched (mirrors LangGraph's `threads.update`). ### Delete a Thread @@ -613,22 +573,18 @@ registry.delete_thread("session-abc") # idempotent — no error if already gone ### Thread TTL and lifecycle -**Sliding idle TTL**: `ttl_seconds` is an *inactivity* window. The expiry resets on each -write (register, update). A thread inactive for longer than `ttl_seconds` is treated as -expired. `None` means threads never expire. +**Sliding idle TTL**: `ttl_seconds` is an *inactivity* window. The expiry resets on each write (register, update). A thread inactive for longer than `ttl_seconds` is treated as expired. `None` means threads never expire. TTL precedence (highest to lowest): 1. Per-call `ttl_seconds` passed to `create_or_register_thread()` / `update_thread()` 2. `create_thread_registry(ttl_seconds=...)` registry-level default 3. No expiry (`None`) -`InMemoryThreadRegistry` evaluates expiry lazily at read time; the service-backed registry -enforces expiry server-side via the `expiresAt` field. +`InMemoryThreadRegistry` evaluates expiry lazily at read time; the service-backed registry enforces expiry server-side via the `expiresAt` field. ### Resource Management: `close()` and Context Manager -`SAPAgentMemoryThreadRegistry` owns an HTTP transport with an OAuth token cache and connection -pool. Call `close()` when the registry is no longer needed, or use the context manager: +`SAPAgentMemoryThreadRegistry` owns an HTTP transport with an OAuth token cache and connection pool. Call `close()` when the registry is no longer needed, or use the context manager: ```python # Explicit close @@ -646,15 +602,11 @@ with create_thread_registry() as registry: ``` > [!NOTE] -> `InMemoryThreadRegistry.close()` is a no-op — it has no external resources to release. -> The context manager interface is available on all registry types for uniform usage. +> `InMemoryThreadRegistry.close()` is a no-op — it has no external resources to release. The context manager interface is available on all registry types for uniform usage. ### Exposing idle TTL with `@agent_config` -Use `@agent_config` from `sap_cloud_sdk.agent_decorators` to expose `ttl_seconds` as an -operator-adjustable configuration field. The key `config.registry.ttl_seconds` groups it -with other registry settings so external tooling can surface it in the low-code UI alongside -model selection and temperature. +Use `@agent_config` from `sap_cloud_sdk.agent_decorators` to expose `ttl_seconds` as an operator-adjustable configuration field. The key `config.registry.ttl_seconds` groups it with other registry settings so external tooling can surface it in the low-code UI alongside model selection and temperature. ```python from sap_cloud_sdk.agent_decorators import agent_config @@ -678,33 +630,17 @@ class MyAgent: ``` > [!NOTE] -> `InMemoryThreadRegistry` state does not survive process restarts. When Agent Memory Service -> credentials are available, `create_thread_registry()` returns `SAPAgentMemoryThreadRegistry` -> instead, and thread expiry is enforced server-side via the `expiresAt` field on the thread -> registry row. +> `InMemoryThreadRegistry` state does not survive process restarts. When Agent Memory Service credentials are available, `create_thread_registry()` returns `SAPAgentMemoryThreadRegistry` instead, and thread expiry is enforced server-side via the `expiresAt` field on the thread registry row. ## LangGraph Checkpointer -> This section covers the LangGraph-specific -> factory for short-term memory (checkpointing). It is only relevant if your agent -> is built with LangGraph or LangChain's `create_agent()`. The core -> `AgentMemoryClient` and `create_client()` above are framework-agnostic and work -> independently of this section. - -For LangGraph agents, the `agent_memory` module provides a `create_checkpointer()` -factory in the `factory` subpackage. It returns a `BaseCheckpointSaver` that -manages short-term session memory — conversation state, thread continuity, and -HITL support — natively within LangGraph. - -`create_checkpointer()` returns an `InMemorySaver`. State is in-process only and -will not survive process restarts. - -> **Deprecated — `create_checkpointer(ttl_seconds=...)`.** Passing `ttl_seconds` -> returns a `TimedInMemorySaver` that evicts inactive threads. This is deprecated: -> thread lifecycle is owned by the **thread registry**, and configuring TTL in two -> places leads to drift. The parameter still works and will not be removed yet -> (existing code keeps running), but it now emits a `DeprecationWarning`. Configure -> TTL on `create_thread_registry()` instead: +> This section covers the LangGraph-specific factory for short-term memory (checkpointing). It is only relevant if your agent is built with LangGraph or LangChain's `create_agent()`. The core `AgentMemoryClient` and `create_client()` above are framework-agnostic and work independently of this section. + +For LangGraph agents, the `agent_memory` module provides a `create_checkpointer()` factory in the `factory` subpackage. It returns a `BaseCheckpointSaver` that manages short-term session memory — conversation state, thread continuity, and HITL support — natively within LangGraph. + +`create_checkpointer()` returns an `InMemorySaver`. State is in-process only and will not survive process restarts. + +> **Deprecated — `create_checkpointer(ttl_seconds=...)`.** Passing `ttl_seconds` returns a `TimedInMemorySaver` that evicts inactive threads. This is deprecated: thread lifecycle is owned by the **thread registry**, and configuring TTL in two places leads to drift. The parameter still works and will not be removed yet (existing code keeps running), but it now emits a `DeprecationWarning`. Configure TTL on `create_thread_registry()` instead: > > ```python > # Deprecated — checkpointer-level TTL @@ -804,30 +740,21 @@ The `uaa` key must contain a JSON string with the XSUAA credentials: ### IAS binding -IAS mode is **auto-detected** — a URL-only binding (`application_url`, no `uaa`/`token_url`) -selects IAS automatically. There is nothing extra to configure on `create_client()`: -`access_strategy` and `tenant` are **ignored** in IAS mode (requests are routed by the -per-request token, not by an XSUAA strategy). +IAS mode is **auto-detected** — a URL-only binding (`application_url`, no `uaa`/`token_url`) selects IAS automatically. There is nothing extra to configure on `create_client()`: `access_strategy` and `tenant` are **ignored** in IAS mode (requests are routed by the per-request token, not by an XSUAA strategy). -Unlike XSUAA, the client holds no credentials and fetches no token of its own. The Bearer -token travels **per request**, read from the request `RuntimeContext` at call time. +Unlike XSUAA, the client holds no credentials and fetches no token of its own. The Bearer token travels **per request**, read from the request `RuntimeContext` at call time. > [!IMPORTANT] -> **The agent must wire `bootstrap(app)` / `IASContextProvider`.** This is what parses the -> inbound `Authorization` header and places the IAS token into the `RuntimeContext` that the -> client reads. Without it, **every** Agent Memory and Thread Registry call fails with: +> **The agent must wire `bootstrap(app)` / `IASContextProvider`.** This is what parses the inbound `Authorization` header and places the IAS token into the `RuntimeContext` that the client reads. Without it, **every** Agent Memory and Thread Registry call fails with: > > ``` > AgentMemoryValidationError: IAS token not found in RuntimeContext. Ensure bootstrap(app) is configured. > ``` > -> Wire it once at startup — for example `bootstrap(app)` on your FastAPI/Flask app — so each -> inbound request populates the context before any client call runs. +> Wire it once at startup — for example `bootstrap(app)` on your FastAPI/Flask app — so each inbound request populates the context before any client call runs. > [!NOTE] -> **Feedback / future improvement:** the cloud-sdk could auto-bootstrap the IAS context -> wiring when it detects an IAS binding at runtime, so agents would not have to call -> `bootstrap(app)` manually. Until then, wiring the context provider is a required manual step. +> **Feedback / future improvement:** the cloud-sdk could auto-bootstrap the IAS context wiring when it detects an IAS binding at runtime, so agents would not have to call `bootstrap(app)` manually. Until then, wiring the context provider is a required manual step. ## Data Models @@ -865,8 +792,7 @@ print(memory.to_dict()) ## Error Handling -The module defines a structured exception hierarchy so you can catch errors at the appropriate -level of specificity: +The module defines a structured exception hierarchy so you can catch errors at the appropriate level of specificity: ``` AgentMemoryError @@ -922,8 +848,7 @@ except AgentMemoryError as e: ## Admin — Retention Config -The retention configuration controls automatic data cleanup. It is a singleton — one config -per tenant. +The retention configuration controls automatic data cleanup. It is a singleton — one config per tenant. ### Get Retention Config @@ -937,8 +862,7 @@ print(f"Registered threads: {rc.thread_days} days") ### Update Retention Config -`update_retention_config` performs a partial update — only the provided fields are -changed; omitted fields remain unchanged. +`update_retention_config` performs a partial update — only the provided fields are changed; omitted fields remain unchanged. ```python client.update_retention_config( @@ -949,29 +873,20 @@ client.update_retention_config( ) ``` -`thread_days` is the default TTL (in days) applied **only to registered threads** -(those created via the Thread Registry API). It is the age-based cutoff for thread -cleanup; threads not registered with the Thread Registry are unaffected. +`thread_days` is the default TTL (in days) applied **only to registered threads** (those created via the Thread Registry API). It is the age-based cutoff for thread cleanup; threads not registered with the Thread Registry are unaffected. -Set a field to `0` to mark all data in that category for deletion at the next nightly scheduled cleanup. The server also accepts `null` to disable -automatic cleanup for that category. +Set a field to `0` to mark all data in that category for deletion at the next nightly scheduled cleanup. The server also accepts `null` to disable automatic cleanup for that category. **When changes take effect** The service runs nightly data cleanup procedures that delete records based on creation timestamp. Changes to retention configuration apply to all future retention sweeps. The new retention window is calculated from each record's original creation timestamp, not from the time of the config change. -_Increasing retention_ — records that were approaching expiry get more time. For example, -if `message_days` is raised from 90 to 120, a message created 89 days ago will now be -retained until it reaches 120 days old rather than being cleaned up after 90 days. +_Increasing retention_ — records that were approaching expiry get more time. For example, if `message_days` is raised from 90 to 120, a message created 89 days ago will now be retained until it reaches 120 days old rather than being cleaned up after 90 days. -_Decreasing retention_ — records outside the new window become eligible for removal. For -example, if `message_days` is reduced from 90 to 30, messages older than 30 days will be -removed at the next retention sweep, even if they fell within the original 90-day limit -when they were created. +_Decreasing retention_ — records outside the new window become eligible for removal. For example, if `message_days` is reduced from 90 to 30, messages older than 30 days will be removed at the next retention sweep, even if they fell within the original 90-day limit when they were created. > [!WARNING] -> Decreasing a retention period is a destructive, irreversible operation. Records outside -> the new window are permanently deleted at the next cleanup sweep. +> Decreasing a retention period is a destructive, irreversible operation. Records outside the new window are permanently deleted at the next cleanup sweep. --- @@ -1032,8 +947,7 @@ def get_conversation(client, agent_id, invoker_id, group_id): ### Paginating through all memories -`list_memories` returns at most `limit` results per call. Use `offset` to page through large -sets, or use `count_memories` first to decide whether pagination is even necessary: +`list_memories` returns at most `limit` results per call. Use `offset` to page through large sets, or use `count_memories` first to decide whether pagination is even necessary: ```python PAGE_SIZE = 100 @@ -1123,14 +1037,11 @@ results = client.search_memories( ### `AgentMemoryNotFoundError` when fetching a resource -The resource was deleted, the ID is incorrect, or the `agent_id`/`invoker_id` passed to a -list or search operation does not match the values used when the resource was created. +The resource was deleted, the ID is incorrect, or the `agent_id`/`invoker_id` passed to a list or search operation does not match the values used when the resource was created. ### `AgentMemoryHttpError` with status 401 -The OAuth2 token has expired and automatic refresh failed, or the configured credentials -(`client_id`, `client_secret`, `token_url`) are incorrect. Verify the credentials in your -environment variables or service binding. +The OAuth2 token has expired and automatic refresh failed, or the configured credentials (`client_id`, `client_secret`, `token_url`) are incorrect. Verify the credentials in your environment variables or service binding. ### `AgentMemoryValidationError: IAS token not found in RuntimeContext` @@ -1138,9 +1049,7 @@ environment variables or service binding. AgentMemoryValidationError: IAS token not found in RuntimeContext. Ensure bootstrap(app) is configured. ``` -The client is running in **IAS mode** but no IAS token reached the request `RuntimeContext`. -The agent must wire `bootstrap(app)` / `IASContextProvider` so the inbound `Authorization` -header is parsed into the context before any client call. See [IAS binding](#ias-binding). +The client is running in **IAS mode** but no IAS token reached the request `RuntimeContext`. The agent must wire `bootstrap(app)` / `IASContextProvider` so the inbound `Authorization` header is parsed into the context before any client call. See [IAS binding](#ias-binding). --- diff --git a/src/sap_cloud_sdk/core/protocol/http/models.py b/src/sap_cloud_sdk/core/protocol/http/models.py index 99e73180..71b247d6 100644 --- a/src/sap_cloud_sdk/core/protocol/http/models.py +++ b/src/sap_cloud_sdk/core/protocol/http/models.py @@ -206,6 +206,11 @@ def get_session(self, tenant_subdomain: Optional[str] = None) -> requests.Sessio ias_token = get_context().get(IAS_TOKEN) if ias_token: session.headers["Authorization"] = f"Bearer {ias_token}" + else: + logger.debug( + "IasAuthProvider: no IAS token in context, " + "session has no Authorization header" + ) return session def invalidate(self, tenant_subdomain: Optional[str] = None) -> None: diff --git a/tests/agent_memory/unit/test_thread_registry_service.py b/tests/agent_memory/unit/test_thread_registry_service.py index afaf62fc..c2d226a3 100644 --- a/tests/agent_memory/unit/test_thread_registry_service.py +++ b/tests/agent_memory/unit/test_thread_registry_service.py @@ -132,8 +132,11 @@ def test_plain_text_conflict_returns_false(self): is False ) - def test_expired_in_exception_message_not_body(self): - assert _is_expired_response(self._exc(msg="expired thread")) is True + def test_expired_in_exception_message_not_body_returns_false(self): + # The fallback searches only the response body, never the exception + # message — an unrelated 409 whose prose mentions "expired" (e.g. an + # OAuth-token error) must not be misclassified as a thread expiry. + assert _is_expired_response(self._exc(msg="expired thread")) is False def test_json_error_code_expired(self): import json @@ -276,7 +279,7 @@ def test_per_call_ttl_overrides_default(self): def test_tenant_id_forwarded(self): reg = _make_registry() reg._request.return_value = _ENTRY.copy() - reg.create_or_register_thread("agent-a", "user-1", tenant_id="acme") + reg.create_or_register_thread("agent-a", "user-1", tenant_subdomain="acme") assert reg._request.call_args[1]["tenant_subdomain"] == "acme" def test_no_tenant_id_passes_none(self): @@ -365,7 +368,7 @@ def test_not_found_raises_thread_not_found(self): def test_tenant_id_forwarded(self): reg = _make_registry() reg._request.return_value = _ENTRY.copy() - reg.get_thread("t-1", tenant_id="acme") + reg.get_thread("t-1", tenant_subdomain="acme") assert reg._request.call_args[1]["tenant_subdomain"] == "acme" @@ -474,7 +477,7 @@ def test_invalid_offset_raises_value_error(self): def test_tenant_id_forwarded(self): reg = _make_registry() reg._request.return_value = self._page_response([]) - reg.list_threads(tenant_id="acme") + reg.list_threads(tenant_subdomain="acme") assert reg._request.call_args[1]["tenant_subdomain"] == "acme" @@ -553,7 +556,7 @@ def test_returns_refreshed_entry(self): def test_tenant_id_forwarded_to_both_calls(self): reg = _make_registry() reg._request.side_effect = [{}, _ENTRY.copy()] - reg.update_thread("t-1", tenant_id="acme") + reg.update_thread("t-1", tenant_subdomain="acme") assert reg._request.call_args_list[0][1]["tenant_subdomain"] == "acme" assert reg._request.call_args_list[1][1]["tenant_subdomain"] == "acme" @@ -587,7 +590,7 @@ def test_idempotent_swallows_not_found(self): def test_tenant_id_forwarded(self): reg = _make_registry() reg._request.return_value = {} - reg.delete_thread("t-1", tenant_id="acme") + reg.delete_thread("t-1", tenant_subdomain="acme") assert reg._request.call_args[1]["tenant_subdomain"] == "acme" def test_other_http_errors_propagate(self):