feat(memory): install tinymemory and source driver admission from it - #5487
Merged
Conversation
Add the tinymemory library as a vendored dependency via a git submodule to provide memory management utilities used by the project. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the tinymemory dependency to a new version, which includes improvements to memory binding behavior. The Cargo.lock and Cargo.toml files were updated accordingly to reflect the new dependency version. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the embedded memory driver encounters a binding that is not present in its registry, it now returns an appropriate error instead of panicking or silently failing. This ensures predictable error handling when the driver is used with an incomplete or mismatched binding configuration. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds vendor/tinymemory as a submodule and moves the memory subsystem's driver-admission rules onto it. Those rules are the one part of binding with the same correct answer for every host: a built-in id's class is fixed and an explicit class line may only confirm it, an unknown id is refused rather than guessed, and an external driver is fail-closed on trust. binding.rs keeps what is genuinely host-side -- the per-workspace cache, the conversion into the kernel's generic driver vocabulary, and guard construction -- and becomes a projection from this host's config shape onto the crate's decision. Only class and trust_state cross; credential_ref and endpoint cannot, because DriverEntry has no such fields. That makes 'no secret can appear in an operator-facing refusal' structural rather than a rule someone has to remember. EMBEDDED_DRIVER_ID is now re-exported from the registry rather than declared a second time. Two string literals that must agree is exactly the pair that silently stops agreeing, and the failure is quiet: admit would stop recognising the driver and every bind would fall back to the null placeholder, discarding memory writes for the whole run. All 28 binding tests pass unchanged, including the twelve admit rule tests and fallback_reason_never_contains_credential_ref_or_endpoint. The dependency is a path dep with no [patch.crates-io] entry, so app/src-tauri/Cargo.toml needs no change. tinymemory-tinycortex names tinycortex by version requirement, so this checkout's existing patch unifies it onto one engine copy -- verified: exactly one tinycortex resolves. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the tinymemory vendored submodule to CI and release workflows, update the Cargo.lock with the new tinymemory crates, and document the memory subsystem architecture in AGENTS.md and the memory plan specification. This change integrates the engine-neutral memory layer alongside the existing tinycortex engine, enabling future support for multiple memory backends through a shared contract and driver admission system. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe change adds TinyMemory as a vendored dependency, routes memory-driver admission through its shared registry, reuses its canonical driver identifier, documents the boundary with TinyCortex, and updates test and release workflows to initialize the new submodule. ChangesTinyMemory integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OpenHumanBinding
participant TinyMemoryRegistry
participant Kernel
OpenHumanBinding->>TinyMemoryRegistry: submit sanitized DriverEntry
TinyMemoryRegistry->>TinyMemoryRegistry: validate admission and resolve driver class
TinyMemoryRegistry-->>OpenHumanBinding: return admission result
OpenHumanBinding->>Kernel: convert and apply driver classification
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
coderabbitai
Bot
added
memory
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
12 tasks
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
2 participants
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.
Installs
tinymemoryas a submodule and moves the memory subsystem's driver-admission rules onto it. Phase 1 of re-targetingdocs/specs/plan-memory.mdso the engine-neutral memory layer lives in its own repository rather than inside the engine's.Depends on tinyhumansai/tinymemory#1 — merge that first, then this PR's gitlink moves to
main.Why
plan-memory.mdwas written when TinyCortex was the only engine in view, so it namedvendor/tinycortexas the destination for everything that moves. That conflates two different things. Engine internals belong in the engine. The contract, capability negotiation, driver admission, the conformance corpus and the per-engine adapters do not — a second engine (Supermemory, mem0, an HTTP backend) cannot be reached through a crate named after the first.This PR moves the first slice of the second category out and wires it up. No behaviour change.
What changed
Driver admission now comes from
tinymemory::registry.binding.rsloses ~90 lines of decision table and keeps what is genuinely host-side: the per-workspace cache, the conversion into the kernel's genericDriverClass/BoundDrivervocabulary, and guard construction. It becomes a projection from this host's config shape onto the crate's decision.Only
classandtrust_statecross into the crate.credential_refandendpointcannot, becauseDriverEntryhas no such fields — so "no secret can appear in an operator-facing refusal" is structural rather than a rule someone has to remember.fallback_reason_never_contains_credential_ref_or_endpointstill passes.EMBEDDED_DRIVER_IDis re-exported from the registry rather than declared a second time. Two string literals that must agree is exactly the pair that silently stops agreeing, and the failure mode is quiet:admitwould stop recognising the driver and every bind would fall back to the null placeholder — loud in the logs, but discarding memory writes for the whole run.Two contracts, converted at one seam — a deliberate deviation, recorded in the spec.
tinycortex-apiwas not turned into a re-export oftinymemory-api, so OpenHuman stays ontinycortex-apias its contract and the ~200tinycortex_api::references are untouched.tinymemory-tinycortexconverts between the two. The cost is real: values crossing the seam are rebuilt, and the contracts can drift. The mitigation is that every conversion destructures exhaustively, so a field added on either side is a compile error naming it. If atinymemory_api::type ever appears outsidememory/driver/, that is the boundary leaking.Dependency wiring. Path dependency with no
[patch.crates-io]entry, soapp/src-tauri/Cargo.tomlneeds no change — verified by building the Tauri world.tinymemory-tinycortexnamestinycortexby version requirement, so this checkout's existing patch unifies it:cargo metadatashows exactly onetinycortexand onetinycortex-api, both resolved tovendor/tinycortex.CI.
vendor/tinymemoryadded to the two release submodule lists and to the targeted inits inci-lite.yml/test-reusable.yml.--recursivematters — tinymemory pins its own nested engine.vendor/tinybuswas already missing from both release lists; fixed here, since a missing entry fails the same way.Testing
cargo check(core) andcargo check(Tauri shell)cargo test --lib openhuman::memoryopenhuman::memory::bindingadmit_*rule testscargo test --lib core::allcargo test --lib --no-default-features core::cargo check-only smoke lane cannot seePre-existing flakiness, not introduced here. The full
--libsuite is order-dependent in this environment: it fails 1–2 tests per run, a different set each time, clustered inagent::harness::archivist, plus stack overflows inagent::harness::sessionunder the default test stack. I verified this against the merge base withtinymemoryremoved fromCargo.tomlentirely and all three source files reverted — the baseline fails the same way.RUST_MIN_STACK=33554432clears the overflows. None of it touches memory, and every memory-scoped suite above is deterministic and green.Not in this PR
Draining the
memory/ops/*bypasses onto the guard. Reading each allowlist entry's recorded reason, 6 of the 16ops/entries are genuinely drainable, not the ~11 I first estimated — the rest are the guard resolver itself, theactive_memory_clientdefinition site, status/liveness probes, and two test-only scanner matches that are all correctly staying. Each of the 6 needs a typed contract method designed and added totinycortex-api(a different repository), implemented in the driver family, decorated in the guard, and tested. That is its own focused pass.profile_conn()/memory_handle()remain undecoratable, so "impossible to skip by construction" is still not true after this PR.Summary by CodeRabbit
New Features
Bug Fixes
Documentation