hard: tenant isolation, RPC timeouts, non-blocking async + tests#2
Merged
Merged
Conversation
The published ADK memory service had three correctness/robustness bugs on the outward-facing combo surface: 1. Cross-tenant LEAK: search_memory stuffed app_name/user_id into the recall query string, but Mimir OR's query terms -> other users'/apps' memories were returned. Now the query is clean and results are post-filtered to the requesting (app_name, user_id), recorded in each body (add_memory now stores them too). Guarantees isolation regardless of recall ranking semantics. 2. No RPC timeout: _rpc did a bare blocking stdout.readline() under a lock -> a hung mimir subprocess deadlocked every memory op forever. Now a daemon reader thread + a configurable timeout_s (default 30s) with request/response id correlation (skips notifications / stale ids). 3. Event-loop blocking: async methods did blocking subprocess I/O inline. They now run it via asyncio.to_thread, so ADK's loop isn't stalled. Also: send notifications/initialized after handshake (MCP spec); stderr=DEVNULL (un-drained PIPE deadlock); UTC timestamps; declare the typing-extensions dep (override backport for 3.10/3.11); add py.typed; fix README/docstring examples that don't run on ADK 2.3.0 (Agent has no memory_service -> Runner; run_async has no agent arg). Adds a test suite (fake Mimir MCP stdio stub) + cross-platform CI: isolation, timeout, id-correlation, handshake. Bump 0.2.0 -> 0.3.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the outward-facing ADK memory package — the "go-to combo" surface that ADK users actually
pip install. Three correctness/robustness bugs + packaging/doc fixes, with the package's first test suite and CI.🔴 HIGH — fixed
search_memoryscoped results by stuffingapp_name/user_idinto the recall query string — but Mimir OR's query terms together, so a search returned memories belonging to other users and apps (and diluted relevance). The query is now sent clean, and every returned item is post-filtered to the requesting(app_name, user_id)recorded in its body.add_memorynow also recordsapp_name/user_idso explicit memories filter correctly. Isolation no longer depends on recall ranking semantics._rpcdid a bare blockingstdout.readline()under a lock — a hungmimirsubprocess blocked every memory op forever. Now a daemon reader thread pumps stdout into a queue and_rpcwaits against a configurabletimeout_s(default 30s), with request/response id correlation (skips notifications and stale/other-id replies).asyncmethods did blocking subprocess I/O inline, stalling ADK's event loop on every memory call. They now run the blocking RPC viaasyncio.to_thread(the existing lock correctly serializes the worker threads).🟡 Also
notifications/initializedafter the handshake (MCP spec compliance).stderr=subprocess.DEVNULL— the un-drainedPIPEcould deadlock a chatty server once its pipe buffer filled._format_timestampwas naive local-tz).typing-extensionsdependency — the service importstyping_extensions.override(the stdlibtyping.overrideonly exists on 3.12+) but it was undeclared.py.typed(PEP 561) — the package ships type hints but exported none.Agenthas nomemory_servicefield (it goes onRunner);run_asynchas noagentparameter (the agent is bound on theRunner). Verified against the installedgoogle-adk 2.3.0.Tests + CI
First test suite for the package (previously zero): a fake Mimir MCP stdio stub that models recall OR-semantics, covering tenant isolation, RPC timeout, id-correlation / notification skipping, and the handshake. New cross-platform
test.yml(Linux/Windows/macOS × py3.10/3.12). All 4 tests pass locally; wheel builds withpy.typedincluded. Version 0.2.0 → 0.3.0 (a PyPI republish is a maintainer gate).🤖 Generated with Claude Code