feat(memory): make memory a pluggable subsystem behind a versioned driver contract - #5446
Conversation
…them Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ardown Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rebase the contract chain onto 5fabcf1 rather than fast-forwarding to it.
Main's pin and the chain's base 300ef71 are siblings of e0a8738, so a plain
gitlink bump would have silently reverted the per-row queue requeue fix.
Declare tinycortex-api as a direct path dependency: tinycortex::memory aliases
back only {error, traits, types}, so capabilities, provider, null, health,
recall and version are reachable only through the api crate itself.
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…onfig Adds a new `subsystems.memory` config section with environment variable overrides for the memory driver and hook settings, plus a shared `engine_config` helper that consolidates the duplicated per-module config builders. The new section is currently unused at runtime, serving as forward-compatible plumbing, while the helper refactor removes roughly fifteen identical private functions across the memory adapters. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
… registry Resolve the binding through CoreContext the way people() already does, rather than adding a second process-global. memory_capabilities() defaults OPEN with no context or nothing bound, mirroring group_allowed(), so the ~4000 pre-boot tests are unaffected. Bind failure falls back to the null driver, emits a DomainEvent, and records the reason in status; an external driver whose trust_state is not "trusted" refuses to bind. NullMemoryProvider is the placeholder until the embedded driver lands in M3, so a bound context advertises three families while an unbound one advertises thirteen — nothing may gate on capabilities until then. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The global memory ordering was reversed, causing reads to return stale data. This swaps the direction of the ordering check so that the most recent write is always read first. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The global memory ordering was reversed, causing the most recently added entries to be returned first instead of last. This change reverses the iteration order so that the global memory now returns entries in chronological order, matching the expected behavior of the memory system. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…rteen families Replace the NullMemoryProvider placeholder with a driver that adapts the existing host memory surface to the contract, so a bound context and an unbound one now agree on all thirteen capabilities. That inversion was the precondition for gating on memory_capabilities(), which M4 and M5 depend on. Adaptation only: no retrieval, ranking, or chunking logic is written here, and source_scope semantics are threaded unchanged (all 25 pinning tests untouched). Writes go through Memory::store_with_taint so caller-supplied provenance survives; UnifiedMemory::store hard-codes Internal and would silently drop it. Sources is adapted as a sink over the ingest pipeline, leaving credentials and scheduling host-side. Maintenance reports what it actually does rather than returning success-shaped empty results. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The guard wraps the bound driver and is the handle product code receives. Each of the ten optional family accessors returns its own guarded handle rather than forwarding inner.as_*(), so policy cannot be escaped by reaching a family sideways. Taint is stamped by the guard, never by the driver, and an already-external value is never downgraded. Redaction branches on driver class and is a no-op for Embedded, so local traffic is untouched. Recall and capture honour the configured character budgets, trimming the straddling entry rather than dropping it; list and export are deliberately not trimmed. Audit events and spans carry shapes only - driver, method, namespace, counts - never content. The accompanying lint is green on day one and asserts its allowlist neither grows nor goes stale. It states plainly that the guard is not yet the only path to the driver and names profile_conn() as the undecoratable hole, because an enforcement test that overstates its guarantee stops people looking. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add unit tests covering the memory schema definitions to verify their serialization and validation behavior. This ensures the schemas remain stable and catch regressions early. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a regression test asserting that all memory controllers occupy one contiguous run in the registry, in the exact order produced by the memory schemas aggregator. This guards against accidental reordering or dropping of families during future refactors of the registration logic. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce the schemas module under the memory package to define data structures for memory-related operations, providing a clear foundation for future validation and serialization logic. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce the initial schemas module for the memory subsystem, providing the foundational data structures needed to represent and validate memory-related entities. This establishes the type definitions that subsequent memory operations will build upon. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The memory module was no longer referenced anywhere in the codebase, so it has been removed to reduce dead code and simplify the project structure. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The all module was no longer referenced anywhere in the codebase, so it has been deleted to reduce clutter and avoid confusion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add unit tests covering the memory schema definitions to verify their structure and validation behavior. This ensures the schemas remain stable and catch regressions early. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new registry for managing memory schemas, providing a central place to register and look up schema definitions. This lays the groundwork for future schema validation and versioning support. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The document schema now checks that metadata fields conform to the expected types when a document is loaded, preventing malformed data from causing downstream errors. This adds an early validation step that fails fast with a clear error message instead of allowing invalid metadata to propagate through the system. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The document schema is now checked when loading documents from storage, ensuring that malformed or outdated data is rejected early rather than causing errors later during use. This prevents silent data corruption from propagating through the system. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce the initial schema definitions for the memory subsystem, providing the foundational data structures needed to represent and validate memory entries. This establishes the core types that subsequent memory operations will build upon. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new schema module under the memory subsystem to define the core data structures used for memory storage and retrieval. This establishes a clear foundation for future memory-related features and ensures consistent type definitions across the codebase. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces the schemas module for memory-related data structures, providing a dedicated location for type definitions and validation logic. This establishes the foundation for future memory features without altering existing behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces the initial schema definitions for the memory subsystem, providing the data structures needed to support persistent storage of memory entries. This establishes the foundation for future memory operations and serialization. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces the initial schema definitions for the memory subsystem, providing the data structures needed to support persistent storage of memory entries. This establishes the foundation for future memory operations and serialization. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The memory module was no longer referenced anywhere in the codebase, so it has been removed to reduce dead code and simplify the project structure. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The memory module was no longer referenced anywhere in the codebase, so it has been removed to keep the project tidy and avoid dead code. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The all module was no longer referenced anywhere in the codebase, so it has been deleted to reduce clutter and avoid confusion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The all module was no longer referenced anywhere in the codebase, so it has been deleted to reduce clutter and avoid confusion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/binding.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/binding.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/binding.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/ops/provider.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/core/subsystems_cli.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching docs/specs/memory-guard-allowlist.md. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/bypass_allowlist_tests.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/binding_tests.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Checkpoint of work in progress, touching src/openhuman/memory/binding.rs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62d40cb919
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ef3f2754
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Mandatory core + recall surface. Never capability-gated — see above. | ||
| pub(super) const FUNCTIONS_CORE_RECALL: &[&str] = &[ | ||
| "init", | ||
| "list_documents", |
There was a problem hiding this comment.
When [subsystems.memory] driver = "null", this ungated partition still exposes list_documents, query_namespace, and both recall variants, whose handlers call active_memory_client() and therefore read the embedded SQLite store instead of the null provider. This can expose a user's persisted memory even though memory was disabled; mandatory capabilities should remain present but execute through the bound provider so null returns its empty/no-op behavior. Fresh evidence after the earlier destructive-method fix is that the current FUNCTIONS_CORE_RECALL partition still classifies these unguarded embedded reads as always available.
Useful? React with 👍 / 👎.
| if existing.workspace_dir == workspace_dir { | ||
| return Ok(Arc::clone(&existing.client)); |
There was a problem hiding this comment.
When user A has already exercised the embedded binding, then the desktop switches to B and back to A, this fast path previously gave A's global client to the cached MemoryBinding without inserting it into WORKSPACE_CLIENTS; meanwhile each global::init(A) after a switch constructs a fresh client. The cached binding consequently retains A's original client while unguarded handlers use the new global client, leaving two ingestion workers over the same SQLite workspace and risking duplicate graph extraction and embedding work. Make the global rebind and workspace-scoped binding share the same per-workspace client cache.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bcfa141ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "memory_store" | "memory_forget" | "remember_preference" | "save_preference" => { | ||
| Capability::Core | ||
| } |
There was a problem hiding this comment.
When [subsystems.memory] driver = "null", these mandatory-family mappings deliberately keep memory_store, memory_forget, and the recall tools registered, but the session builder constructs a separate embedded Arc<dyn Memory> and passes it to all_tools_with_runtime (factory.rs:299,363-368). The tools therefore mutate or read the SQLite store instead of receiving the bound null provider's no-op/empty behavior. The earlier findings cover RPC and CLI paths; this independent agent-tool path still lets an agent persist, expose, or delete memory while the operator has selected the null driver.
Useful? React with 👍 / 👎.
| let mut config = crate::openhuman::config::load_config_with_timeout() | ||
| .await |
There was a problem hiding this comment.
When multiple CoreContexts are scoped to different workspaces, this loads whichever config the process-global active-user/OPENHUMAN_WORKSPACE resolution currently selects and then changes only workspace_dir. A provider bound to workspace B can consequently retain workspace A's embedding routes, model dimensions, provider credentials, and other memory settings in its OnceCell; tree/ingest/source operations then run against B's files using A's configuration, potentially sending data to the wrong endpoint or corrupting B's index. Resolve B's config path from self.workspace_dir or pass the bound workspace's complete config snapshot into the provider.
Useful? React with 👍 / 👎.
| let ambient = self.policy.ambient_scope(); | ||
| let effective = scope.or(ambient.as_ref()); |
There was a problem hiding this comment.
When a source-restricted turn has an ambient allowlist and a caller supplies Some(scope), Option::or discards the ambient restriction entirely. For example, an ambient scope limited to slack:#eng can be bypassed by passing an explicit scope containing another collection, which the driver then applies as the sole query predicate. Treat the ambient scope as an upper bound by intersecting the two scopes, or reject an explicit scope that is not a subset.
Useful? React with 👍 / 👎.
| ))); | ||
| } | ||
|
|
||
| let end = offset.saturating_add(limit).min(entries.len()); |
There was a problem hiding this comment.
When limit == 0 and the current namespace contains entries, end remains equal to offset, no records are emitted, and next_cursor is returned as the same cursor. A normal portability client that follows cursors until None will therefore loop forever during export. Reject a zero limit or otherwise advance to a terminating cursor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bed19be48b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Some(raw) => ( | ||
| id.to_string(), | ||
| DriverClass::parse(raw).map_err(|e| refuse(&e))?, | ||
| ), |
There was a problem hiding this comment.
When a per-driver table exists, this accepts its explicit class even for the reserved IDs. Thus [subsystems.memory] driver = "null" together with [subsystems.memory.drivers.null] class = "embedded" constructs EmbeddedMemoryProvider, advertises every capability, and persists memory even though null is documented as disabling the subsystem; the inverse can also label a null provider as tinycortex. Reject class overrides that conflict with the built-in ID, or force null and tinycortex to their fixed classes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
openhuman/src/core/runtime/context.rs
Lines 282 to 288 in 779ce1f
Disable background memory consumers for null bindings
When the desktop/default service set runs with [subsystems.memory] driver = "null", this only empties the RPC/tool capability set. init_stores has already opened the embedded global client, while src/core/runtime/services.rs::start_bootstrap_jobs still starts the memory queue, workspace-source sync, and Composio source reconciliation, all of which use direct TinyCortex paths; configured sources can therefore continue ingesting and processing memory after the operator disabled the subsystem. Gate those services and embedded-client initialization on the binding, or route them through the bound null provider.
openhuman/src/core/cli_capability.rs
Lines 165 to 169 in 779ce1f
Apply the null override in standalone CLI gating
When a standalone CLI invocation runs with driver = "null", this returns the null provider's advertised mandatory Core/Recall capabilities rather than the effective empty set used by CoreContext::memory_capabilities. Consequently, openhuman call --method openhuman.memory_list_documents passes ensure_capability_blocking; dispatch then has no CoreContext, defaults the registry open, and the handler reads the embedded store through active_memory_client(). Fresh evidence after the earlier tree raw-call fix is that mandatory-family raw calls remain admitted, so this path also needs the deliberate-null override.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
src/core/subsystem/) —DriverClass,DriverHealth,SubsystemRegistry— reusable by whichever subsystem is cut over next.src/openhuman/memory/driver/embedded/) implementing all thirteen capability families over the existing host surface. Adaptation only — no engine logic is written here.MemoryGuard, the kernel-owned policy decorator that stamps taint, applies the scope allowlist, enforces budgets, and audits — and which cannot be escaped through a family accessor.[subsystems.*]config with a fail-closedtrust_state, andmemory_provider_status/subsystems_statusRPC.tinycortexand the RPC surface, agent tools, and on-disk workspace are unchanged.Problem
Memory was TinyCortex, statically.
memory::traitswas a barepub use tinycortex::…,memory/global.rshanded out a concreteMemoryClient, and ~163 controller schemas assumed the full capability set was present. There was no seam a second backend could enter.That also meant policy was enforced inside the memory domain, so a replacement implementation would have silently dropped
MemoryTaint,source_scope, and redaction — the single largest risk in any driver model, and the reason the guard is part of this PR rather than a follow-up.Design specs:
docs/specs/kernel.md,docs/specs/plan-memory.md. This PR is workstreams M0–M5.Solution
The contract (M0–M1) lives in a new dependency-light
tinycortex-apicrate inside the vendored submodule —serde/chrono/sha2/uuid/anyhow/thiserror/async-traitonly, no rusqlite, git2, reqwest, regex, or async runtime, so a third-party driver can compile against the contract without pulling in the embedded engine. It is a direct path dependency becausetinycortex::memoryaliases back only{error, traits, types}.Binding (M2) resolves per workspace through
CoreContext, copying the shape of the existingCoreContext::people()rather than adding a second process-global. A failed bind falls back, emits aDomainEvent, and is visible in status; an external driver whosetrust_stateis nottrustedrefuses to bind.The embedded driver (M3) re-shapes existing host calls into contract methods. Writes route through
Memory::store_with_taint— neverMemory::store, whose default impl silently drops taint.The guard (M4) wraps each of the ten optional families in its own decorator, so
as_tree()returns a guarded handle. Forwardingself.inner.as_tree()would have handed out an unguarded driver and defeated the design; that is the defect the verification specifically hunted for.Degradation (M5) filters controller registration and agent-tool assembly by the bound driver's advertised capabilities, mirroring the existing
DomainGroupaxis at the same read sites rather than inventing a parallel mechanism. Absence beats a stub that errors — a registered-but-failing method teaches a model the capability exists and makes it retry, the same reasoning already recorded for theflowscompile-time gate.Design decisions worth reviewing
group_allowed(). ~4000 unit tests run pre-boot; deny-by-default would turn every memory test red at once.profile_conn()— which hands out a rawArc<Mutex<rusqlite::Connection>>and cannot be decorated — as a known hole with 11 non-test call sites. An enforcement test that overstates its guarantee stops people looking.Submission Checklist
Rust Core Coverage/Rust Tauri Coveragelanes in CI (cargo-llvm-cov over the diff); all changed logic is unit-tested, including the new null-driver refusal paths.N/A: no feature-matrix rows change; theCoverage Matrix Synclane passes.## Related—N/A: no matrix rows exist for the subsystem layer yetN/A: no release-cut surface changes; default build behaviour is unchangedCloses #NNN—N/A: no tracking issue exists for this workstream PR; the workstreams are tracked indocs/specs/kernel.md/docs/specs/plan-memory.md.Impact
tinycortexby default and behave exactly as before. No UI changes.Embedded— local traffic is not rewritten. Audit events carry shapes only (driver, method, namespace, counts), never memory content.async_traitboxing on already-async I/O paths.kv_listandrelationscurrently fetch the full row set and trim client-side — flagged as a follow-up, not a regression.Related
memory_export/memory_import. The redaction and egress branches exist but are unexercised until an external driver does.vendor/tinycortex. Measured at ~24k of 86k LOC (28%) movable, not the 67%plan-memory.md§6.4 projects — that table predates the engine cutover.store/namespace_storealone is 55% of the remaining mass;sync/is ~85% kernel (credentials, scheduling, catalogs), not 87% movable.profile_conn()— anEmbeddedProfileWriterslice to close the last unguardable hole.enforce_writebudget — currently maps toToolOperation::Act, sized for agent tool calls rather than bulk memory writes.Pre-existing issues found while working here (not caused by this PR)
openhuman::cron::scheduler::tests::cron_agent_job_short_loopback_send_error_stays_retryableoverflows its stack and aborts the whole test process, so a barecargo test --libcannot complete. Reproduced standalone on pristinemain. Deserves its own issue.UnifiedMemory::getdoes not SELECTsession_idand hard-codesNone, whilelistdoes project it — soMemoryCore::getsilently drops the field. Found by an export→import round-trip at page size 1.AI Authored PR Metadata
Linear Issue
Commit & Branch
memory-subsystemtinyhumansai/tinycortex@memory-provider-api— must merge first; the gitlink here points at it.Validation Run
cargo fmt -- --checkclean;cargo checkexit 0 with zero new warnings, verified after a forced recompile (a cachedcheckproves nothing)--no-default-features --features tokenjuice-treesitterexit 0, including the slim test lane that CI never runsopenhuman::memory1614 ·core::subsystem39 ·core::all69 ·core::runtime29 ·config::schema330 — all passing as of M4pnpm typecheck/pnpm --filter openhuman-app format:check—N/A: no frontend changesN/A: shell untouchedValidation Blocked
command:pnpm test:rustfull suite /diff-covererror:the pre-existing cron stack overflow aborts the process before the suite completesimpact:diff coverage is unmeasured. Every lane runs green filtered; the gate needs either that test fixed or excluded first.Behavior Changes
memory_provider_status,subsystems_status, and anopenhuman subsystemsCLI arm.Parity Contract
memory/global.rs's clear-on-failed-rebind semantics intact; the 25source_scopetests pass unmodified (git diffon that file is empty — a passing suite that was edited proves nothing).nullrather than panicking; capability filtering defaults open; the M5.1 aggregator split is a pure refactor with an identical registered-controller set.tinyhumansai/tinycortex#memory-provider-apimust merge first. The gitlink points at a branch commit; until it lands on that repo'smain, this PR is not buildable from a clean clone.chore(core): remove unused all module-style messages that do not describe real work. Worth squashing to the milestone commits before review.Nothing here is known-broken — every milestone was independently verified with command output — but points 1 and 2 mean it should not merge yet.