Repository navigation
feat(memory): MCP-backed memory provider — bind a memory system by config, not by a factory arm - #7661
feat(memory): MCP-backed memory provider — bind a memory system by config, not by a factory arm#7661serrrfirat wants to merge 4 commits into
Conversation
Adds `ironclaw_memory_mcp`: a memory provider bound to a backend by CONFIGURATION rather than by a compiled factory arm. Native and mem0 each require a crate plus a match arm, so adding a memory system meant changing IronClaw; a system speaking the memory-over-MCP tool contract now binds with a server URL, a credential, and two tool names. Implements two lanes -- `read_long_term` and `record_interaction` -- which is retrieve-before-run and record-after-turn. `read_short_term` and `profile_read` fall through to the trait defaults; the host only calls the lifecycle hooks a manifest declares, so a subset is a supported shape. Boundary: `substrates` may depend only on contracts/substrates, and `ironclaw_mcp` is `runtimes` (it owns capability adaptation and resource governance, not just protocol). So the MCP client is injected through the `McpMemoryTransport` seam by composition, the one layer permitted to name both crates. Same shape as mem0's transport port, and it makes every mapping unit-testable with no server present. Safety properties pinned by tests: tool arguments carry the trusted ResourceScope and the query is the only model-influenced field; result sets are bounded before host budgeting; a transport failure degrades the lane as Unavailable rather than returning empty, so the degradation note from #7553 can tell broken from empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `ironclaw_memory_mcp` to MEMORY_PROVIDER_CRATES while nothing depends on it yet, so the wiring change has to add a residue row deliberately instead of discovering the rule after the fact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-7661 environment in ironclaw-ci-preview
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: dependencies
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Automatic trigger · attempt 1 of 3 · completed in 3m 29s IronLoop completed the review and posted it to GitHub. 🔗 Result |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 IronLoop review
The new MCP provider can bypass the host’s cross-scope memory admission guard.
Findings: 🔴 High 1
🔴 High · Preserve remote candidate scope for host admission
Inline on crates/extensions/packages/memory-mcp/src/service.rs:149. See the inline comment for details.
Validation
- ✅ MCP provider unit tests — All 8 crate unit tests passed.
Review details
- Run:
b0c06a67-f832-44e2-b385-3e410c3714bd - Workflow: Review
- Attempts: 1
| tenant_id: scope.tenant_id.as_str().to_string(), | ||
| user_id: scope.user_id.as_str().to_string(), | ||
| agent_id: scope.agent_id.as_ref().map(|id| id.as_str().to_string()), | ||
| project_id: scope.project_id.as_ref().map(|id| id.as_str().to_string()), |
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🔴 High · Preserve remote candidate scope for host admission
Every remote candidate is relabeled with the invoking scope before it reaches the host. If a buggy or malicious MCP server returns another tenant’s/user’s record, its original ownership is discarded and the host’s cross-scope filter sees it as in-scope, allowing its content into the current turn’s prompt. Require and validate response scope fields (dropping missing/mismatched rows), or otherwise establish an equivalent trusted isolation boundary before constructing snippets.
|
Tracking issue for the remaining work — composition wiring, Mnesis as first consumer, and publishing the contract: #7664 |
The target-tree gate failed because §5 draws no home for the new package. Amending §5 is the right fix rather than an EXCEPTIONS row: the crate sits exactly where a [memory] provider package belongs, next to memory-native and mem0 — §5 simply had not been told about it. EXCEPTIONS is for owned deltas between the drawing and the tree, and there is no delta here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # Cargo.toml
This branch was successfully deployed
1 participant
What this is
The first half of making memory pluggable: a memory provider that is bound to a
backend by configuration instead of by a compiled factory arm.
Today
nativeandmem0are each a workspace crate plus amatcharm inmemory_provider_factory.rs, so "plug in your memory system" means opening a PRagainst IronClaw.
ironclaw_memory_mcpis one provider that serves any memorysystem speaking the memory-over-MCP tool contract — a server URL, a credential,
and two tool names.
The property that makes it generic rather than a single-vendor adapter: tool
names are configuration, not constants (
tool_names_come_from_configurationpins this). The defaults are the names Mnesis Core publishes in its
manifests/mcp-tools.json, since it is the first implementer.Scope — this PR does not wire anything
Deliberately provider-only. No composition factory arm, no manifest, no config
plumbing, so nothing binds this yet and no runtime behavior changes. It is
additive and inert; the wiring is the next PR. See Follow-ups.
Lanes
read_long_termmemory_search)record_interactionmemory_add_session)read_short_termprofile_readTwo lanes is retrieve-before-run and record-after-turn, which is the loop that
makes memory feel like memory. The unmapped lanes fall through to the trait
defaults: the host only calls the lifecycle hooks a provider's manifest declares,
so shipping a subset is a supported shape rather than a partial implementation.
Why the transport is injected
substratesmay depend only oncontracts/substrates, andironclaw_mcpisruntimes— it owns capability adaptation and resource governance, not just theprotocol. So this crate cannot depend on the MCP lane. It takes an injected
McpMemoryTransport, and composition (the one layer permitted to name bothcrates) supplies an adapter over the host-mediated
McpHostHttpClient.That keeps the host-mediated egress guarantee intact, keeps the crate inside the
same internal-dependency boundary mem0 already passes, and makes every mapping
unit-testable with no server present. It is the same shape as mem0's
Mem0Transportport.Safety properties, pinned by tests
ResourceScope;the query string is the only model-influenced field. A memory server cannot be
steered across tenants by prompt content.
read_long_termreturns rawcandidate text; the host keeps cross-scope filtering, sanitization, the
untrusted-memory envelope, and every model-visible budget. This division is
what makes accepting a third-party memory backend defensible.
untrusted input and may answer a
limit: 4request with thousands of rows.Unavailable, never as empty. This is what lets thedegradation note from fix(memory): ranked recall retrieval + a visible difference between broken and empty memory (#7185) #7553 keep telling "retrieval broke" apart from "nothing
matched" — the distinction Memory not reliably recalled across conversations #7185 showed is corrosive to lose.
Test Strategy
composition, so there is no production-wired path to drive. Integration
coverage lands with the composition PR that binds the provider.
property above plus cross-vendor response-envelope tolerance.
ironclaw_memory_mcpadded toMEMORY_PROVIDER_CRATESwhile nothing depends on it, so the gate covers it from its first commit and
the wiring PR must add a residue row deliberately.
reborn_dependency_boundaries— 42 passed.MemoryServiceconformance suite(
ironclaw_memory::test_support). It needs a stateful fake MCP server, the waymem0's does; that lands with the wiring PR.
Verified locally:
cargo fmt,cargo clippy -p ironclaw_memory_mcp --all-features --tests -- -D warnings(clean),cargo test -p ironclaw_memory_mcp(8 passed),cargo test -p ironclaw_architecture_tests --test reborn_dependency_boundaries(42 passed).Follow-ups
one manifest-keyed factory arm serving every MCP provider rather than one
arm per vendor. Plus the conformance suite against a fake server.
Blocked on a licensing question:
neo-sky/mnesis-corecurrently publishes nolicense, so we cannot ship or recommend an integration that depends on it.
vendor-facing document plus an externally runnable conformance suite. Held at
provisional until a second independent implementer exists, so we are not
freezing a contract designed from one data point.
Risk / rollback
Additive and unbound: no existing provider, binding, or runtime path changes.
Rollback is deleting the crate and its
MEMORY_PROVIDER_CRATESentry.Refs #7185.