diff --git a/docs/concepts/memory.md b/docs/concepts/memory.md index f548a24..d22f66e 100644 --- a/docs/concepts/memory.md +++ b/docs/concepts/memory.md @@ -146,6 +146,15 @@ an explicit `user_id` argument, the invocation user from the ADK tool context, `default_owner_id`, then `default_user_id`. Configured defaults are only used when no per-call or context user is available. +For idempotent retries, `CreateMemoryTool.run_async` accepts an optional +application-level `id` argument that is not exposed to the LLM. On the managed +Redis Agent Memory backend, a client-supplied `id` is combined with the resolved +namespace and user to derive a collision-resistant, managed-safe record ID. +Retrying with the same `id` in the same scope upserts instead of creating a +duplicate, while the same application ID in another scope remains isolated. +The self-hosted `opensource-agent-memory` backend cannot honor client IDs; the +tool logs a warning and writes with a server-generated ID. + ## MCP vs SDK Decision | | MCP | SDK Tools | diff --git a/src/adk_redis/tools/memory/create.py b/src/adk_redis/tools/memory/create.py index df11423..badb744 100644 --- a/src/adk_redis/tools/memory/create.py +++ b/src/adk_redis/tools/memory/create.py @@ -126,6 +126,18 @@ async def run_async(self, **kwargs: Any) -> dict[str, Any]: user_id: Optional user ID override. When omitted, the user is resolved from the ADK tool_context invocation user, then the configured defaults. + id: Optional client-supplied memory ID for idempotent retries. + This is an application-level parameter and is intentionally + not exposed in the LLM tool declaration; direct callers pass + it via ``run_async(id=..., content=...)`` or + ``run_async(args={"id": ..., ...})``. On the managed Redis + Agent Memory backend the ID and resolved namespace/user scope + are mapped to a collision-resistant record ID, so retrying with + the same ID in the same scope upserts instead of creating a + duplicate. The self-hosted + opensource-agent-memory backend cannot honor client IDs; a + warning is logged and the write proceeds with a + server-generated ID. Returns: A dictionary with status and memory_id. @@ -139,6 +151,7 @@ async def run_async(self, **kwargs: Any) -> dict[str, Any]: memory_type_raw = args.get("memory_type", "semantic") namespace = self._get_namespace(args.get("namespace")) user_id = self._get_user_id(args.get("user_id"), tool_context=tool_context) + client_memory_id = args.get("id") if not content: return {"status": "error", "message": "content is required"} @@ -158,6 +171,12 @@ async def run_async(self, **kwargs: Any) -> dict[str, Any]: try: if self._config.backend == OPENSOURCE_AGENT_MEMORY_BACKEND: + if client_memory_id is not None: + logger.warning( + "A client-supplied memory id is not supported by the " + "opensource-agent-memory backend: add_memory_tool generates " + "its own memory ID. Proceeding without the client id." + ) session_id = f"standalone_{uuid.uuid4().hex[:8]}" response = await self._get_agent_memory_server_client().add_memory_tool( session_id=session_id, @@ -182,13 +201,24 @@ async def run_async(self, **kwargs: Any) -> dict[str, Any]: "message": response.get("summary", "Failed to create memory"), } - memory_id = stable_memory_id( - "tool", - namespace, - user_id or "", - memory_type, - content, - ) + if client_memory_id is not None: + raw_client_memory_id = str(client_memory_id) + if not raw_client_memory_id.strip(): + raise ValueError("Client-supplied memory id must not be empty") + memory_id = stable_memory_id( + "client", + namespace, + user_id or "", + raw_client_memory_id, + ) + else: + memory_id = stable_memory_id( + "tool", + namespace, + user_id or "", + memory_type, + content, + ) record = { "id": memory_id, "text": content, diff --git a/tests/tools/test_memory_tools.py b/tests/tools/test_memory_tools.py index f87d7e6..de73ed8 100644 --- a/tests/tools/test_memory_tools.py +++ b/tests/tools/test_memory_tools.py @@ -15,6 +15,7 @@ """Tests for Redis Agent Memory tools.""" import asyncio +import logging from types import SimpleNamespace from unittest.mock import patch @@ -22,6 +23,7 @@ from adk_redis import OPENSOURCE_AGENT_MEMORY_BACKEND from adk_redis import REDIS_AGENT_MEMORY_BACKEND +from adk_redis.memory._utils import stable_memory_id from adk_redis.tools.memory import CreateMemoryTool from adk_redis.tools.memory import DeleteMemoryTool from adk_redis.tools.memory import MemoryPromptTool @@ -236,6 +238,125 @@ async def test_create_memory_tool_writes_record(config, fake_client): assert fake_client.created_records[0]["memoryType"] == "semantic" +@pytest.mark.asyncio +async def test_create_memory_tool_uses_client_supplied_id(config, fake_client): + """CreateMemoryTool derives a scoped ID from a client-supplied id.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + result = await tool.run_async( + args={"content": "User likes tea.", "id": "retry-abc-123"} + ) + + assert result["status"] == "success" + expected_id = stable_memory_id("client", "test-ns", "alice", "retry-abc-123") + assert result["memory_id"] == expected_id + assert fake_client.created_records[0]["id"] == expected_id + + +@pytest.mark.asyncio +async def test_create_memory_tool_preserves_client_id_uniqueness( + config, fake_client +): + """Distinct raw client IDs remain distinct after managed-safe mapping.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + await tool.run_async( + args={"content": "User likes tea.", "id": "retry:abc_123"} + ) + await tool.run_async( + args={"content": "User likes tea.", "id": "retry_abc_123"} + ) + + assert ( + fake_client.created_records[0]["id"] + != fake_client.created_records[1]["id"] + ) + + +@pytest.mark.asyncio +async def test_create_memory_tool_scopes_client_id(config, fake_client): + """The same application ID cannot collide across users.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + await tool.run_async(args={"content": "Alice memory", "id": "request-1"}) + await tool.run_async( + args={ + "content": "Bob memory", + "id": "request-1", + "user_id": "bob", + } + ) + + assert ( + fake_client.created_records[0]["id"] + != fake_client.created_records[1]["id"] + ) + + +@pytest.mark.asyncio +async def test_create_memory_tool_scopes_client_id_to_context_user( + config, fake_client +): + """Invocation users isolate the same application-level client ID.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + await tool.run_async( + args={"content": "Alice memory", "id": "request-1"}, + tool_context=SimpleNamespace(user_id="alice"), + ) + await tool.run_async( + args={"content": "Bob memory", "id": "request-1"}, + tool_context=SimpleNamespace(user_id="bob"), + ) + + assert ( + fake_client.created_records[0]["id"] + != fake_client.created_records[1]["id"] + ) + + +@pytest.mark.asyncio +async def test_create_memory_tool_treats_zero_as_client_id(config, fake_client): + """A numeric zero is a supplied client ID, not a missing value.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + result = await tool.run_async(args={"content": "User likes tea.", "id": 0}) + + expected_id = stable_memory_id("client", "test-ns", "alice", "0") + assert result["status"] == "success" + assert fake_client.created_records[0]["id"] == expected_id + + +@pytest.mark.asyncio +async def test_create_memory_tool_defaults_to_stable_id(config, fake_client): + """CreateMemoryTool falls back to the stable content-derived ID.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + result = await tool.run_async(args={"content": "User likes tea."}) + + expected_id = stable_memory_id( + "tool", "test-ns", "alice", "semantic", "User likes tea." + ) + assert result["status"] == "success" + assert fake_client.created_records[0]["id"] == expected_id + + +@pytest.mark.asyncio +async def test_create_memory_tool_same_id_is_idempotent(config, fake_client): + """Two calls with the same client id send the same record ID.""" + tool = CreateMemoryTool(config=config) + with patch.object(tool, "_get_client", return_value=fake_client): + await tool.run_async(args={"content": "First attempt.", "id": "retry-1"}) + await tool.run_async(args={"content": "First attempt.", "id": "retry-1"}) + + assert len(fake_client.created_records) == 2 + assert ( + fake_client.created_records[0]["id"] + == fake_client.created_records[1]["id"] + == stable_memory_id("client", "test-ns", "alice", "retry-1") + ) + + @pytest.mark.asyncio async def test_search_memory_tool_uses_owner_and_namespace_filter( config, fake_client @@ -425,3 +546,31 @@ async def test_create_memory_tool_can_use_agent_memory_server_backend(): assert fake_client.add_memory_kwargs["text"] == "User likes tea." assert fake_client.add_memory_kwargs["namespace"] == "test_ns" assert fake_client.add_memory_kwargs["user_id"] == "alice" + + +@pytest.mark.asyncio +async def test_create_memory_tool_warns_on_client_id_for_self_hosted(caplog): + """Self-hosted warning explains behavior without exposing the client id.""" + fake_client = FakeAgentMemoryServerClient() + config = MemoryToolConfig( + backend=OPENSOURCE_AGENT_MEMORY_BACKEND, + default_namespace="test_ns", + default_user_id="alice", + ) + tool = CreateMemoryTool(config=config) + + with patch.object( + tool, + "_get_agent_memory_server_client", + return_value=fake_client, + ): + with caplog.at_level(logging.WARNING, logger="adk_redis"): + result = await tool.run_async( + args={"content": "User likes tea.", "id": "retry-abc-123"} + ) + + assert result["status"] == "success" + assert fake_client.add_memory_kwargs["text"] == "User likes tea." + assert "retry-abc-123" not in caplog.text + assert "not supported by the opensource-agent-memory backend" in caplog.text + assert "Proceeding without the client id" in caplog.text