feat(memory-tools): accept client supplied memory ids in CreateMemoryTool - #27
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for application-supplied memory IDs to CreateMemoryTool.run_async to enable idempotent retries for managed Redis Agent Memory writes, while explicitly not exposing the parameter to the LLM tool declaration. For the self-hosted backend, the tool warns that client IDs are not supported and proceeds with the server-generated ID.
Changes:
- Accept optional
idinCreateMemoryTool.run_async, sanitize it for the managed backend, and use it as the record ID to support upserts on retries. - Log a warning when
idis provided for the self-hostedopensource-agent-memorybackend (which cannot honor client IDs). - Add tests for client ID passthrough, sanitization, stable-id fallback, idempotency behavior, and the self-hosted warning path; document the behavior in the memory concepts docs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/adk_redis/tools/memory/create.py |
Adds optional client-supplied id handling, managed-ID sanitization, and self-hosted warning behavior. |
tests/tools/test_memory_tools.py |
Adds coverage for client ID behavior (managed) and warning behavior (self-hosted). |
docs/concepts/memory.md |
Documents the optional application-level id and the differing backend behaviors. |
Suppressed comments (1)
src/adk_redis/tools/memory/create.py:205
client_memory_idis checked using truthiness (if client_memory_id:), which treats values like0as absent and falls back tostable_memory_id. If callers derive IDs from numeric task IDs, this breaks the idempotency contract. Use an explicitNonecheck so any provided ID value is honored (or rejected by sanitization).
if client_memory_id:
memory_id = sanitize_managed_identifier(str(client_memory_id))
else:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d20ca6e20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/adk_redis/tools/memory/create.py:177
- The warning for the self-hosted backend logs the raw client-supplied memory id. Since this is an application-level id (often derived from queue/task identifiers), logging it at WARNING can unintentionally leak sensitive identifiers into logs. Consider logging the limitation without echoing the provided id (or log a redacted/hash form).
logger.warning(
"Client-supplied memory id %r is not supported by the "
"opensource-agent-memory backend: add_memory_tool generates "
"its own memory ID. Proceeding without the client id.",
client_memory_id,
53cb536 to
a121b25
Compare
Closes #24
CreateMemoryTool.run_asyncnow accepts an optional application-levelid. On the managed Redis Agent Memory backend, the ID is combined with the resolved namespace and user to derive a collision-resistant, managed-safe record ID. A retry with the same application ID in the same scope upserts instead of creating a duplicate, while equal IDs in different user or namespace scopes remain isolated. When absent, the existingstable_memory_idfallback is unchanged.Design decisions:
opensource-agent-memorybackend cannot honor client IDs becauseadd_memory_toolgenerates its own ID. The tool logs the limitation without exposing the supplied ID and proceeds with a server-generated ID.main, including invocation-user resolution throughtool_context.Tests cover scoped ID derivation, raw-ID collision resistance, namespace/user isolation including
tool_contextusers, numeric zero, stable fallback behavior, same-ID idempotency, and redacted self-hosted warnings.make checkpasses fully: format, lint, mypy, 142 passed, 9 skipped.