feat(memory): add per-user scoping to holographic memory plugin - #10427
nericervin wants to merge 3 commits into
Conversation
The holographic memory plugin stores all facts in a single global table without any user isolation. When multiple users share a Hermes instance via the API server gateway, every user's facts are visible to every other user — a data leak. This commit adds a `user_scope` column to the facts table and threads the gateway `user_id` (from the `X-Hermes-User-Id` header) through `AIAgent → HolographicMemoryProvider → MemoryStore → FactRetriever`. Changes: - store.py: `user_scope` column with composite unique index on (content, user_scope), auto-migration for existing databases, `scope_clause()` helper for consistent WHERE filtering - __init__.py: extract `user_id` from kwargs in `initialize()`, pass as `user_scope` to MemoryStore; scope `system_prompt_block` count query - retrieval.py: apply `scope_clause()` to all retrieval paths (FTS candidates, probe, related, reason, contradict, vector scoring) - api_server.py: read `X-Hermes-User-Id` header, pass through `_run_agent` → `_create_agent` → `AIAgent(user_id=...)` for all three endpoints (chat completions, responses, streaming) Backwards-compatible: when `user_id` is None (CLI sessions), all facts remain visible — no behaviour change for single-user setups. Follows up on NousResearch#5895 which threaded `user_id` to Mem0 and Honcho but left Holographic unscoped.
The previous commit added user_id to _run_agent and the handler calls but missed adding it to _create_agent's signature and to the AIAgent constructor call. This caused: "unexpected keyword argument 'user_id'" at runtime.
|
Replaces closed #7256 (rebased on current main to resolve conflicts, per PR description). |
|
Hi @alt-glitch — thanks for confirming. Pushed a small follow-up commit to add my email to Re: the failing |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling a real isolation gap. Current main already forwards AIAgent._user_id to memory providers (agent/agent_init.py:1384-1410), but API-server agents still do not receive one (gateway/platforms/api_server.py:1234-1363), so the core premise remains valid.
Problems
- The existing-store migration is unsafe: current databases retain
content TEXT NOT NULL UNIQUE(plugins/memory/holographic/store.py:19). Addinguser_scopeand a composite index does not remove that table constraint, so duplicate content for a second user still fails. update_fact,remove_fact, andrecord_feedbackremain fact-id-only (plugins/memory/holographic/store.py:262-325,370-390), so the new read filters do not enforce scope for mutations.- Current main validates caller-provided memory scope in
_parse_session_key_header(gateway/platforms/api_server.py:1088-1135). The new header needs equivalent validation and must also cover the newer persisted-session chat routes (gateway/platforms/api_server.py:1876-1995).
Suggested changes
- Rebuild/migrate legacy tables to replace the old UNIQUE constraint; add an upgrade regression test.
- Apply scope checks to every mutation and add cross-user mutation-denial coverage.
- Generalize the current validated scope-header path and cover every API agent entry point.
Automated hermes-sweeper review.
| # memory plugins (holographic, mem0, honcho) isolate facts per | ||
| # user. When absent, all facts are visible (CLI behaviour). | ||
| provided_user_id = request.headers.get( | ||
| "X-Hermes-User-Id", "" |
There was a problem hiding this comment.
Please route this through a validated scope-header helper rather than accepting it directly. Current main's analogous X-Hermes-Session-Key parser requires API-key configuration and rejects unsafe or oversized values (gateway/platforms/api_server.py:1088-1135); this identifier becomes persistent tenant state.
| self._conn.execute( | ||
| "CREATE UNIQUE INDEX IF NOT EXISTS idx_facts_content_user " | ||
| "ON facts(content, COALESCE(user_scope, ''))" | ||
| ) |
There was a problem hiding this comment.
This does not replace the legacy table-level UNIQUE(content) constraint on existing databases; SQLite retains that constraint after ALTER TABLE ... ADD COLUMN. Rebuild/copy the table before relying on scoped duplicate inserts, and add an upgrade test with identical content under two scopes.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the reported cross-user leakage in holographic memory by propagating an API-provided user ID into the memory store and filtering retrievals by that scope. #10427 is the rebased replacement for #7256, but its diff does not yet enforce isolation across legacy-schema migrations, mutations, header validation, and all persisted-session routes.
Related pull requests
- #7256 [closed]
duplicate— (+375/-30) — superseded duplicate: The holographic-memory portion adds the same user-scope column, API-header propagation, and scoped reads later carried by #10427, but this closed PR also bundles unrelated Mattermost, WhatsApp, lifecycle-status, and messaging-tool changes. It remains relevant as the original implementation, but was explicitly closed in favor of the rebased #10427. - #10427
related— (+148/-19) — keep open with a salvage path: The diff isolates many read and retrieval paths and forwards X-Hermes-User-Id through the API agent path, directly addressing cross-user reads. Consistent with the keep_open review on #10427, it still needs a table-rebuild migration that removes the legacy content UNIQUE constraint, scope checks on update_fact/remove_fact/record_feedback, validated caller scope equivalent to _parse_session_key_header, and coverage of the newer persisted-session chat routes.
Duplicates
#7256 and #10427 implement substantially the same holographic-memory scoping change; #10427 explicitly replaces #7256, so #7256 should remain closed as a duplicate of #10427.
Suggested consolidation
Keep open with a salvage path for #10427: retain its focused API user-ID propagation and scoped retrieval work, then address the concrete isolation gaps identified by the contributor keep_open review—legacy-table reconstruction, mutation scoping, header validation, and persisted-session route coverage. Keep #7256 closed as a duplicate of #10427 because its relevant memory changes were superseded and its diff also contains unrelated platform-specific modifications.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup7256 ["PRs duplicating each other"]
P7256["PR #7256 (closed)"]
P10427["PR #10427 (open)"]
end
class P7256 closed
class P10427 open
class P10427 target
click P7256 "https://github.com/NousResearch/hermes-agent/pull/7256"
click P10427 "https://github.com/NousResearch/hermes-agent/pull/10427"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 51 kB of PR diffs, 3 kB of issue/PR text, 2 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Adds per-user fact scoping to the Holographic memory plugin, allowing multi-tenant deployments where each user's memories are isolated.
Replaces #7256 (rebased on current main to resolve conflicts).
Changes
plugins/memory/holographic/store.py: Adduser_scopecolumn to facts table, filter queries by user when providedplugins/memory/holographic/retrieval.py: Passuser_scopethrough retrieval pipelineplugins/memory/holographic/__init__.py: Acceptuser_idparameter in memory plugin interfacegateway/platforms/api_server.py:X-Hermes-User-Idheader for per-user memory scopinguser_idthrough_create_agentand_run_agentto the memory pluginUse case
When Hermes serves multiple users via the API server (e.g. from an Odoo Discuss integration), each user's holographic memories should be isolated. Without this, all users share the same fact store, leading to cross-contamination of context.
The
X-Hermes-User-Idheader is set by the client (e.g. Odoo webhook) to identify the user.Test plan
X-Hermes-User-Idheader is sent, behavior is unchanged (global scope)