Skip to content

feat(integrations): migrate and validate standalone Hermes Mem0 plugin - #7372

Open
kartik-mem0 wants to merge 12 commits into
mainfrom
feature/hermes-standalone-migration
Open

kartik-mem0 wants to merge 12 commits into
mainfrom
feature/hermes-standalone-migration

Conversation

@kartik-mem0

@kartik-mem0 kartik-mem0 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

N/A — this is a same-repository branch, exempt from the accepted-issue PR gate.

Description

Bring the standalone Nous Research Mem0 provider into integrations/hermes-plugin-mem0/, retaining its provider name, configuration, storage paths, tools, and three backend modes. Use the same Apache 2.0 LICENSE as OpenClaw for Mem0 contributions, preserve the imported source's MIT copyright and permission text verbatim in the third-party section of LICENSE (no separate NOTICE file), and declare both applicable licenses in package metadata. The directory is independent of shared resources and needs no build step.

Remove HANDOFF.md after reviewing it: it contains takeover instructions, not runtime code. Preserve source attribution, installation guidance, and the bundled-provider precedence warning in the README.

Fix confirmed problems in the imported code:

  • Changing an OSS embedder's dimensions fails with a useful error instead of deleting a Qdrant collection or dropping a PGVector table.
  • Setup uses Hermes' existing atomic writers with owner-only permissions for .env and OSS configuration, including database credentials.
  • Invalid tool arguments return validation errors without hitting the backend or consuming the circuit breaker budget.
  • Automatic recall respects the configured rerank preference; a self-hosted server can omit its API key when authentication is disabled.
  • Setup rotates legacy inline API keys correctly, honors the Platform --user-id flag, and excludes stored credentials from dry-run output.
  • OSS embedding requests resolve OpenAI credentials and endpoint through the active Hermes profile, failing closed when its key is missing. Distinct LLM/embedder keys are retained separately in the private OSS configuration.
  • PGVector Docker setup no longer removes an existing container when it cannot connect.
  • Resolve Ruff/isort findings and align the setup version warning with the declared minimum SDK version.

Add the integration table entry, regression tests, and a reproducible real-host smoke. Update the Hermes documentation page, its llms.txt entry, and the plugin README with standalone installation, three backend modes, existing-user migration prerequisites, configuration precedence, capture limits, and troubleshooting. No shared runtime, other plugins, or workflows change. Existing upstream capture/truncation and concurrency behavior remains outside these fixes.

Rollout: Hermes PR #114569 is merged, but Mem0 is still bundled and absent from the catalog at verification time. After this PR merges, Hermes maintainers must approve a mem0 catalog entry pointing to https://github.com/mem0ai/mem0, subdirectory integrations/hermes-plugin-mem0, pinned to a reviewed full commit SHA, and coordinate removal of the bundled copy. Existing users keep memory.provider: mem0, mem0.json, MEM0_*, user identity and database paths. This PR alone does not enable automatic migration. The plugin retains CLI setup/status support; it does not add a Desktop config panel or provider-specific CLI commands.

Migration verification: Tested plugin b183aadb6dd6c9bf4634fef14aa297eb9c427901 against Hermes main 01382698fc32ec7740b6a204d9b7a6abeac74d33. A temporary catalog entry drove the real pinned GitHub installer, scanner, dependency checks, activation, and external loader. Memories created by the actual bundled provider remained searchable after switching to the external copy; credentials, config and identity were retained, and CRUD/recall passed. Public catalog migration remains unavailable until Mem0 is admitted.

Upstream multi-profile blockers found: Hermes' migrate_all_homes() checks provider presence in the active home rather than each target home, so a second profile is skipped once the first has Mem0 installed. recover_at_startup() also suppresses further attempts by provider name across profiles in one process. Both failures reproduced with real profile discovery; installing under the second profile's explicit home scope succeeds. These host issues must be resolved or worked around before claiming seamless migration for all profiles. The scanner returned caution (7 findings), not dangerous; review is still needed at catalog admission. Model responses were simulated locally; storage and installer behavior were real.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (no functional changes)
  • Documentation update

AI Assistance

  • No AI assistance
  • AI-assisted (autocomplete, or I asked a model questions while writing this)
  • AI-generated (an agent wrote most or all of this diff)

Codex imported the upstream source, made the scoped fixes, reviewed Hermes' migration implementation, and ran the checks below. The human author has not yet attested to independent line-by-line review.

  • I can explain every line of this diff and how it interacts with the rest of the codebase, without asking an AI tool.

Breaking Changes

No configuration or tool-signature changes. An OSS collection dimension mismatch now fails initialization instead of deleting existing data; restore the previous embedder or explicitly migrate to a new collection.

Test Coverage

  • I added/updated unit tests

  • I added/updated integration tests

  • I tested manually (describe below)

  • No tests needed (explain why)

  • Six regression tests failed against the imported source before fixes. Eight further regression cases failed before the follow-up fixes. Final offline suite: 15 passed, including self-hosted HTTP request/authentication contracts, key rotation, user identity, profile credentials, dry-run secrecy, distinct model keys, and container preservation.

  • Real-host smoke passed on Hermes v0.21.3 (345cd2b057a452236de401d3534b8502a7465e8d) and main (c62bd9f2078a946108f1c9d9b24bf118963277ef) with Hermes' pinned mem0ai==2.0.10, openai==2.24.0, and local qdrant-client==1.19.1.

  • Smoke covers external discovery, real CLI setup/status, private files, all four tools, recall, background extraction, existing user identity, authenticated embedding requests under a multiplexed profile despite conflicting process credentials, shutdown, rejection of dimension changes without data loss, and persistence across restart.

  • Storage/lifecycle smoke also passed with mem0ai==2.0.20 before testing the pinned version.

  • Hermes' dependency conflict checker accepted the plugin requirements under core constraints.

  • Ruff, isort, pre-commit hooks, and git diff --check passed.

  • Licensing: verified LICENSE contains OpenClaw's Apache text and the upstream MIT notice verbatim, with no separate NOTICE file, and package TOML parses.

  • Documentation: Mintlify build validation passed under Node 22, and python3 scripts/check-llms-txt-coverage.py confirmed the docs index is in sync.

Smoke uses real Hermes, the real Mem0 SDK, a real temporary on-disk Qdrant database, and a localhost OpenAI-compatible test service. Model responses/embeddings are deterministic; no live cloud credentials, real model quality, running PostgreSQL, or production catalog migration are claimed. Optional spaCy/fastembed extras were absent. Reproduction commands are in the plugin README.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated documentation if needed

@mintlify

mintlify Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
mem0 🟢 Ready View Preview Sep 18, 2026, 11:37 AM

@github-actions github-actions Bot added documentation Improvements or additions to documentation integrations Agent and editor integrations plugin Editor and agent plugins (Claude Code, Cursor, Codex, OpenClaw, Pi Agent) labels Sep 18, 2026
Copy all upstream files at 3fc36950b2b7c19cdd81c6de99f10d2cbed850af
without modifications. Remove the prior implementation changes and tests,
restore docs, and retain only the integrations table entry.

Validation: identical Git subtree dd6a312;
all five Python files compile. Pre-commit hooks were run: upstream has
Ruff E741 and isort differences. Formatter edits were reverted to preserve
the explicitly requested byte-identical import.
@kartik-mem0 kartik-mem0 changed the title feat(integrations): migrate Hermes Mem0 plugin into standalone subdirectory feat(integrations): copy upstream Hermes Mem0 plugin unchanged Sep 18, 2026
@kartik-mem0 kartik-mem0 changed the title feat(integrations): copy upstream Hermes Mem0 plugin unchanged feat(integrations): migrate and validate standalone Hermes Mem0 plugin Sep 18, 2026
@karthik-indla

Copy link
Copy Markdown
Contributor

Gave this a thorough pass across all six areas — setup/credentials, backend, plugin core + tools, the OSS/LLM shims, licensing/docs, and tests — and verified each claimed fix against the actual code rather than the description. Short version: the import is genuinely solid, and all 15 named fixes are implemented as described. There are two things I'd fix before merge and a handful of follow-ups; details below, grouped by priority.

Worth fixing before merge

  1. The auto-provisioned pgvector container is network-exposed with a hardcoded password. The interactive OSS setup runs docker run … -e POSTGRES_PASSWORD=hermes … -p {port}:5432, which publishes on 0.0.0.0 with the world-known password hermes. On a shared network, a peer can psql -U postgres and read/modify every stored memory. Suggest binding 127.0.0.1:{port}:5432 and generating a random password per install. (This also turns the currently-LOW "password appears in the CalledProcessError string printed on failure" into a real leak once the password is per-install, so scrub that error output too.)

  2. The setup wizard crashes on a pre-release SDK version. The version check does int(x) on the installed mem0ai version, so 2.1.0rc1 (or any PEP 440 pre-release) raises ValueError, which isn't caught by the surrounding suppress(ImportError) and aborts the whole setup command. Same class of bug: a non-integer --oss-vector-port crashes unhandled. Both want a guarded parse with a friendly error.

Strongly worth closing (especially if this can run against a shared account)

  1. The data-loss guard fails open. The dimension-detection body in _recreate_collection_if_dims_changed is wrapped in a blanket suppress(Exception), so any transient DB/Qdrant error during detection silently skips the check. On the pinned mem0ai this only degrades to a clean insert-time error (current mem0 doesn't auto-drop), but the <3 range leaves recreate-on-mismatch paths where it would become the silent data loss the guard is meant to prevent. Catching only the "can't determine dims" cases (and logging a WARNING when detection is skipped) would make it airtight. Two smaller facets: the guard is also silently skipped for a Qdrant {host, port} config (only path/url are probed) and for unknown-dimension embedders.

  2. mem0_update / mem0_delete aren't scoped to the caller's user_id (search and add are). On a Hermes deployment where several users share one Mem0 account, user A could update/delete user B's memory given B's memory UUID. It's mitigated by scoped search (IDs aren't normally cross-visible) and non-guessable UUIDs, and the underlying update/delete API doesn't take a user_id — but a get-and-verify-ownership step before the mutation would be good defense in depth.

Test gaps to be aware of

The 15-test suite is genuinely good and non-vacuously protects ~9 of the 11 fixes. The credible holes: DirectOpenAILLM (the LLM-side credential isolation / no-os.environ / OpenRouter-bypass) has no unit coverage — only the smoke script; the self-hosted keyless (AUTH_DISABLED) header-omission branch is untested; the "atomic" writes are faked in tests so only the 0o600 mode is checked, not atomicity (and the OSS test doesn't assert the DB password stays out of stdout); and the PGVector data-loss guard is unit-only, not exercised end-to-end like the Qdrant one. If any follow-up touches _openai_llm.py or the self-hosted auth, a green suite would give false confidence.

Smaller hardening notes

  • OpenAI client is created with no explicit timeout (600s SDK default) → a hung endpoint lingers a background thread; base_url isn't scheme/TLS-validated → an http:// endpoint would send the bearer key in cleartext; the base-URL env resolution differs between the LLM and embedder shims (OPENAI_API_BASE is honored by the embedder but not the LLM), which could split-brain routing.
  • The embedder's provider guard is exact-match (== "openai") while the LLM's is normalized — safe today because mem0's factory is equally exact, but a latent fail-open if mem0 ever normalizes provider names; worth aligning for defense in depth.
  • Minor robustness: OSSBackend.close can leak the vector-store client if memory.close() throws; the prefetch thread is .start()ed outside its lock; sync_turn holds its lock across a 5s join; initialize() can raise on a non-numeric sync_max_chars. Also worth a doc note that passing API keys via CLI flags exposes them in ps/shell history, and that inline creds in a self-hosted URL get echoed to stdout.
  • The 0600 guarantee ultimately depends on the (out-of-repo) Hermes atomic writer creating the temp file 0600-at-creation rather than write-then-chmod — worth confirming host-side, since it's load-bearing for the credential-privacy claim.
  • Auto-capture truncates each message to 450 chars before background extraction — lossy on long turns, but by-design, documented, and configurable, and explicit mem0_add stores verbatim, so just flagging for awareness.
  • _recreate_collection_if_dims_changed never recreates (it only rejects) — the name invites the opposite assumption in future edits.

What I verified as clean

  • Licensing is correct and complete — Apache-2.0 body matches OpenClaw's, the upstream MIT notice + Copyright (c) 2025 Nous Research are reproduced verbatim (diffed against the live upstream file at the pinned SHA), both are declared via SPDX Apache-2.0 AND MIT in pyproject, and it's coherent without a separate NOTICE. Attribution is intact.
  • Docs are accurate — every substantive hermes.mdx claim (4 tools, top_k 10/50, rerank default, 450-char truncation, the 3s/5s waits, breaker 5/120s, self-hosted routes/auth, the dimension-guard behavior, three modes, migration prereqs) cross-checks against the code; llms.txt is in sync with a scope tag + "Use when…"; the AGENTS.md row is added; and HANDOFF.md is confirmed absent.
  • Credentials fail closed — resolved through the Hermes profile scope (never os.environ), with a ValueError when the key is missing; LLM and embedder keys stay distinct; the OpenRouter-hijack path is bypassed; no key in logs or exceptions; no TLS-verification disabling.
  • Core behavior is sound — strict arg validation before any backend call, circuit breaker that isn't consumed by validation failures (thread-safe, half-open cooldown), fail-open tool path, user_id-scoped recall, keyless self-hosted auth, and a correct self-hosted HTTP contract (routes/timeout/bounded retries/error classification). No shell injection (all list-form subprocess), no path traversal, no SSRF (the self-hosted host is operator config).
  • The PR is also honest about the out-of-scope upstream Hermes multi-profile migration blockers, which are host-side and can't be addressed here.

Net: the licensing, docs, structure, and the bulk of the fixes are well done — this is close. The pgvector exposure and the pre-release-version crash are the two concrete blockers; the guard fail-open and the update/delete scoping are the two I'd most want closed given shared-account deployments.

(Static review — verified against the PR-branch source via the contents API since main predates this directory; didn't run the suites. Given the diff is AI-generated and the line-by-line attestation box is unchecked, the verification above is meant to stand in for a good chunk of that, but the human owner should still consciously accept the pgvector and scoping trade-offs.)

The setup wizard published the pgvector Docker container's port with
a bare -p, exposing PostgreSQL on 0.0.0.0, and always set the same
hardcoded password. Bind to 127.0.0.1 only, generate a random
password per install, and scrub it from any subprocess error text
before printing.
…vector-port

The min-version check parsed mem0.__version__ with a bare int() per
segment, raising an uncaught ValueError on pre-release strings like
2.1.0rc1 (outside the surrounding suppress(ImportError)). A
non-numeric --oss-vector-port also crashed unhandled instead of
producing a friendly error.
_recreate_collection_if_dims_changed wrapped its dimension detection
in contextlib.suppress(Exception), so any transient connection error
silently skipped the data-loss guard. Split detection into its own
method, log a warning (instead of swallowing) when it can't be
determined, and add support for Qdrant's host/port config alongside
path/url. Also warn when an embedder's dimensions are unknown instead
of skipping the guard without a trace.
mem0_update and mem0_delete mutated any memory_id without checking
ownership, unlike search and add, letting one user mutate or delete
another user's memory given its UUID. Add a get-and-verify-ownership
step before both mutations, backed by a new get() method on each
backend (Platform, self-hosted, OSS).
Add a 120s OpenAI client timeout, align OPENAI_API_BASE/OPENAI_BASE_URL
resolution between the LLM and embedder shims, normalize the embedder
provider guard, isolate close() failures per-object instead of aborting
the whole chain, move prefetch thread start() inside its lock, avoid
holding sync_lock across a 5s join, guard non-numeric sync_max_chars,
and rename _recreate_collection_if_dims_changed to
_reject_dimension_mismatch to match what it actually does.
…nc_max_chars

Add coverage for the review gaps: DirectOpenAILLM reads its API key through
the secret scope and bypasses OpenRouter, raises when no key is available,
SelfHostedBackend omits X-API-Key for AUTH_DISABLED servers, and a
non-numeric sync_max_chars falls back to the default instead of crashing
initialize(). Update the two dimension-guard tests for the
_reject_dimension_mismatch rename.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation integrations Agent and editor integrations plugin Editor and agent plugins (Claude Code, Cursor, Codex, OpenClaw, Pi Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants