feat(memory): expose platform memory as an agent tool - #319
Conversation
github-actions
Bot
added
the
missing-template
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (10)
WalkthroughThe gateway adds the ChangesMemory tooling and composite dispatch
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
removed
the
missing-template
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gateway/api/routes/_pipeline.py`:
- Around line 925-933: Update the memory setup flow around
`_resolve_platform_memory` and `MemoryBackend` so the resolved `scope` and
`recall_limit` values are forwarded when constructing the backend or issuing
memory requests. Preserve the existing `enabled` permission check and ensure
workspace policy limits are not silently discarded.
- Around line 921-923: Remove the duplicate platform_base pre-check from the
surrounding pipeline flow and rely on _resolve_platform_memory’s existing
validation, preserving the established 500 response for missing
platform.base_url. If the detail is still referenced locally, replace the
repeated "Hybrid mode is misconfigured" literal with a named *_DETAIL constant
following the file’s existing convention.
In `@src/gateway/services/memory_backend.py`:
- Around line 77-79: Update MemoryBackend.__aenter__ in
src/gateway/services/memory_backend.py:77-79 to perform a lightweight
connectivity probe after creating the AsyncClient, so memory-only requests fail
before streaming commits. Keep the eager-open behavior and claims in
src/gateway/api/routes/_pipeline.py:1212-1221 and
src/gateway/api/routes/_pipeline.py:1582-1591 unchanged because they are
corrected by this root-cause fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0eb6877-ac3c-4b9a-8fb5-d705ab618dd9
📒 Files selected for processing (9)
src/gateway/api/routes/_pipeline.pysrc/gateway/api/routes/_platform.pysrc/gateway/api/routes/_tools.pysrc/gateway/services/composite_backend.pysrc/gateway/services/memory_backend.pytests/unit/test_composite_backend.pytests/unit/test_memory_backend.pytests/unit/test_memory_resolve.pytests/unit/test_pipeline_settlement.py
| platform_base = ctx.config.platform.get("base_url") | ||
| if not platform_base: | ||
| raise adapter.error(502, "Hybrid mode is misconfigured", ErrorKind.API) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Status code diverges from the established "misconfigured hybrid mode" convention.
This pre-check raises 502 for a missing platform.base_url, but every other path that hits this exact condition (_resolve_platform_web_search, and _resolve_platform_memory itself if this check were removed) raises 500 with the same "Hybrid mode is misconfigured" detail. Since _resolve_platform_memory already guards this internally, this duplicate check just gives the same failure a different status code depending on which tool triggered it — a minor but confusing inconsistency for anyone debugging via status codes. Also worth promoting the literal string to a named *_DETAIL constant to match the file's convention (MEMORY_HYBRID_ONLY_DETAIL, MEMORY_NOT_ENABLED_DETAIL).
🔧 Suggested fix
- assert ctx.user_token is not None # guaranteed by the hybrid-mode preamble
- platform_base = ctx.config.platform.get("base_url")
- if not platform_base:
- raise adapter.error(502, "Hybrid mode is misconfigured", ErrorKind.API)
- # Platform owns per-workspace memory enablement: 403 when the workspace has it off.
- memory_policy = await _resolve_platform_memory(config=ctx.config, user_token=ctx.user_token)
+ assert ctx.user_token is not None # guaranteed by the hybrid-mode preamble
+ # Platform owns per-workspace memory enablement: 403 when the workspace has it off.
+ # `_resolve_platform_memory` itself raises 500 "Hybrid mode is misconfigured" if
+ # base_url is missing, matching every other resolve call's convention.
+ memory_policy = await _resolve_platform_memory(config=ctx.config, user_token=ctx.user_token)
+ platform_base = ctx.config.platform["base_url"] # guaranteed non-empty by the resolve call above📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| platform_base = ctx.config.platform.get("base_url") | |
| if not platform_base: | |
| raise adapter.error(502, "Hybrid mode is misconfigured", ErrorKind.API) | |
| assert ctx.user_token is not None # guaranteed by the hybrid-mode preamble | |
| # Platform owns per-workspace memory enablement: 403 when the workspace has it off. | |
| # `_resolve_platform_memory` itself raises 500 "Hybrid mode is misconfigured" if | |
| # base_url is missing, matching every other resolve call's convention. | |
| memory_policy = await _resolve_platform_memory(config=ctx.config, user_token=ctx.user_token) | |
| platform_base = ctx.config.platform["base_url"] # guaranteed non-empty by the resolve call above |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gateway/api/routes/_pipeline.py` around lines 921 - 923, Remove the
duplicate platform_base pre-check from the surrounding pipeline flow and rely on
_resolve_platform_memory’s existing validation, preserving the established 500
response for missing platform.base_url. If the detail is still referenced
locally, replace the repeated "Hybrid mode is misconfigured" literal with a
named *_DETAIL constant following the file’s existing convention.
| memory_policy = await _resolve_platform_memory(config=ctx.config, user_token=ctx.user_token) | ||
| if not memory_policy.get("enabled"): | ||
| raise adapter.error(403, MEMORY_NOT_ENABLED_DETAIL, ErrorKind.PERMISSION) | ||
| use_memory = True | ||
| memory_url = f"{platform_base.rstrip('/')}/gateway/memory" | ||
| # Memory needs both tokens (dual-auth): the gateway token proves machine identity, | ||
| # the user token resolves the caller's workspace + member. Both go to the platform. | ||
| memory_gateway_token = ctx.config.platform_token | ||
| memory_user_token = ctx.user_token |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
mem0 workspace scope recall_limit memory policy enforcement server-side
💡 Result:
Mem0 manages memory workspace scoping and policy enforcement primarily through entity-based partitioning and server-side authentication [1][2][3]. WORKSPACE SCOPE AND IDENTIFIERS Mem0 separates memories into four dimensions to prevent cross-contamination and ensure data isolation [2]: * user_id: Persistent persona or account. * agent_id: Distinct agent or tool. * app_id: Application or product surface. * run_id: Session-scoped or short-lived flow [4][2]. These identifiers are used as filters during memory operations [1][2]. A critical constraint in the current architecture is that entity scopes are stored separately; consequently, using an AND filter to combine multiple entity IDs (e.g., user_id AND agent_id) will return empty results [1][4][5]. Developers should use OR logic or separate queries to retrieve data across scopes [1][2][5]. RECALL LIMITS AND THRESHOLDS Recall behavior is managed via SDK parameters passed during memory operations [6][5][7]: * top_k: Controls the maximum number of memories returned per recall or search [6][7]. * threshold (or searchThreshold): Sets the minimum similarity score (typically 0–1) required for a memory to be included in results [6][5][7]. The default is often 0.1, which can be increased for stricter relevance [5]. SERVER-SIDE POLICY ENFORCEMENT The Mem0 self-hosted server enforces security and policy boundaries through the following mechanisms: * Authentication: Auth is enabled by default, requiring API keys or JWTs for access [3][8]. The server includes security defaults to protect against unauthorized access [9][8]. * Ownership Checks: Recent hardening efforts have introduced ownership verification on server endpoints [10][9]. For example, destructive operations (e.g., DELETE /memories, DELETE /entities) and unfiltered read operations (e.g., GET /memories without filters) now require administrative roles or explicit ownership checks (validating that the API key belongs to the owner of the memory) [10][9]. * Governance Layers: While core Mem0 remains lean, community patterns and external integrations (such as TealTiger) exist to implement pre-write PII detection, scope enforcement, and audit logging as a governance layer that wraps memory operations [11]. Configuration of settings such as default scopes (e.g., global vs. project) is persisted locally in client-side configuration files (e.g., ~/.mem0/settings.json) or handled as runtime overrides in the server's database [12][3][8].
Citations:
- 1: https://docs.mem0.ai/platform/features/entity-scoped-memory
- 2: https://github.com/mem0ai/mem0/blob/HEAD/skills/mem0/references/architecture.md
- 3: https://github.com/mem0ai/mem0/tree/main/server
- 4: https://github.com/mem0ai/mem0/blob/HEAD/skills/mem0/references/api-reference.md
- 5: https://github.com/mem0ai/mem0/blob/HEAD/skills/mem0/references/sdk-guide.md
- 6: https://github.com/mem0ai/mem0/blob/v1.0.10/docs/core-concepts/memory-operations/search.mdx
- 7: https://docs.mem0.ai/integrations/openclaw
- 8: https://github.com/mem0ai/mem0/blob/3e6ab394/server/README.md
- 9: fix(server): harden self-hosted server — pgvector upgrade, admin auth, endpoint security mem0ai/mem0#5360
- 10: fix(server): add ownership checks to memory and entity endpoints mem0ai/mem0#5384
- 11: TealTiger governance layer for memory operations (PII detection, access control, audit) mem0ai/mem0#5507
- 12: https://docs.mem0.ai/integrations/opencode
scope and recall_limit are resolved but not forwarded. _resolve_platform_memory returns them, but this block only consumes enabled and then builds MemoryBackend without either value. If the platform is meant to enforce those limits server-side, this is fine; otherwise the gateway is silently ignoring workspace policy here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gateway/api/routes/_pipeline.py` around lines 925 - 933, Update the
memory setup flow around `_resolve_platform_memory` and `MemoryBackend` so the
resolved `scope` and `recall_limit` values are forwarded when constructing the
backend or issuing memory requests. Preserve the existing `enabled` permission
check and ensure workspace policy limits are not silently discarded.
| async def __aenter__(self) -> MemoryBackend: | ||
| self._client = await self._stack.enter_async_context(httpx.AsyncClient(timeout=self._timeout_s)) | ||
| return self |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== memory_backend outline ==\n'
ast-grep outline src/gateway/services/memory_backend.py --view expanded || true
printf '\n== pipeline outline (targeted symbols if any) ==\n'
ast-grep outline src/gateway/api/routes/_pipeline.py --view expanded || true
printf '\n== search for memory reachability / errors ==\n'
rg -n "MemoryNotReachable|NotReachable|memory.*health|health.*memory|memory.*reach|reach.*memory|__aenter__|enter_async_context\\(CompositeToolBackend|open_stream|run_streaming_with_fallback" src/gateway -SRepository: mozilla-ai/otari
Length of output: 12282
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== memory_backend excerpt ==\n'
nl -ba src/gateway/services/memory_backend.py | sed -n '1,220p'
printf '\n== pipeline excerpt 1 ==\n'
nl -ba src/gateway/api/routes/_pipeline.py | sed -n '1188,1240p'
printf '\n== pipeline excerpt 2 ==\n'
nl -ba src/gateway/api/routes/_pipeline.py | sed -n '1560,1610p'
printf '\n== memory-specific symbols ==\n'
rg -n "MemoryBackend|MemoryNotReachable|memory backend|memory.*reach|reach.*memory|health" src/gateway -SRepository: mozilla-ai/otari
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [
"src/gateway/services/memory_backend.py",
"src/gateway/api/routes/_pipeline.py",
]
for path in paths:
print(f"\n== {path} ==")
text = Path(path).read_text()
lines = text.splitlines()
if path.endswith("memory_backend.py"):
for start, end in [(1, 120), (170, 230)]:
print(f"\n-- lines {start}-{end} --")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
else:
for start, end in [(1190, 1230), (1578, 1600)]:
print(f"\n-- lines {start}-{end} --")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: mozilla-ai/otari
Length of output: 11813
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
print("== gateway/memory route references ==")
for path in Path("src").rglob("*"):
if path.is_file():
try:
text = path.read_text()
except Exception:
continue
if "/gateway/memory" in text or "memory_search" in text or "memory_store" in text or "memory_forget" in text:
print(path)
for needle in ["/gateway/memory", "memory_search", "memory_store", "memory_forget", "health"]:
if needle in text:
print(f" contains {needle!r}")
PYRepository: mozilla-ai/otari
Length of output: 470
Memory-only streams still bypass fail-fast entry
MemoryBackend.__aenter__ only creates the client, so a dead memory backend is still discovered on the first tool call after streaming may already have committed. If the eager-open path is meant to cover memory-only requests, add a lightweight probe here; otherwise trim the memory-only pre-200 claim in _pipeline.py.
📍 Affects 2 files
src/gateway/services/memory_backend.py#L77-L79(this comment)src/gateway/api/routes/_pipeline.py#L1212-L1221src/gateway/api/routes/_pipeline.py#L1582-L1591
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/gateway/services/memory_backend.py` around lines 77 - 79, Update
MemoryBackend.__aenter__ in src/gateway/services/memory_backend.py:77-79 to
perform a lightweight connectivity probe after creating the AsyncClient, so
memory-only requests fail before streaming commits. Keep the eager-open behavior
and claims in src/gateway/api/routes/_pipeline.py:1212-1221 and
src/gateway/api/routes/_pipeline.py:1582-1591 unchanged because they are
corrected by this root-cause fix.
c8a7107 to
efb6e45
Compare
GitHub Actions
Failure
|
Note: this comment was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's. @dpoulopoulos a design question on this one before we go further, not a blocker on the implementation quality (the wiring and tests look solid). Right now The thing I keep coming back to: functionally this is an MCP server. Three tools, remote dispatch, auth headers. And the gateway already knows how to advertise MCP tools, run the loop, and forward calls to a remote server generically via So the question: why does any memory code need to live in otari at all, rather than memory being an MCP server hosted by otari-ai? If it were an MCP server, The one piece I would keep is Is there a constraint I am missing that makes the built-in-tool approach necessary? The two I can think of are (a) the ergonomic opt-in, which the flag preserves, and (b) automatic non-tool recall injection, which is a separate feature and could live on the platform side of the per-request resolve anyway. If neither is load-bearing, exposing memory as a platform MCP server seems like it keeps the OSS gateway free of a feature only one hosted backend can serve. Curious where you land. |
35dc974 to
28e46a8
Compare
GitHub Actions
Inactive
28e46a8 to
4ef0599
Compare
GitHub Actions
Inactive
Add a gateway-native memory tool (memory_search / memory_store / memory_forget) that the
model can call, backed by the platform's dual-auth /gateway/memory/{search,store,forget}
endpoints. Mirrors the web-search backend: Tool.MEMORY (otari_memory), a MemoryBackend
implementing the tool-loop protocol, _resolve_platform_memory for enablement, and
prepare_gateway_tools wiring (hybrid-only; 403 when disabled).
Introduce CompositeToolBackend so memory can run alongside any other gateway tool (MCP,
web search, code execution) in a single tool loop: dispatch composes the active backends
into one pool, preserving lazy-MCP-open on the streaming path and eager open elsewhere.
The tool loop itself is unchanged.
Signed-off-by: Dimitris Poulopoulos <dimitris.a.poulopoulos@gmail.com>
4ef0599 to
0674e94
Compare
GitHub Actions
Inactive
2 participants
Description
Exposes platform memory as a gateway-managed agent tool:
Tool.MEMORY("otari_memory") alongside the existingotari_web_search/otari_code_executiontypes.MemoryBackend(src/gateway/services/memory_backend.py), which exposesmemory_search/memory_store/memory_forgetas model-callable tools and dispatches them to the platform's/gateway/memory/{search,store,forget}endpoints (dual-auth: gateway token + user token).CompositeToolBackend(src/gateway/services/composite_backend.py): memory is the one gateway tool designed to run alongside whichever other backend is active (MCP, web search, or sandbox) rather than being mutually exclusive with them, so this fans out the tool-loop protocol (openai_tools,owns_tool,purpose_hints,call_tool) across an ordered list of backends._extract_memory_tool/_resolve_platform_memoryinto_pipeline.py's shared tool-context builder, used by all three request formats (chat completions, Anthropic messages, Responses API) —otari_memoryis hybrid-mode only (400 if standalone) and gated on the workspace's platform-side memory config (403 if not enabled there).PR Type
Relevant issues
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).Notes on the checklist:
uv run ruff check src tests scripts(clean),uv run mypy(clean, 199 files), anduv run pytest tests/unit(615 passed).tests/integrationrequires Testcontainers/Docker Postgres and was not run in this environment.uv run python scripts/generate_openapi.py --checkconfirms the committed spec is already up to date (the tool-type string doesn't change the request schema).AGENTS.mdstill only documentsotari_code_execution/otari_web_searchand hasn't been updated to mentionotari_memory— flagging as a known gap rather than silently checking the docs box.AI Usage
AI Model/Tool used: Claude Code (Claude Sonnet 5)
Any additional AI details you'd like to share:
This PR (and the companion platform-side PR mozilla-ai/otari-ai#1344) were authored by Claude Code across a multi-turn session implementing and wiring up the workspace memory feature.
Summary
This change adds a new platform-managed “memory” capability exposed as the
otari_memorytool. It lets the gateway perform three actions—searching saved memories, storing new memories, and deleting memories—while enforcing that memory works only in hybrid mode and only when workspace policy allows it. The gateway now coordinates this memory tool alongside existing tool sources (MCP, web search, and sandbox) during both chat and Responses-style requests, including correct behavior for streaming tool execution.It also introduces a shared “composite” tool coordinator so the gateway can route tool calls through the right backend in a consistent way, and improves dispatch/error handling (especially for streaming and hybrid fallbacks). Unit tests were added for the composite coordinator and memory components, and integration tests were updated to assert the new composite pooling behavior.
AGENTS.mdstill needs updating forotari_memory.Technical notes