Skip to content

KnowledgeGraph module-level singleton prevents safe multi-tenant use #1136

@cschnatz

Description

@cschnatz

Summary

mempalace.mcp_server._kg is constructed once at module import, so any deployment that wants to route the same Python process for multiple isolated palaces (multi-user hosted setups, per-user background workers, per-project process pools) can only target one SQLite file for the lifetime of the process. MEMPALACE_PALACE_PATH env var changes between calls are silently ignored by the KG (they still work for Chroma because get_collection reads env vars per-call).

Reproduction (v3.3.0)

import mempalace.mcp_server as mcp
import os

# Pretend we're a multi-tenant dispatcher:
os.environ["MEMPALACE_PALACE_PATH"] = "/tmp/tenant_a"
mcp.tool_kg_add(subject="alice_secret", predicate="owns", object="repo_a")
os.environ["MEMPALACE_PALACE_PATH"] = "/tmp/tenant_b"
facts = mcp.tool_kg_query(entity="alice_secret")
# Expected for tenant B: empty
# Actual: returns tenant A's fact — both calls hit /tmp/.mempalace or whatever path was resolved at import

Root cause (code path)

mcp_server.py around line 76:

_kg = KnowledgeGraph(db_path=os.path.join(_config.palace_path, "knowledge_graph.sqlite3"))
  • _config is a module-level MempalaceConfig() built at import
  • _config.palace_path is a property that reads env, but this read happens once at _kg construction
  • After that, _kg.db_path is frozen; env var changes do not rebind it

The rest of the library is already per-call safe:

  • _get_collection reads _config.collection_prefix as a property → ChromaDB routing follows env changes correctly
  • Only the KG singleton is constructed eagerly

Impact

For self-hosted single-user setups (the target user in README.md) this is benign — there is only one palace anyway.

For any deployment that wants to route the same process to multiple isolated palaces, the singleton forces all callers into the same SQLite file, so per-palace isolation via env vars is structurally impossible. Reviewing your own issue history it looks like multi-palace support is on the roadmap anyway; this is one of the concrete blockers.

A local workaround exists (swapping mcp._kg per call under a dispatch lock and restoring it in finally), but it monkey-patches library internals and is fragile against upstream refactors. Happy to send a PR that implements one of the upstream-friendly options below if you confirm the direction.

Suggested fix options

Option A — Lazy per-call property

Make _kg a module-level property that reads the current palace_path on access:

# mcp_server.py
_kg_by_path: dict[str, KnowledgeGraph] = {}

def _get_kg() -> KnowledgeGraph:
    path = os.path.join(_config.palace_path, "knowledge_graph.sqlite3")
    if path not in _kg_by_path:
        _kg_by_path[path] = KnowledgeGraph(db_path=path)
    return _kg_by_path[path]

And replace _kg.x(...) call sites with _get_kg().x(...). Caches per path so repeated calls stay cheap.

Pro: minimal behavior change for single-user, works for multi-palace out of the box.
Con: 5 call-site replacements in mcp_server.py. Need to audit for thread-safety of the cache dict.

Option B — Path parameter on every tool function

def tool_kg_add(..., db_path: str = None):
    kg = KnowledgeGraph(db_path=db_path) if db_path else _get_default_kg()
    kg.add_triple(...)

Pro: fully explicit, no globals.
Con: API change, every caller has to pass the path.

Option C — Thread-local _kg

Stash _kg in a threading.local() and set it at request boundaries.
Pro: backwards compatible.
Con: not reentrant across asyncio.to_thread, easier to misuse.

Preferred direction

Option A. Minimal API change, same ergonomics for single-user, correct for multi-palace. Happy to PR if you agree.

Environment

  • mempalace v3.3.0
  • Python 3.14

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/kgKnowledge graphbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions