Repository navigation
Fix three setup gaps in the MCP-based adapters (engram, Hindsight, aionforge) - #4
Merged
Merged
Conversation
- Name the mcp client SDK in the engram, Hindsight and aionforge docstrings' install lines; the adapters import it but nothing said so. - Hindsight and aionforge: use streamable_http_client with an httpx client from create_mcp_http_client, since mcp 2.x removed streamablehttp_client and yields two streams instead of three. Works on mcp >= 1.24, including 2.x. - Engram: create ENGRAM_DATA_DIR before starting `engram mcp` or the CLI in it, so a fresh path no longer fails health(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks, Jeff. All three are real, and the write-up made them quick to check. I went through the diff against mcp 1.27.1: both names exist there, the bearer still goes on every request through the client, and create_mcp_http_client’s default timeouts (30 s, 300 s read) match what streamablehttp_client used, so Hindsight’s long sync_retain calls are unaffected. Creating the engram data dir at first use rather than in init is the right place. Merging. Glad the adapters are useful for the Particles currency benchmark. If you publish numbers from them, a link here would be welcome |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi, thanks for adebench. I'm running its Hindsight, mem0, cognee and engram adapters unmodified (at
6b23060, Python 3.11, macOS arm64) from a currency benchmark for the Particles memory project. On the way I hit three setup gaps in the MCP-based adapters. None of them changes a result once it's worked around, but each one cost a failed start, and the first two hit anyone who installs fresh today. This PR fixes all three.1. The
mcprequirement wasn't documented. The README says each memory's install is described in its adapter's docstring.engram.py,hindsight.pyandaionforge.pyimportmcpinside the adapter, but none of their docstrings names it. In a fresh venv the first connection raisesModuleNotFoundError: No module named 'mcp'; for engram that shows up only ashealth()returning False. Each docstring now has apip installline:mcp>=1.24for the two HTTP adapters (see 2), and plainmcpfor engram, whose stdio client hasn't changed.2.
mcp2.x removedstreamablehttp_client.hindsight.pyandaionforge.pyimportedmcp.client.streamable_http.streamablehttp_client. Inmcp2.3.0 that module exports onlystreamable_http_client, so an unpinnedpip install mcpgives anImportErroron connect. The new function takes headers through anhttpxclient rather than aheaders=argument. It also yields two streams in 2.x, where 1.x yielded three (the third was the session-id getter). Both adapters now:create_mcp_http_client(headers=...), so the bearer still goes on every request;Both names exist from
mcp1.24.0 on, hence the floor.3. The engram adapter failed on a data directory that didn't exist yet. The docstring asks for a FRESH
ENGRAM_DATA_DIR, andEngramAdapteruses that directory as the working directory of bothengram mcp(_conn) and the CLI (_cli). A subprocess can't start in a missing directory, so a fresh path madehealth()return False, andexamples/engram_import.pystopped with "Engram does not answer", which doesn't point at the directory. Both paths now callos.makedirs(self.data_dir, exist_ok=True)first. I put it there rather than in__init__so that building an adapter never creates~/.engram.Testing
python -m pytest -q: 274 passed withmcp1.30.0 and with 2.3.0._Mcpconnected to a local FastMCP streamable-HTTP server, listed its tools and called one, withmcp1.24.0, 1.30.0 and 2.3.0. The server saw theAuthorizationheader on every request.ENGRAM_DATA_DIR,EngramAdapter._clinow creates the directory and runs.mcp2.x. On 1.30.0 the engram and Hindsight adapters ran clean end to end for me before this change.🤖 Generated with Claude Code