Skip to content

fix(memory): scope memory_store/memory_recall per channel sender - #1290

Open
guzzi235 wants to merge 1 commit into
RightNow-AI:mainfrom
guzzi235:fix/memory-scoped-per-sender
Open

guzzi235 wants to merge 1 commit into
RightNow-AI:mainfrom
guzzi235:fix/memory-scoped-per-sender

Conversation

@guzzi235

Copy link
Copy Markdown

Summary

memory_store/memory_recall both write to a single fixed shared_memory_agent_id() namespace — documented in-code as intentional ("so all agents read/write to the same namespace"), but in practice this means any end user messaging any agent through any channel can read and overwrite every other user's memory entries. There's no isolation beyond whatever key text the model happens to pick (e.g. self.* vs shared.* is a naming convention, not an enforced boundary).

Concretely: person A messaging the bot from a Nextcloud Talk / Telegram / Discord conversation, and person B in a completely unrelated conversation, currently share one flat KV store.

Fix

Threads a sender_id (the real channel actor's identity, e.g. Nextcloud Talk's actor_id) from the channel bridge down through the agent loop into the tool-execution layer:

  • New ChannelBridgeHandle::send_message_from(agent_id, message, sender_id) — default implementation falls back to the existing send_message(), so this is non-breaking for any other implementer of the trait.
  • OpenFangKernel::send_message_from(...) mirrors send_message(...) but resolves the handle the same way and threads sender_id through to execute_llm_agent/streaming.
  • execute_tool(...) gains a caller_sender_id: Option<&str> parameter.
  • tool_memory_store/tool_memory_recall now prefix keys as user:<sender_id>:<key> whenever a sender identity is available, via a small scoped_memory_key() helper. Dashboard/API/cron callers (already owner-trusted, no channel sender) keep the previous unprefixed behavior.
  • Updated the one other execute_tool call site (routes.rs, MCP-over-HTTP) to pass None for the new parameter.

Test plan

  • Verified end-to-end against a real Nextcloud Talk instance: a memory_store call triggered from a chat message now shows up in GET /api/memory/agents/:id/kv as user:<actor_id>:<key> instead of the bare key.
  • Verified the direct dashboard/API message path (no channel sender identity) is unaffected and still stores/recalls under the plain key.
  • cargo build --release succeeds with no new warnings.

🤖 Generated with Claude Code

memory_store and memory_recall both write to a single fixed
`shared_memory_agent_id()` namespace, shared by every agent, every
channel, and every end user of the whole installation. In practice
this means: person A messaging the bot via a Telegram DM, person B
in an unrelated Discord server, and a completely different agent can
all read and overwrite the same memory keys. There is no isolation
at all beyond whatever key text the LLM happens to choose.

This adds a `sender_id` plumbed from the channel bridge down through
the agent loop into the tool-execution layer, and a new
`ChannelBridgeHandle::send_message_from()` (default falls back to the
existing `send_message()`, so this is non-breaking for any other
implementer of the trait) that carries the real channel actor's
identity. `tool_memory_store`/`tool_memory_recall` now prefix keys as
`user:<sender_id>:<key>` whenever a sender identity is available,
leaving dashboard/API/cron callers (which are already owner-trusted
and have no channel sender) on the previous unprefixed behavior.

## Test plan
- Verified end-to-end against a real Nextcloud Talk instance: a
  memory_store call from a message now shows up in
  `GET /api/memory/agents/:id/kv` as `user:<actor_id>:<key>` instead
  of the bare key.
- Verified the direct dashboard/API message path (no sender identity)
  is unaffected and still stores/recalls under the plain key.
- `cargo build --release` succeeds with no new warnings.

🤖 Generated with [Claude Code](https://claude.com/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.

1 participant