From e546845ccbbb4369eecfabc0462e95208f95ddcd Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Thu, 20 Aug 2026 23:10:24 +0300 Subject: [PATCH] fix(memory): align stale capability tests with the pinned-artifact fix PR #5620 corrected ModuleMemoryProvider::capabilities() to advertise only the 13 families the pinned tinymemory v1.0.1 artifact serves, instead of claiming the full 18-family contract (issue #5598's root cause). It updated memory_tests.rs to match but missed three other test files that independently hardcoded the old full-contract expectation, so main's Rust Core Coverage job has been red since that merge: - core::cli_capability::tests::bound_driver_probe_reports_the_default_module_driver - openhuman::memory::binding::tests::module_binding_advertises_every_family - openhuman::memory::ops::provider::tests::bound_driver_status_reports_id_class_contract_and_capabilities Update all three to assert the corrected 13-family set (and, where it strengthens the test, explicitly assert the 5 not-yet-served families are absent), mirroring the reasoning already accepted in #5620. No production code changes. Co-authored-by: Medulla --- src/core/cli_capability_tests.rs | 22 ++++++++++- src/openhuman/memory/binding_tests.rs | 57 +++++++++++++++++++++++---- src/openhuman/memory/ops/provider.rs | 25 ++++++------ 3 files changed, 85 insertions(+), 19 deletions(-) diff --git a/src/core/cli_capability_tests.rs b/src/core/cli_capability_tests.rs index dbd0ad1a95..a1a33bf168 100644 --- a/src/core/cli_capability_tests.rs +++ b/src/core/cli_capability_tests.rs @@ -94,13 +94,33 @@ fn message_never_contains_a_credential_or_endpoint() { #[cfg(feature = "modules")] #[tokio::test] async fn bound_driver_probe_reports_the_default_module_driver() { + // Was asserting `capabilities() == Capabilities::all()`. That encoded + // #5598 as expected: the pinned v1.0.1 tinymemory artifact serves thirteen + // of the contract's eighteen families (missing `chunks`, `episodic`, + // `people`, `profile`, `retrieval`), so claiming the full contract made + // those five answer `UnknownMethod` instead of reporting themselves + // absent. `modules::memory::ARTIFACT_CAPABILITIES` was narrowed to match + // what is actually served (see its module docs); this pins the same + // corrected boundary rather than the stale full-contract claim. let cfg = MemorySubsystemConfig::default(); let binding = binding_for("default", cfg.clone()); assert_eq!( binding.driver_id(), crate::openhuman::memory::binding::MODULE_ID ); - assert_eq!(binding.capabilities(), Capabilities::all()); + let advertised = binding.capabilities(); + assert!(advertised.contains_all(Capabilities::mandatory())); + assert!(advertised.contains(Capability::Tree)); + assert!( + !advertised.contains(Capability::Retrieval), + "the pinned v1.0.1 artifact has no bus member for `retrieval` (#5598)" + ); + assert!(Capabilities::all().contains_all(advertised)); + assert_ne!( + advertised, + Capabilities::all(), + "advertising the whole contract is the #5598 over-claim" + ); } /// The negative control that makes the assertions above mean something. diff --git a/src/openhuman/memory/binding_tests.rs b/src/openhuman/memory/binding_tests.rs index 43e241bfa4..db3948013a 100644 --- a/src/openhuman/memory/binding_tests.rs +++ b/src/openhuman/memory/binding_tests.rs @@ -361,21 +361,64 @@ fn module_class_binds_the_module_driver_not_null() { } #[test] -fn module_binding_advertises_every_family() { - // Widened once per M3 step; M3d is the last one. The interesting assertion - // is the second: a *bound* context and an *unbound* one now agree, which - // they did not for the whole of M2/M3a-c. +fn module_binding_advertises_the_pinned_artifacts_families() { + // Was `module_binding_advertises_every_family`, asserting `advertised == + // Capabilities::all()`. That encoded #5598 as expected: the host claimed + // all eighteen contract families while the pinned v1.0.1 artifact serves + // thirteen, so the other five (`people`, `chunks`, `retrieval`, `profile`, + // `episodic`) answered `UnknownMethod` instead of reporting themselves + // absent. `modules::memory::ARTIFACT_CAPABILITIES` was narrowed to match + // what the pinned release actually serves (see its module docs); this + // test now pins the same, corrected boundary instead of the old + // "bound equals unbound-default" coincidence, which no longer holds. let dir = tempfile::tempdir().unwrap(); let binding = for_workspace(dir.path(), &MemorySubsystemConfig::default()).expect("default bind"); let advertised = binding.capabilities(); assert!(advertised.contains_all(Capabilities::mandatory())); - for family in Capability::ALL { + + const ARTIFACT_SERVES: [Capability; 13] = [ + Capability::Core, + Capability::Recall, + Capability::Ingest, + Capability::Documents, + Capability::Tree, + Capability::Entities, + Capability::Graph, + Capability::Diff, + Capability::Goals, + Capability::ToolMemory, + Capability::Sources, + Capability::Maintenance, + Capability::Portability, + ]; + for family in ARTIFACT_SERVES { assert!(advertised.contains(family), "{family} must be advertised"); } - assert_eq!(advertised, Capabilities::all()); - assert_eq!(advertised, unbound_default_capabilities()); + + const NOT_YET_SERVED: [Capability; 5] = [ + Capability::People, + Capability::Chunks, + Capability::Retrieval, + Capability::Profile, + Capability::Episodic, + ]; + for family in NOT_YET_SERVED { + assert!( + !advertised.contains(family), + "{family} has no bus member in the pinned v1.0.1 artifact (#5598); \ + widen this only together with the registry version bump" + ); + } + + // The contract may be ahead of the artifact but never behind it. + assert!(Capabilities::all().contains_all(advertised)); + // Unbound contexts still assume the widest set (deny-by-default is wrong + // pre-boot, per `unbound_default_capabilities`'s own docs) — that is a + // different, deliberately permissive default, not a claim that a bound + // module actually serves everything. + assert_eq!(unbound_default_capabilities(), Capabilities::all()); } #[test] diff --git a/src/openhuman/memory/ops/provider.rs b/src/openhuman/memory/ops/provider.rs index b18e83ebef..8a26826a43 100644 --- a/src/openhuman/memory/ops/provider.rs +++ b/src/openhuman/memory/ops/provider.rs @@ -168,30 +168,33 @@ mod tests { crate::openhuman::memory::api::CONTRACT_VERSION ) ); - // All eighteen families — the thirteen of M3d plus the five this port - // added (`chunks`, `episodic`, `people`, `profile`, `retrieval`). - // Spelled out rather than derived from `Capabilities::all()` on - // purpose: this is the wire surface the frontend reads, so the strings - // themselves are the assertion. A family added to the contract without - // a driver serving it should fail here, not silently widen. + // The thirteen families the pinned v1.0.1 tinymemory artifact actually + // serves — not the eighteen the contract crate declares. This used to + // assert all eighteen, including `chunks`, `episodic`, `people`, + // `profile`, and `retrieval`; that encoded #5598 as expected: those + // five have no bus member in the pinned release, so calling them + // answered `UnknownMethod` rather than reporting themselves absent. + // `modules::memory::ARTIFACT_CAPABILITIES` was narrowed to match what + // is actually served (see its module docs); this pins the same + // corrected boundary. Spelled out rather than derived from + // `Capabilities::all()` on purpose: this is the wire surface the + // frontend reads, so the strings themselves are the assertion. A + // family the pinned artifact starts serving should widen this + // deliberately, together with the registry version bump — not + // silently. assert_eq!( status.capabilities, vec![ - "chunks", "core", "diff", "documents", "entities", - "episodic", "goals", "graph", "ingest", "maintenance", - "people", "portability", - "profile", "recall", - "retrieval", "sources", "tool_memory", "tree"