Repository navigation
Conversation
…ronClaw memory service facade Implements the M1 slice of docs/reborn/memory-rd/01-memory-placement/engineering.md: - Adds MemoryService trait + NativeMemoryService in ironclaw_memory as the provider-neutral facade. Native memory remains the default backing provider. - Routes builtin.memory_search/write/read/tree and builtin.profile_set through the facade instead of directly to MemoryBackend. - Routes ProductionMemoryPromptContextService (loop context retrieval) through MemoryService. - Adds MemoryProfileBindingConfig / RequiredMemoryProfileId / resolve_memory_profile_bindings in ironclaw_host_runtime as the declarative profile-to-extension binding resolver (memory.context_retrieval.v1, memory.interaction_log.v1, memory.document_store.v1, memory.semantic_search.v1). - Adds host port ID constants for storage and audit in ironclaw_host_api. - Enforces that model/api-visible extension capabilities must declare prompt_doc_ref in manifest v2. - Adds caller-level contract tests for all new shapes. Host runtime keeps authority: scope, grants, approvals, audit, prompt-write safety. IronClaw memory facade owns memory semantics and native provider behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…provider Carve ironclaw_memory into two crates so memory is modeled as a pluggable provider behind a shared interface (issue #3537): - ironclaw_memory: the provider-agnostic memory layer — the MemoryService interface + operation DTOs, scope/path/context value types, prompt-write safety vocabulary, and audit/event contracts. What the host and every provider share. - ironclaw_memory_native (new): the native filesystem provider — NativeMemoryService, backend/repository, the /memory adapter, chunking/ search/indexing, and the prompt-write-safety enforcement engine. One provider, a peer to a future ironclaw_memory_honcho. Depends on ironclaw_memory. Consumers depend on ironclaw_memory for the interface; host_runtime and the native crate's own tests use ironclaw_memory_native for the concrete provider. Behavior-preserving — pure reorganization. ironclaw_memory and ironclaw_memory_native test suites are green; host_runtime results are identical to the M1 baseline (244 passed, same 5 pre-existing Docker-socket / prompt_doc_ref fixture failures); the lone event_projections failure (extension_lifecycle) is pre-existing and imports no memory code. The only internal change is MemorySignificantEvent::search_performed, which now takes full_text/vector primitives instead of &MemorySearchRequest so the agnostic crate stays free of search-impl dependencies; it builds the identical event. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nt-m2-lift # Conflicts: # Cargo.toml
The M1 facade's MemoryProfileBindingTarget/RequiredMemoryProfileId build CapabilityProfileId/ExtensionId from hardcoded valid literals via .expect(). The no-panics CI gate flags any production .expect() unless the line carries an inline `// safety:` comment (lowercase, same line). The prior comments were `// Safety:` block comments above the call, which the scanner does not honor. Annotate both sites inline; behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es (#3537) Issue #3537 specifies prompt_doc_ref — the model-facing prompt documentation for a capability — is required only for model-visible capabilities. The M1 facade over-broadened the manifest-v2 rule to also reject api-visible capabilities, which broke api-visible fixtures across the tree and contradicted both the issue and the contract docs. Scope the rule to CapabilityVisibility::Model (api/host-internal exempt, as before). Flip the two tests that asserted api-visible rejection, add prompt_doc_ref to the incidental model-visible test fixtures the rule legitimately rejects (capabilities, event_projections, extensions, host_runtime), and reconcile the extensions/host-runtime contract docs. No production manifest is affected — first-party assets already declare prompt_doc_ref for every model-visible capability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s, coverage) Follow-ups from review of the ironclaw_memory / ironclaw_memory_native split: - Add a dependency-boundary rule for ironclaw_memory_native mirroring ironclaw_memory, and add it to product_adapters' forbidden lower-layer list — the new provider crate was previously ungoverned (fail-open). - Replace the unchecked public MemoryDocumentPath::from_validated_parts with from_scope, which re-validates the relative path and returns Result, so no caller in another crate can build a malformed path. - Fix rename stragglers: agnostic-crate rustdoc that misnamed the impl crate; the native crate's CLAUDE.md/AGENTS.md title + validation command; the moved contract-test breadcrumbs; add guardrail files to the agnostic ironclaw_memory crate. - Re-establish provider-side coverage for the context-retrieval scope- isolation and non-finite-score invariants that moved into NativeMemoryService. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
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:
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a public composition seam to override the default native memory provider with a caller-supplied MemoryService (e.g., for testing or benchmarking) on the local-dev build path. The reviewer suggests failing loudly by returning a specific configuration error variant if memory_service_override is provided in build_production_shaped, rather than silently ignoring it.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // The memory-service injection seam is wired through the local-dev | ||
| // build path only (see `build_local_runtime`); the production-shaped | ||
| // path keeps the default native provider. Threading an override here | ||
| // would also require a process-backend-aware registry variant, which | ||
| // this slice deliberately defers. | ||
| memory_service_override: _, |
There was a problem hiding this comment.
Silently ignoring memory_service_override in build_production_shaped can lead to unexpected behavior (e.g., tests or benchmark harnesses silently falling back to the default native provider and hitting the real filesystem instead of using the injected override). To prevent this, we should explicitly check if memory_service_override is Some and return a specific error variant (such as MalformedConfig or a dedicated configuration error variant) to fail loud, rather than reusing generic or misleading error variants.
References
- When adding preflight configuration checks, introduce specific error variants (e.g.,
MalformedConfig) for configuration errors rather than reusing existing, potentially misleading variants.
e7c33e4 to
60324a5
Compare
… path Stacked on #5163 (reborn/memory-placement-m2-lift). Expands the NATIVE memory provider so it can optionally be initialized with a set of starting documents, exposed on the public composition build path. This is a general capability (tests / demos / migrations), not benchmark-specific. - `ironclaw_memory_native`: - New `SeedMemoryDocument { tenant_id, user_id, agent_id, project_id, path, content, metadata }` — a starting document with its own scope. - `NativeMemoryService::seed_documents(docs)` writes each seed through the REAL native write path by calling `MemoryService::write` — the exact entrypoint an agent write uses. Seeds are therefore stored, versioned, prompt-safety-scanned, and chunked/indexed identically to agent writes. Per-doc scope is honored via a `MemoryInvocation` built from each seed's tenant/user/agent/project (the native path derives the `MemoryContext` exactly as for an agent write). Nothing is special-cased or bypassed. - `ironclaw_reborn_composition`: - `RebornBuildInput::with_seed_memory(impl IntoIterator<Item = SeedMemoryDocument>)`. - Threaded into the factory's native construction on all paths: local-dev / hosted-single-tenant (`build_local_runtime`) and production libsql/postgres (`build_production_shaped` -> `RebornProductionBuildContext` -> `build_backend_production`). Seeds are written over the same composed filesystem the runtime's memory capability reads, before the runtime is returned. Empty (the default) writes nothing. Search, capabilities, and backend behavior are UNTOUCHED: `search.rs`, `backend.rs`, `indexer.rs`, the `MemoryBackendCapabilities`, and the whole `ironclaw_memory` crate have zero changes. An in-memory backend still has no full-text search, so search behavior is unchanged. Default (no seed) is byte-identical to today on every path. Drops the earlier `with_memory_service` provider-swap (would have allowed a custom-search provider — the opposite of using ironclaw's real native memory). Tests: seeds write through the native path and are readable/listed (native crate); end-to-end through the real runtime memory dispatch and per-doc scope honored; default path yields an empty memory tree. Builds clean under default, libsql, and postgres features; clippy clean (the one `secret_store` warning pre-exists on the base branch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
60324a5 to
3b09bb7
Compare
f3fac01 to
09f7731
Compare
Summary
Stacked on #5163 (
reborn/memory-placement-m2-lift). Merge #5163 first.Expands the native memory provider so it can optionally be initialized with a set of starting documents, exposed on the public composition build path. This is a general capability (tests / demos / migrations), not benchmark-specific.
The benchmark (and any caller) uses ironclaw's real native memory — real write/read/tree/search/scope/safety — not a swapped-in custom provider. An earlier
with_memory_serviceprovider-swap was dropped (it would have allowed a custom-search provider, the opposite of what we want).Public API
SeedMemoryDocumentshape (each seed carries its own scope):Seeds go through the REAL native write path
NativeMemoryService::seed_documents(docs)ingests each seed by callingMemoryService::write— the exact entrypoint an agent write uses (crates/ironclaw_memory_native/src/service.rs, inseed_documents, callingself.write(invocation, request)). So each document is stored, versioned, prompt-safety-scanned, and chunked/indexed identically to an agent write. Per-doc scope is honored via aMemoryInvocationbuilt from the seed's tenant/user/agent/project; the native path derives theMemoryContextexactly as it does for an agent write. Nothing is special-cased or bypassed.Where seeds get written before the runtime is returned:
crates/ironclaw_reborn_composition/src/factory.rs—build_local_runtimecallsseed_native_memory(filesystem, seed_memory_documents)just before constructing the host runtime, over the same composed/memoryfilesystem the runtime's memory capability reads.build_production_shaped→RebornProductionBuildContext { seed_memory_documents }→build_backend_production, which callsseed_native_memory(stores.filesystem, ..)before returning.seed_native_memorybuilds aNativeMemoryService::from_filesystem(fs, None)over the runtime filesystem and callsseed_documents. TheNoneprompt-write-safety event sink means seeds emit no audit events (they are setup, not agent actions) — but the native pipeline still runs the same prompt-safety policy scan, versioning, and indexing.Search / capabilities UNTOUCHED
search.rs,backend.rs,indexer.rs, theMemoryBackendCapabilities, and the entireironclaw_memorycrate have zero changes (verified by diff). No FTS added, no bench-specific anything. An in-memory backend still has no full-text search, so search behavior is unchanged. Default (no seed supplied) is byte-identical to today on every path — proven by a test asserting an empty memory tree when no seeds are given.Verification
cargo clippyclean (the onesecret_store"never used" warning pre-exists on the base branch under--tests, unrelated to this change).with_seed_memoryrecords docs / defaults empty; end-to-end — building a runtime withwith_seed_memoryand reading the doc back through the real native memory dispatch (memory_read/memory_tree); and a no-seed build yields an empty memory tree.🤖 Generated with Claude Code