feat(memory-tools): resolve user from ADK tool_context - #26
Conversation
Memory tools now resolve the operation user in four steps: the explicit user_id argument, the invocation user from the ADK tool_context, then config.default_owner_id and config.default_user_id. The context user is read defensively so a broken context never crashes a tool, and direct callers that pass user_id or rely on config defaults are unchanged. Closes #23
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29e5741342
ℹ️ 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
This PR updates the adk-redis memory tool suite to resolve the acting user from the ADK tool_context when no explicit user_id is provided, enabling safe reuse of a process-wide Agent and Runner in multi-user workers while preserving existing config-default behavior.
Changes:
- Added defensive
tool_context.user_idhandling to the shared_get_user_idresolution order. - Plumbed
tool_contextthrough memory toolrun_asyncimplementations (create, search, update, delete, prompt). - Added tests for user resolution and end-to-end scoping behavior, and documented the new resolution order.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tools/test_memory_tools.py | Adds coverage for user resolution order and tool_context-based scoping in tool execution. |
| src/adk_redis/tools/memory/_base.py | Implements the new _get_user_id(..., tool_context=...) resolution order with defensive context reads. |
| src/adk_redis/tools/memory/create.py | Uses resolved user (including tool_context) when creating memories and stamping ownerId. |
| src/adk_redis/tools/memory/search.py | Uses resolved user (including tool_context) to scope search filters by ownerId. |
| src/adk_redis/tools/memory/update.py | Uses resolved user (including tool_context) to scope updates via owner_id where applicable. |
| src/adk_redis/tools/memory/delete.py | Wires tool_context into user resolution and updates documentation around user_id. |
| src/adk_redis/tools/memory/prompt.py | Uses resolved user (including tool_context) to scope prompt enrichment memory search. |
| docs/concepts/memory.md | Documents the updated user resolution order for tools running inside an ADK agent loop. |
Suppressed comments (1)
src/adk_redis/tools/memory/delete.py:125
tool_context,_get_namespace(...), and_get_user_id(...)are called but their results are unused. Since these helpers have no side effects, these lines are effectively dead code and suggest that namespace/user scoping is applied when it is not.
tool_context = kwargs.get("tool_context")
memory_ids = args.get("memory_ids", [])
self._get_namespace(args.get("namespace"))
self._get_user_id(args.get("user_id"), tool_context=tool_context)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/adk_redis/tools/memory/prompt.py:132
- MemoryPromptTool now resolves the user via
tool_context, but there are no tests covering this path (unlike create/search/update/delete). Adding a prompt-tool test that asserts the generated request is scoped totool_context.user_idwould prevent regressions in multi-user deployments.
args = kwargs.get("args", kwargs)
tool_context = kwargs.get("tool_context")
query = args.get("query")
system_prompt = args.get("system_prompt", "")
namespace = self._get_namespace(args.get("namespace"))
user_id = self._get_user_id(args.get("user_id"), tool_context=tool_context)
src/adk_redis/tools/memory/delete.py:140
- DeleteMemoryTool validates scope by fetching each memory one-by-one before deleting. For large
memory_idslists this adds N extra round-trips (and is sequential), which can significantly increase latency and load. Consider using a backend bulk lookup, or parallelizing the preflight checks with bounded concurrency if per-id validation is required.
if self._config.backend == OPENSOURCE_AGENT_MEMORY_BACKEND:
client = self._get_agent_memory_server_client()
for memory_id in memory_ids:
memory = await client.get_long_term_memory(memory_id=memory_id)
self._require_memory_scope(
memory,
namespace=namespace,
user_id=user_id,
)
response = await client.delete_long_term_memories(memory_ids=memory_ids)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/adk_redis/tools/memory/delete.py:151
memory_idsis assumed to be a list of non-empty strings, but tool args can arrive with the wrong type (for example a single string). In that case, the current code will treat the string as an iterable and issue backend calls for individual characters, leading to confusing failures or unintended behavior. Validate the input type and elements and return a clear error before proceeding.
memory_ids = args.get("memory_ids", [])
namespace = self._get_namespace(args.get("namespace"))
user_id = self._get_user_id(args.get("user_id"), tool_context=tool_context)
if not memory_ids:
Closes #23
Memory tools now resolve the acting user in four steps: explicit user_id argument, then the invocation user from the ADK tool_context (its public user_id property), then config.default_owner_id, then config.default_user_id.
ADK already passes tool_context into run_async; the tools previously ignored it, which forced per-turn tool/agent/runner construction in multi-user workers just to scope memory via config.default_user_id. With this change a process-wide Agent and Runner can serve all users with correct owner scoping.
Implementation notes:
make check passes fully (format, lint, mypy, 119 passed / 12 skipped).