Skip to content

fix(memory): pass per-turn user_id through model tool calls + compat fallback - #45

Closed
dizhaky wants to merge 1 commit into
mainfrom
fix/codex-tool-call-user-scoping
Closed

fix(memory): pass per-turn user_id through model tool calls + compat fallback#45
dizhaky wants to merge 1 commit into
mainfrom
fix/codex-tool-call-user-scoping

Conversation

@dizhaky

@dizhaky dizhaky commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Cross-user tool call scoping (tool_executor.py, memgw, mem0): Model-invoked memory tools (memgw_recall, mem0_search, etc.) were called without user_id, so in shared-thread gateway sessions the provider used the stale _user_id from initialize() (the first user in the session). This could leak one user's memories to another. Fix: pass user_id=agent._user_id at the tool_executor dispatch site (already refreshed per-turn since gateway/run.py:16368) and thread it through to memgw.handle_tool_call and mem0.handle_tool_call.
  • Backward compat for old external providers (memory_manager.py): Old third-party providers that override sync_turn/prefetch/queue_prefetch without the user_id kwarg raised TypeError, which was swallowed by the broad except Exception — silently breaking sync/prefetch. Fix: add an explicit except TypeError fallback that retries the call without user_id, keeping old plugins functional.

Closes remaining Codex P2 findings from PR #30 review and PR #33 review.

Test plan

  • In a shared gateway thread session, verify memgw_recall and mem0_search use the current turn's user_id, not the first user's
  • Verify a third-party plugin without user_id kwarg still syncs/prefetches correctly (TypeError fallback)
  • Regression: single-user CLI sessions are unaffected (empty user_id falls through to self._user_id)

🤖 Generated with Claude Code

https://claude.ai/code/session_01QKeVgEJBrwJQSH2BSXA4oL


Generated by Claude Code


Note

Medium Risk
Touches memory isolation in multi-user gateway sessions (security-sensitive); changes are narrow and fall back to prior behavior when user_id is empty or providers lack the kwarg.

Overview
Fixes cross-user memory leakage in shared gateway sessions by passing the current turn’s agent._user_id into MemoryManager.handle_tool_call from the sequential tool executor, and honoring it in mem0 and memgw tool handlers (recall/search/reflect/retain/conclude) instead of only the initialize()-time user.

Adds TypeError fallbacks in MemoryManager for prefetch, queue_prefetch, and sync_turn: providers that don’t accept user_id are retried without it so older third-party plugins keep working instead of failing silently.

Reviewed by Cursor Bugbot for commit c2cb8b248510f3f3cc3b7a41c9e43a73ffe735da. Configure here.

@github-actions

Copy link
Copy Markdown

🔎 Lint report: fix/codex-tool-call-user-scoping vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8649 on HEAD, 8649 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4570 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

…fallback

Addresses two remaining gaps flagged in Codex reviews on PRs #30 and #33.

**P2 — model-facing tool calls used stale init-time user_id**
`tool_executor.py` called `handle_tool_call` without `user_id`, so when
the model invoked memgw_recall/retain/reflect or mem0_search/profile/conclude
in a shared gateway session, both providers fell back to the user_id captured
at `initialize()` time (i.e. the first user's id). In shared-thread sessions
(`thread_sessions_per_user=False`) this meant one user's tool calls could read
or write another user's memory scope.

Fix: pass `user_id=agent._user_id` at the `tool_executor` call site
(already refreshed per-turn by `gateway/run.py:16368`); thread it through
`memory_manager.handle_tool_call` → provider, then use it in both
`memgw.handle_tool_call` (via `_user_scope(call_user_id)`) and
`mem0.handle_tool_call` (as per-call `read_filters` / `write_filters`).

**P2 — external providers without user_id kwarg raised TypeError silently**
Old third-party providers that override `sync_turn`/`prefetch`/`queue_prefetch`
without the `user_id` keyword arg raised `TypeError`, which was caught by the
broad `except Exception` and logged — causing silent sync/prefetch failures.

Fix: add a specific `except TypeError` in `memory_manager.sync_all`,
`prefetch_all`, and `queue_prefetch_all` that retries the call without the
`user_id` kwarg, keeping old plugins functional while new ones get full
per-user scoping.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QKeVgEJBrwJQSH2BSXA4oL
@dizhaky
dizhaky force-pushed the fix/codex-tool-call-user-scoping branch from c2cb8b2 to f24945a Compare June 28, 2026 09:16

dizhaky commented Jun 28, 2026

Copy link
Copy Markdown
Owner Author

Claude Code automated review — PR #45 (Codex usage-limit bypass)

Codex hit its usage cap before reviewing this PR. Running a manual pass to cover the gap.


Summary of changes

File Change Assessment
agent/memory_manager.py TypeError fallback in prefetch_all, queue_prefetch_all, sync_all ✅ Correct
agent/tool_executor.py Pass user_id=agent._user_id into handle_tool_call ✅ Correct
plugins/memory/mem0/__init__.py Derive call_user_id per-call; use it for all three tools ✅ Correct
plugins/memory/memgw/__init__.py call_user_id from kwargs → _user_scope(call_user_id) + _retain(..., user_id=...) ✅ Correct

Detailed findings

TypeError fallbacks (Codex PR#33 P2) — correctly scoped. The fallback catches TypeError before the existing Exception handler, retries without user_id, and only logs on secondary failure. This keeps older provider plugins functional without silently swallowing the retry.

tool_executor.py dispatchgetattr(agent, "_user_id", "") or "" is properly defensive for agent instances that predate the _user_id attribute. Combined with the gateway already refreshing agent._user_id per turn (PR #33 follow-up), this ensures the per-turn user reaches the provider.

mem0 per-call filterscall_user_id = kwargs.get("user_id", "") or self._user_id correctly falls back to the initialized user in single-user (CLI) sessions. All three handled tools (mem0_profile, mem0_search, mem0_conclude) use the new read_filters/write_filters. This closes the Codex P1: mem0_conclude was still writing via _write_filters()self._user_id as flagged in the 2026-06-28 05:13 comment on PR #33.

memgw per-call scope_user_scope(call_user_id) already falls back via uid = user_id or self._user_id internally (line 365 of __init__.py), so empty-string CLI calls are handled identically to mem0. All three tool branches (memgw_recall, memgw_retain, memgw_reflect) use the new per-call scope.


Lint / type check

CI passed cleanly — 0 new ruff issues, 0 new ty errors. No changes to public interfaces.


Verdict

This is a narrow, targeted fix with a safe compat fallback. All Codex P1 findings from the PR #30#33 chain are now closed by this PR. No concerns. Approve.


Automated review by Claude Code — substituting for Codex (usage limit reached).


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants