From e0de0a62c3063267cc3a0edd61c5ed01be2c0100 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 03:08:44 +0530 Subject: [PATCH 1/6] test(memory): restore host coverage for three RPC handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `status_list_rpc`, `sync_status_rpc` and `add_rpc` had no test naming them. They are not leftovers from a removed domain — all three are registered controllers the desktop app calls, and what they lost was coverage of **host policy the engine never owned**, so nothing upstream replaces it. They were exercised only by `tests/raw_coverage/memory_threads_*` and `memory_sync_tree_round21_*`, 4,800-line targets that drove ~40 subjects at once through an in-process engine. #6170 deleted those with the engine, which was right for most of what they asserted and wrong for this. **`status_list_rpc`** degrades every failure path to an empty list — a binding error, a driver that does not serve `SourceSync`, and a driver call that fails. Its comment says this is inherited and deliberate: "this surface renders a status table, and every caller of it today treats 'no rows' as 'nothing syncing'". Two of the three are covered here; the third needs a poisoned binding cache, which no seam exposes. **`sync_status_rpc`** filters to ACTIVE slack connections and answers a fixed zero-value shape because per-connection detail is no longer readable. The four zeros are asserted individually rather than as a struct compare, since each is a separate promise and a literal would hide which one broke. The log line is asserted too: the zeros are indistinguishable from "a connection that has synced nothing yet", so without it the shape is a silent lie. **`add_rpc`** mints the `src_` id and applies per-kind caps before the registry sees the entry. `apply_kind_defaults` is `tinymemory-sources`' and upstream's to test; what belongs here is that this handler *calls* it, so an add whose caps the user left unset cannot reach the registry uncapped. Every assertion is mutation-tested — the behaviour broken, the test watched go red, the behaviour restored: degradation → surface the driver error → red filter → drop the is_active check → red shape → change per_channel_cursors → red defaults → skip apply_kind_defaults → red (left: None, right: Some(10)) That is the issue's own acceptance criterion, and it is there because #6170 shipped three assertions that could not fail — two of them passing without the driver being reached at all. Two fixtures worth naming. `sync_status_rpc` needs connections back, so it uses wiremock against `config.api_url`, the pattern `file_storage/tools_tests.rs` already sets for `/agent-integrations/*` — local only, no egress. `add_rpc` writes through a process-global config loader, so its test pins `OPENHUMAN_WORKSPACE` while holding the crate's `TEST_ENV_LOCK`, which is `documents_tests.rs`'s existing pattern and serialises rather than races. Closes #6172 --- src/openhuman/memory/sources/rpc_tests.rs | 123 +++++++++ .../composio/providers/slack/rpc_tests.rs | 98 +++++++ .../memory/sync/sync_status/rpc_tests.rs | 240 ++++++++++++++++++ 3 files changed, 461 insertions(+) diff --git a/src/openhuman/memory/sources/rpc_tests.rs b/src/openhuman/memory/sources/rpc_tests.rs index 04f71bacca..23b33bbd1f 100644 --- a/src/openhuman/memory/sources/rpc_tests.rs +++ b/src/openhuman/memory/sources/rpc_tests.rs @@ -163,3 +163,126 @@ fn composio_rows_dispatch_to_the_connector_and_everything_else_to_the_driver() { "the refusal must name the row and say what to do about it, got: {error}" ); } + +// ── what `add_rpc` owns ───────────────────────────────────────────────────── +// +// The handler generates the source id, maps the request into a +// `MemorySourceEntry`, and applies conservative per-kind caps before the +// registry sees it. Only the first two are its own code; the third is +// `tinymemory-sources`' `apply_kind_defaults`, and what belongs here is that +// this handler *calls* it — an add whose caps the user left unset must not +// reach the registry uncapped. +// +// Covered only by `tests/raw_coverage/memory_threads_raw_coverage_e2e.rs` +// before now, which went with the engine (#6161) although none of this is the +// engine's (#6172). + +/// Pins `OPENHUMAN_WORKSPACE` for the duration, holding the crate's env lock so +/// concurrent tests cannot observe the change. `add_rpc` writes through +/// `registry::add_source`, which resolves its config with +/// `load_config_with_timeout` — process-global, so the workspace has to be +/// pinned rather than passed. +struct WorkspaceEnvGuard { + _env_lock: std::sync::MutexGuard<'static, ()>, + previous: Option, +} + +impl WorkspaceEnvGuard { + fn pin(workspace: &std::path::Path) -> Self { + let env_lock = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let previous = std::env::var_os("OPENHUMAN_WORKSPACE"); + std::env::set_var("OPENHUMAN_WORKSPACE", workspace); + Self { + _env_lock: env_lock, + previous, + } + } +} + +impl Drop for WorkspaceEnvGuard { + fn drop(&mut self) { + match self.previous.take() { + Some(value) => std::env::set_var("OPENHUMAN_WORKSPACE", value), + None => std::env::remove_var("OPENHUMAN_WORKSPACE"), + } + } +} + +fn github_add_request() -> AddRequest { + AddRequest { + kind: tinymemory_sources::types::SourceKind::GithubRepo, + label: "A repository".into(), + enabled: true, + toolkit: None, + connection_id: None, + path: None, + glob: None, + url: Some("https://github.invalid/owner/repo".into()), + branch: None, + paths: Vec::new(), + // The three the caller left unset, which is the whole point. + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + } +} + +#[tokio::test] +async fn add_generates_an_id_and_caps_a_request_that_left_its_limits_unset() { + let tmp = tempfile::TempDir::new().expect("tempdir"); + let workspace = tmp.path().join("workspace"); + std::fs::create_dir_all(&workspace).expect("workspace dir"); + let _env = WorkspaceEnvGuard::pin(&workspace); + + let added = add_rpc(github_add_request()) + .await + .expect("add_rpc") + .value + .source; + + // ── the id is the handler's ───────────────────────────────────────────── + // + // The caller never supplies one; a request that could name its own id would + // let two sources collide by construction. + assert!( + added.id.starts_with("src_") && added.id.len() == "src_".len() + 32, + "the handler must mint a `src_` id, got {:?}", + added.id + ); + + // ── the caps came from somewhere ──────────────────────────────────────── + // + // Asserted as "no longer None" plus the concrete GitHub values, because the + // two say different things: the first is that this handler applies defaults + // at all, the second that it applied *these* — a handler that filled them + // with zeros would satisfy the first and cap every sync at nothing. + assert_eq!(added.max_prs, Some(10), "per-kind PR cap"); + assert_eq!(added.max_issues, Some(10), "per-kind issue cap"); + assert_eq!(added.max_commits, Some(50), "per-kind commit cap"); + + // ── and it reached the registry ───────────────────────────────────────── + // + // The response alone would be satisfied by a handler that shaped an entry + // and dropped it. + let fetched = get_rpc(GetRequest { + id: added.id.clone(), + }) + .await + .expect("get_rpc") + .value + .source; + assert_eq!( + fetched.map(|s| s.id), + Some(added.id.clone()), + "the added source is not readable back from the registry" + ); +} diff --git a/src/openhuman/memory/sync/composio/providers/slack/rpc_tests.rs b/src/openhuman/memory/sync/composio/providers/slack/rpc_tests.rs index f051d6fdcb..dd72bbb6a3 100644 --- a/src/openhuman/memory/sync/composio/providers/slack/rpc_tests.rs +++ b/src/openhuman/memory/sync/composio/providers/slack/rpc_tests.rs @@ -93,3 +93,101 @@ async fn list_slack_connections_resolves_direct_variant_when_mode_is_direct() { // a valid empty envelope), that's also acceptable — the // factory still routed correctly. } + +// ── the status surface's degraded shape ───────────────────────────────────── +// +// `sync_status_rpc` does two things nothing asserted: it filters the connection +// list to slack rows that are ACTIVE, and it answers a fixed zero-value shape +// because per-connection sync detail is no longer readable — the connector +// module keeps its cursor internally. +// +// The deleted `slack_sync_status_rpc_reports_the_degraded_zero_value_shape` +// covered exactly this, in `tests/raw_coverage/memory_sync_tree_round21_*`, +// which went with the engine (#6161) although the assertion was never about the +// engine. Restored here against a local mock backend (#6172). + +#[tokio::test] +async fn status_filters_to_active_slack_and_reports_the_degraded_zero_value_shape() { + use crate::openhuman::security::credentials::{ + AuthService, APP_SESSION_PROVIDER, DEFAULT_AUTH_PROFILE_NAME, + }; + use serde_json::json; + use std::collections::HashMap; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/agent-integrations/composio/connections")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "success": true, + "data": { "connections": [ + // The one row that must survive both filters. + { "id": "conn-slack-active", "toolkit": "slack", "status": "ACTIVE" }, + // Dropped by the status filter… + { "id": "conn-slack-pending", "toolkit": "slack", "status": "PENDING" }, + // …and this one by the toolkit filter. + { "id": "conn-gmail-active", "toolkit": "gmail", "status": "ACTIVE" }, + ] } + }))) + .mount(&server) + .await; + + let tmp = tempfile::tempdir().expect("tempdir"); + let mut config = Config { + config_path: tmp.path().join("config.toml"), + workspace_dir: tmp.path().join("workspace"), + action_dir: tmp.path().join("workspace"), + ..Config::default() + }; + config.secrets.encrypt = false; + config.api_url = Some(server.uri()); + std::fs::create_dir_all(&config.workspace_dir).expect("workspace dir"); + AuthService::from_config(&config) + .store_provider_token( + APP_SESSION_PROVIDER, + DEFAULT_AUTH_PROFILE_NAME, + "test-session-token", + HashMap::new(), + true, + ) + .expect("store app session token"); + + let outcome = sync_status_rpc(&config, SyncStatusRequest::default()) + .await + .expect("status rpc"); + + // ── the filter ────────────────────────────────────────────────────────── + assert_eq!( + outcome.value.connections.len(), + 1, + "only the ACTIVE slack connection qualifies; got {:?}", + outcome.value.connections + ); + let row = &outcome.value.connections[0]; + assert_eq!(row.connection_id, "conn-slack-active"); + + // ── the degraded shape ────────────────────────────────────────────────── + // + // Four fixed zero values, asserted individually rather than as a struct + // comparison: each one is a separate promise to the status table, and a + // struct literal would hide which of them a future change broke. + assert_eq!(row.per_channel_cursors, "{}"); + assert_eq!(row.synced_ids_count, 0); + assert_eq!(row.requests_used_today, 0); + assert_eq!(row.daily_request_limit, 0); + + // ── and the log that explains it ──────────────────────────────────────── + // + // The zeros are indistinguishable from "a connection that has synced + // nothing yet", so the log line is what tells an operator the detail is + // gone rather than empty. Without it the shape above is a silent lie. + assert!( + outcome + .logs + .iter() + .any(|line| line.contains("connections=1") && line.contains("no longer available")), + "the status log must explain the degraded read: {:?}", + outcome.logs + ); +} diff --git a/src/openhuman/memory/sync/sync_status/rpc_tests.rs b/src/openhuman/memory/sync/sync_status/rpc_tests.rs index c24d47601d..5c2329ed91 100644 --- a/src/openhuman/memory/sync/sync_status/rpc_tests.rs +++ b/src/openhuman/memory/sync/sync_status/rpc_tests.rs @@ -10,3 +10,243 @@ fn response_keeps_top_level_statuses_array() { .get("statuses") .is_some_and(serde_json::Value::is_array)); } + +// ── the degradation contract ──────────────────────────────────────────────── +// +// `status_list_rpc` answers `Ok` with an empty list on **every** failure path: +// the binding failing to resolve, the bound driver not serving `SourceSync`, +// and the driver's own call returning an error. Its comment says this is +// inherited behaviour kept deliberately — "this surface renders a status table, +// and every caller of it today treats 'no rows' as 'nothing syncing'" — and +// that tightening it belongs with whoever owns that screen. +// +// Nothing asserted any of the three. They were covered only by +// `tests/raw_coverage/memory_threads_raw_coverage_e2e.rs`, a 4,800-line target +// that drove forty subjects through an in-process engine and went with it +// (#6161). A later "tighten this into an RPC error" would turn a polled status +// table into a visible failure, and no test would have objected (#6172). + +use crate::openhuman::config::Config; +use crate::openhuman::memory::api::provider::MemoryProvider; +use crate::openhuman::memory::binding; +use std::sync::Arc; +use tempfile::TempDir; +use tinymemory_api::null::NullMemoryProvider; + +fn degradation_config() -> (TempDir, Config) { + let tmp = TempDir::new().expect("tempdir"); + let mut cfg = Config::default(); + cfg.workspace_dir = tmp.path().to_path_buf(); + cfg.config_path = tmp.path().join("config.toml"); + (tmp, cfg) +} + +/// A driver that serves `SourceSync` and fails the call — the third path, which +/// neither the null driver nor a healthy one can produce. +#[derive(Debug, Default)] +struct FailingSourceSync { + /// Proves the handler reached this driver. An empty list is also what an + /// *unreached* driver yields, so the assertion below would pass without it + /// even if the binding never resolved here. + calls: std::sync::Mutex, +} + +#[async_trait::async_trait] +impl tinymemory_api::provider::MemorySourceSync for FailingSourceSync { + async fn sync_statuses( + &self, + ) -> Result, tinymemory_api::error::MemoryError> + { + *self.calls.lock().expect("calls lock") += 1; + Err(tinymemory_api::error::MemoryError::Backend( + "the backend is unreachable".into(), + )) + } + + async fn source_sync_state( + &self, + _toolkit: &str, + _connection_id: &str, + ) -> Result, tinymemory_api::error::MemoryError> + { + Ok(None) + } + + async fn run_connection_sync( + &self, + _toolkit: &str, + _connection_id: &str, + ) -> Result { + Ok(tinymemory_api::provider::SyncRunOutcome::default()) + } + + async fn sync_audit_log( + &self, + _limit: Option, + ) -> Result, tinymemory_api::error::MemoryError> + { + Ok(Vec::new()) + } + + async fn estimate_sync_cost_usd( + &self, + _input_tokens: u64, + _output_tokens: u64, + ) -> Result { + Ok(0.0) + } + + async fn raw_archive_coverage( + &self, + _tree_scope: &str, + _archive_source_id: &str, + ) -> Result + { + Ok(tinymemory_api::provider::RawArchiveCoverage::default()) + } + + async fn rebuild_from_raw_archive( + &self, + _tree_scope: &str, + _archive_source_id: &str, + ) -> Result + { + Ok(tinymemory_api::provider::RawRebuildOutcome::default()) + } +} + +#[async_trait::async_trait] +impl tinymemory_api::provider::MemoryCore for FailingSourceSync { + async fn store( + &self, + _namespace: &str, + _key: &str, + _content: &str, + _category: tinymemory_api::types::MemoryCategory, + _session_id: Option<&str>, + _taint: tinymemory_api::types::MemoryTaint, + ) -> Result<(), tinymemory_api::error::MemoryError> { + Ok(()) + } + async fn get( + &self, + _namespace: &str, + _key: &str, + ) -> Result, tinymemory_api::error::MemoryError> + { + Ok(None) + } + async fn forget( + &self, + _namespace: &str, + _key: &str, + ) -> Result { + Ok(false) + } + async fn namespaces( + &self, + ) -> Result, tinymemory_api::error::MemoryError> + { + Ok(Vec::new()) + } + async fn list( + &self, + _namespace: Option<&str>, + _category: Option<&tinymemory_api::types::MemoryCategory>, + _session_id: Option<&str>, + ) -> Result, tinymemory_api::error::MemoryError> { + Ok(Vec::new()) + } +} + +#[async_trait::async_trait] +impl tinymemory_api::provider::MemoryRecall for FailingSourceSync { + async fn recall( + &self, + _query: &str, + _limit: usize, + _opts: &tinymemory_api::recall::OwnedRecallOpts, + _scope: Option<&tinymemory_api::provider::SourceScope>, + ) -> Result, tinymemory_api::error::MemoryError> { + Ok(Vec::new()) + } +} + +#[async_trait::async_trait] +impl tinymemory_api::provider::MemoryPortability for FailingSourceSync { + async fn export_page( + &self, + _cursor: Option<&str>, + _limit: usize, + ) -> Result { + Ok(tinymemory_api::provider::ExportPage::default()) + } + async fn import_records( + &self, + _records: Vec, + ) -> Result { + Ok(tinymemory_api::provider::ImportOutcome::default()) + } +} + +#[async_trait::async_trait] +impl MemoryProvider for FailingSourceSync { + fn driver_id(&self) -> &str { + "failing-source-sync-test-driver" + } + fn capabilities(&self) -> tinymemory_api::capabilities::Capabilities { + tinymemory_api::capabilities::Capabilities::mandatory() + } + async fn health(&self) -> tinymemory_api::health::MemoryHealth { + tinymemory_api::health::MemoryHealth::Ready + } + fn as_source_sync(&self) -> Option<&dyn tinymemory_api::provider::MemorySourceSync> { + Some(self) + } +} + +/// Path two: the driver is bound and does not serve `SourceSync`. +#[tokio::test] +async fn a_driver_without_the_family_reports_an_empty_table_rather_than_an_error() { + let (_tmp, cfg) = degradation_config(); + binding::install_for_test( + &cfg.workspace_dir, + &cfg.subsystems.memory, + Arc::new(NullMemoryProvider::new()) as Arc, + ); + + let outcome = status_list_rpc(&cfg) + .await + .expect("an unserved family must not surface as an RPC error"); + assert!( + outcome.value.statuses.is_empty(), + "the table is empty, not broken" + ); +} + +/// Path three: the driver serves the family and the call fails. +/// +/// The one a null driver cannot produce, and the one most likely to be +/// "tightened" by someone who has not read the comment — a backend blip is +/// exactly what looks like it deserves an error. +#[tokio::test] +async fn a_driver_error_is_degraded_to_an_empty_table_not_surfaced() { + let (_tmp, cfg) = degradation_config(); + let driver = Arc::new(FailingSourceSync::default()); + binding::install_for_test( + &cfg.workspace_dir, + &cfg.subsystems.memory, + Arc::clone(&driver) as Arc, + ); + + let outcome = status_list_rpc(&cfg) + .await + .expect("a driver-side failure must not surface as an RPC error"); + assert!(outcome.value.statuses.is_empty()); + assert_eq!( + *driver.calls.lock().expect("calls lock"), + 1, + "the handler must have reached the driver — an empty table from an \ + unreached driver would prove nothing about the degradation" + ); +} From 5b3b947143eabfd530ae193ffd20b7ab5fba61ba Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 04:03:05 +0530 Subject: [PATCH 2/6] chore(modules): re-pin tinymemory to the v1.15.3 release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openhuman#6170 declared a deliberate pin drift: the host compiled the memory contract against tinymemory main while the registry kept the published v1.15.2 artifact, because no release contained tinymemory#148, #150 and #151 yet. v1.15.3 now does, together with #146 (the explicit document-id upsert openhuman#6147 needs) and #152. All five pin sites move together, which is what `check-module-pins.mjs` enforces: the `TINYMEMORY` record's version, release_url and eleven archive names with the digests taken verbatim from the release's `checksum.toml`; `ARTIFACT_CAPABILITIES_PIN`; the four `memory_version`/ `memory_sha256` blocks across ci-full, ci-lite and e2e-reusable (twice); and the `vendor/tinymemory` gitlink, which now sits on the tag itself (`git describe --tags` = v1.15.3). The exemption goes with them — an exemption that has stopped being true fails the gate as surely as one that was never declared. The submodule move is forward-only: 5c55431 is an ancestor of the tag. `node scripts/ci/check-module-pins.mjs` passes, `cargo metadata --locked` is unchanged (no vendored crate version moved), and the capability list `bound_driver_status_reports_id_class_contract_and_capabilities` pins against the artifact is identical under the new release. --- .github/workflows/ci-full.yml | 4 +- .github/workflows/ci-lite.yml | 4 +- .github/workflows/e2e-reusable.yml | 8 ++-- scripts/ci/module-pin-exemptions.json | 6 --- src/openhuman/modules/memory_part_01.rs | 2 +- src/openhuman/modules/registry_part_01.rs | 48 +++++++++++------------ vendor/tinymemory | 2 +- 7 files changed, 34 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci-full.yml b/.github/workflows/ci-full.yml index 2406375a3e..2a648f6926 100644 --- a/.github/workflows/ci-full.yml +++ b/.github/workflows/ci-full.yml @@ -132,8 +132,8 @@ jobs: # and leave a stale digest that only fails after the download. Both values # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`, # which is the authoritative pin — copy them from there, never recompute. - memory_version="1.15.2" - memory_sha256="32eb22f09c2ede0981a8462e3e4438db40455a6369ae5acc577f8d4493f1e84a" + memory_version="1.15.3" + memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e" module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" memory_dir="$module_root/tinymemory" juice_dir="$module_root/tinyjuice" diff --git a/.github/workflows/ci-lite.yml b/.github/workflows/ci-lite.yml index a6f5707131..e15253c654 100644 --- a/.github/workflows/ci-lite.yml +++ b/.github/workflows/ci-lite.yml @@ -831,8 +831,8 @@ jobs: # and leave a stale digest that only fails after the download. Both values # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`, # which is the authoritative pin — copy them from there, never recompute. - memory_version="1.15.2" - memory_sha256="32eb22f09c2ede0981a8462e3e4438db40455a6369ae5acc577f8d4493f1e84a" + memory_version="1.15.3" + memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e" module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" memory_dir="$module_root/tinymemory" juice_dir="$module_root/tinyjuice" diff --git a/.github/workflows/e2e-reusable.yml b/.github/workflows/e2e-reusable.yml index e7fce4dd20..1b440b9513 100644 --- a/.github/workflows/e2e-reusable.yml +++ b/.github/workflows/e2e-reusable.yml @@ -168,8 +168,8 @@ jobs: # and leave a stale digest that only fails after the download. Both values # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`, # which is the authoritative pin — copy them from there, never recompute. - memory_version="1.15.2" - memory_sha256="32eb22f09c2ede0981a8462e3e4438db40455a6369ae5acc577f8d4493f1e84a" + memory_version="1.15.3" + memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e" module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" memory_dir="$module_root/tinymemory" memory_archive="$memory_dir/tinymemory-module-${memory_version}-ubuntu-22.04-x86_64.tar.gz" @@ -374,8 +374,8 @@ jobs: # and leave a stale digest that only fails after the download. Both values # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`, # which is the authoritative pin — copy them from there, never recompute. - memory_version="1.15.2" - memory_sha256="32eb22f09c2ede0981a8462e3e4438db40455a6369ae5acc577f8d4493f1e84a" + memory_version="1.15.3" + memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e" module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" memory_dir="$module_root/tinymemory" memory_archive="$memory_dir/tinymemory-module-${memory_version}-ubuntu-22.04-x86_64.tar.gz" diff --git a/scripts/ci/module-pin-exemptions.json b/scripts/ci/module-pin-exemptions.json index 78b09f7619..09e0d2f576 100644 --- a/scripts/ci/module-pin-exemptions.json +++ b/scripts/ci/module-pin-exemptions.json @@ -29,12 +29,6 @@ "submodule": "vendor/tinymcp", "expect": "v0.3.2-2-g8b0627d", "reason": "The host compiles the MCP contract against tinyhumansai/tinymcp#13 (Supervisor::tick returns a TickReport; needed by openhuman#5931), merged to tinymcp main but not yet in a tagged release, while the registry keeps the published v0.3.2 artifact. The drift is compile-only: the tinymcp module is registry-entered but not wired (AGENTS.md, 'step two of the extraction'), so no build downloads or loads that artifact. Delete this entry when tinymcp cuts its next release and the registry pin moves onto it." - }, - { - "id": "tinymemory", - "submodule": "vendor/tinymemory", - "expect": "v1.15.2-15-g5c55431", - "reason": "The host compiles the memory contract against tinymemory main (tinyhumansai/tinymemory#148, #150 and #151 \u2014 the engine-free conformance driver openhuman#6161 binds in place of an in-process engine), merged upstream but not yet in a tagged release, while the registry keeps the published v1.15.2 artifact. The drift cannot reach a runtime mismatch, and that was checked rather than assumed: the entire v1.15.2..5c55431 delta to the crates this build ships is (a) one additive re-export, `pub use tinymemory_bus::chrono` in tinymemory-api, so a driver crate depending on the contract alone can name the `DateTime` two MemoryTree methods already take, (b) a `#[cfg(test)] mod summarise_tests;` line in tinymemory-core, and (c) 31 lines of doc comment on MemoryDocuments::list_documents and delete_document. Zero removals, no method added or changed, no wire slot moved, CONTRACT_VERSION untouched \u2014 so the v1.15.2 artifact serves exactly the contract compiled here. Everything else in the range is tinymemory-conformance, a test-only crate that is a dev-dependency here and is not in the module artifact at all. Delete this entry when tinymemory cuts its next release and the registry pin moves onto it." } ] } diff --git a/src/openhuman/modules/memory_part_01.rs b/src/openhuman/modules/memory_part_01.rs index b6439664d3..b923388a81 100644 --- a/src/openhuman/modules/memory_part_01.rs +++ b/src/openhuman/modules/memory_part_01.rs @@ -8,7 +8,7 @@ use tinymemory_api::capabilities::{Capabilities, Capability}; /// Checked against the registry pin by `the_capability_list_matches_the_pinned_release`, /// so bumping the pin without re-reading the list is a red test rather than a /// silent over-claim. -pub(crate) const ARTIFACT_CAPABILITIES_PIN: &str = "1.15.2"; +pub(crate) const ARTIFACT_CAPABILITIES_PIN: &str = "1.15.3"; /// The capability families the **pinned artifact** actually serves. /// diff --git a/src/openhuman/modules/registry_part_01.rs b/src/openhuman/modules/registry_part_01.rs index 40fd1ccf9d..ee9eaebd6e 100644 --- a/src/openhuman/modules/registry_part_01.rs +++ b/src/openhuman/modules/registry_part_01.rs @@ -175,63 +175,63 @@ const TINYMEMORY: ModuleRecord = ModuleRecord { description: "Local memory engine: store, ranked recall, and portable export", bus_name: "ai.tinyhumans.tinymemory.Memory", object_path: "/ai/tinyhumans/tinymemory/Memory", - version: "1.15.2", - release_url: "https://github.com/tinyhumansai/tinymemory/releases/tag/v1.15.2", + version: "1.15.3", + release_url: "https://github.com/tinyhumansai/tinymemory/releases/tag/v1.15.3", assets: &[ PlatformAsset { host_key: "ubuntu-24.04-x86_64", - archive: "tinymemory-module-1.15.2-ubuntu-24.04-x86_64.tar.gz", - sha256: "30425c82be486c1166bb4d2a5c13329b698de488eaf204ab32fdda150f4a5e17", + archive: "tinymemory-module-1.15.3-ubuntu-24.04-x86_64.tar.gz", + sha256: "cdf1bc2f1deb32f7d52d5c0caa235ee28be458ce10a1dc69f2c0e611bed15b8d", }, PlatformAsset { host_key: "ubuntu-24.04-arm64", - archive: "tinymemory-module-1.15.2-ubuntu-24.04-arm64.tar.gz", - sha256: "36e940c480ebe585a0a3b8957dd63eae1a66492023887ba39e6daab48db22dbf", + archive: "tinymemory-module-1.15.3-ubuntu-24.04-arm64.tar.gz", + sha256: "cb4e135a4be90953f277fba311e056d46d44d15db5fb027cbe83bee6247f0995", }, PlatformAsset { host_key: "ubuntu-22.04-x86_64", - archive: "tinymemory-module-1.15.2-ubuntu-22.04-x86_64.tar.gz", - sha256: "32eb22f09c2ede0981a8462e3e4438db40455a6369ae5acc577f8d4493f1e84a", + archive: "tinymemory-module-1.15.3-ubuntu-22.04-x86_64.tar.gz", + sha256: "5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e", }, PlatformAsset { host_key: "ubuntu-22.04-arm64", - archive: "tinymemory-module-1.15.2-ubuntu-22.04-arm64.tar.gz", - sha256: "261eab717806dcd6e79c5b500aefe1f523913cac2e21e39c3b92c8a00afb1b8e", + archive: "tinymemory-module-1.15.3-ubuntu-22.04-arm64.tar.gz", + sha256: "1992869bdea71ca7a96d5a7e0e32e8abc659a8bbb16d0cf34b2ba4440ca6892d", }, PlatformAsset { host_key: "macos-26-arm64", - archive: "tinymemory-module-1.15.2-macos-26-arm64.tar.gz", - sha256: "f09919269b005ab40e3ef251b01e615409efda3b770d62c05a982b72bb829fc0", + archive: "tinymemory-module-1.15.3-macos-26-arm64.tar.gz", + sha256: "63c64b35aa9364f2fbc7d83eb8dbab2895352441223f30a4e6226aa479c7563d", }, PlatformAsset { host_key: "macos-26-x86_64", - archive: "tinymemory-module-1.15.2-macos-26-x86_64.tar.gz", - sha256: "8b2deb7fe4b28a07f4ff6d55c5727063c888521ebe38b862a9e9f5444e1558fd", + archive: "tinymemory-module-1.15.3-macos-26-x86_64.tar.gz", + sha256: "61cc6b44627d11344957ff47a5047f2e25068d1d9fa158a08dbc37fbc78bc1ef", }, PlatformAsset { host_key: "macos-15-arm64", - archive: "tinymemory-module-1.15.2-macos-15-arm64.tar.gz", - sha256: "3e9169703242d1d21dc8fda185d809faee0f3d16ca1607e3656dbcdaf213999c", + archive: "tinymemory-module-1.15.3-macos-15-arm64.tar.gz", + sha256: "4feed28d2bd4b8025ce975d3518014c85ff002bce4ca5ab7c9cfd923dc5f7686", }, PlatformAsset { host_key: "macos-15-x86_64", - archive: "tinymemory-module-1.15.2-macos-15-x86_64.tar.gz", - sha256: "b764d373545120f22bb2e33c6b2444585153fc965c7245201f311dc13931a318", + archive: "tinymemory-module-1.15.3-macos-15-x86_64.tar.gz", + sha256: "bf567c16b0d26c8b00b6d5e4b44e37bfeec820dd5dcb7e941505859a87a3fa8c", }, PlatformAsset { host_key: "windows-2025-x86_64", - archive: "tinymemory-module-1.15.2-windows-2025-x86_64.zip", - sha256: "889ba7ee12e42ef68c2dd8a73c912b1512f349dd7e4c4ddc3b0f57c0094be4ca", + archive: "tinymemory-module-1.15.3-windows-2025-x86_64.zip", + sha256: "e741da2d02ee3b2bd99b7bbada298b92bfa3b233d903e05290a4a25cc3b68d62", }, PlatformAsset { host_key: "windows-2022-x86_64", - archive: "tinymemory-module-1.15.2-windows-2022-x86_64.zip", - sha256: "6deaaf9efca8afcddf2ae16d0c37319455add32c04f8c14920665232e6bb4b94", + archive: "tinymemory-module-1.15.3-windows-2022-x86_64.zip", + sha256: "ad93120dcfeab2dcbf2d90bdadab220a2de46e087461e39d2203d483d868fb85", }, PlatformAsset { host_key: "windows-11-arm64", - archive: "tinymemory-module-1.15.2-windows-11-arm64.zip", - sha256: "743daee97da22e401da74b303c952dfef6b9c3e06213145bf12c2b2ae21f5c66", + archive: "tinymemory-module-1.15.3-windows-11-arm64.zip", + sha256: "0242470cde6e802e93eedeb55ed8722da9483740fee8c0e5e60555a67d7b6da3", }, ], // Eager, unlike the two codecs above. A codec that is never asked for should diff --git a/vendor/tinymemory b/vendor/tinymemory index 5c55431efd..e8b6f36739 160000 --- a/vendor/tinymemory +++ b/vendor/tinymemory @@ -1 +1 @@ -Subproject commit 5c55431efd3015d05719aae6ad47f25b3b288f24 +Subproject commit e8b6f36739a55698582a130ca0cdf917701710a8 From 1e63524af57961354f2885f90fad262f350e0013 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 04:03:15 +0530 Subject: [PATCH 3/6] test(memory): tighten the add-source assertions, and say why the third path is not here MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the new handler tests. The minted id was checked as `src_` plus thirty-two characters, which `src_zzzz…` satisfies. It is a uuid-simple, so the suffix is now required to be thirty-two lowercase hex digits — the shape a collision argument actually rests on. The registry read-back compared only the id, so a registry that persisted the row with every other field defaulted still passed. It now compares the four fields the request set away from their defaults — kind, label, enabled and url — each of which is a mapping `add_rpc` had to carry. The third finding asked for the remaining degradation path, the binding itself failing to resolve. It is not addable: `binding::for_subtree` returns `Err` only on a poisoned process-wide lock or a raised process-wide exit gate, and `memory::exit`'s own doc gives that as the reason the gate is a type with one production instance rather than a bare static — "a process-wide refusal to bind memory would reach every other test in the same process". A test that raised it would fail every test that binds memory after it. The comment now records that, so the gap reads as a decision rather than an omission. --- src/openhuman/memory/sources/rpc_tests.rs | 37 ++++++++++++++++--- .../memory/sync/sync_status/rpc_tests.rs | 12 ++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/openhuman/memory/sources/rpc_tests.rs b/src/openhuman/memory/sources/rpc_tests.rs index 23b33bbd1f..cf8fffcf6e 100644 --- a/src/openhuman/memory/sources/rpc_tests.rs +++ b/src/openhuman/memory/sources/rpc_tests.rs @@ -253,10 +253,19 @@ async fn add_generates_an_id_and_caps_a_request_that_left_its_limits_unset() { // // The caller never supplies one; a request that could name its own id would // let two sources collide by construction. + let minted = added.id.strip_prefix("src_").unwrap_or_else(|| { + panic!( + "the handler must mint a `src_`-prefixed id, got {:?}", + added.id + ) + }); assert!( - added.id.starts_with("src_") && added.id.len() == "src_".len() + 32, - "the handler must mint a `src_` id, got {:?}", - added.id + minted.len() == 32 + && minted + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)), + "the suffix must be a uuid-simple — 32 lowercase hex digits — and not \ + merely 32 characters, got {minted:?}" ); // ── the caps came from somewhere ──────────────────────────────────────── @@ -280,9 +289,25 @@ async fn add_generates_an_id_and_caps_a_request_that_left_its_limits_unset() { .expect("get_rpc") .value .source; + let fetched = fetched.expect("the added source is not readable back from the registry"); assert_eq!( - fetched.map(|s| s.id), - Some(added.id.clone()), - "the added source is not readable back from the registry" + fetched.id, added.id, + "the registry read back a different source" + ); + + // The id alone would be satisfied by a registry that persisted an entry + // with every other field defaulted. These four are the request's + // non-default fields, so each one is a mapping the handler had to carry. + assert_eq!( + fetched.kind, + tinymemory_sources::types::SourceKind::GithubRepo, + "the request's kind must survive the round trip" + ); + assert_eq!(fetched.label, "A repository", "the request's label"); + assert!(fetched.enabled, "the request asked for an enabled source"); + assert_eq!( + fetched.url.as_deref(), + Some("https://github.invalid/owner/repo"), + "the request's url" ); } diff --git a/src/openhuman/memory/sync/sync_status/rpc_tests.rs b/src/openhuman/memory/sync/sync_status/rpc_tests.rs index 5c2329ed91..78bb7cc8ae 100644 --- a/src/openhuman/memory/sync/sync_status/rpc_tests.rs +++ b/src/openhuman/memory/sync/sync_status/rpc_tests.rs @@ -25,6 +25,18 @@ fn response_keeps_top_level_statuses_array() { // that drove forty subjects through an in-process engine and went with it // (#6161). A later "tighten this into an RPC error" would turn a polled status // table into a visible failure, and no test would have objected (#6172). +// +// Two of the three are asserted below. The first — the binding itself failing +// to resolve — is deliberately not, because it cannot be reached from a unit +// test without damaging the rest of the binary. `binding::for_subtree` returns +// `Err` on exactly two conditions: the process-wide `BINDINGS` lock being +// poisoned, and the process-wide exit gate being raised. `memory::exit`'s own +// doc gives the reason that gate is a type with one production instance rather +// than a bare static — "a process-wide refusal to bind memory would reach every +// other test in the same process" — so a test that raised it would fail every +// test that binds memory after it. The arm is a two-line `warn` and `Vec::new()` +// in `rpc.rs`; what would have to change for it to start propagating is the +// `match` around it, which the two cases below already hold in place. use crate::openhuman::config::Config; use crate::openhuman::memory::api::provider::MemoryProvider; From 7d37c00a6208f1931031e7a06d995651a41a4501 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 04:03:28 +0530 Subject: [PATCH 4/6] test(memory): settle the memory module before asserting a settled answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo test --lib -- openhuman::memory` fails on this branch, and the failure is not this branch's: two assertions race the module load. A module is loaded once per process by whichever caller asks first, and `modules::ops::state_of` reports `Loading` for the whole of it. Two correct answers to that transient are indistinguishable from a regression. `MemoryProvider::health` returns `degraded("the memory module is loading")`, so `bound_driver_status_reports_id_class_contract_and_ capabilities` reads "degraded" where it asserts "ready". A handler that reaches the driver refuses with "memory is still starting", so `search_entities_rpc_rejects_unknown_entity_kind` gets that instead of "unknown entity kind: bogus". Both pass alone and both fail in the nine-hundred-test filter — with this branch's four new tests skipped, which is what places the fault outside them. The filter is derived from the changed paths, so it is assembled only by a PR that touches memory broadly; that is why the flake surfaces here rather than on main. `settle_memory_module` awaits the resolution rather than polling `state_of`, which is the difference between closing the window and narrowing it: `Ready` and `Failed` are both terminal — tinybus keeps a refused library mapped, so a resolution is never retried — so the state cannot return to `Loading` afterwards. The outcome is ignored on purpose, because a host with no artifact for its platform resolves to `Failed`, which is settled too. `binding::test_module_config` is the same configuration the test-build `module_provider` binds through, named so the wait and the binding cannot drift apart. Filter run: 900 passed, 0 failed, three times over; 899/1 before. --- src/openhuman/memory/binding.rs | 37 +++++++++++++------ src/openhuman/memory/ops/provider_tests.rs | 4 ++ src/openhuman/memory/test_support/mod.rs | 37 +++++++++++++++++++ .../tree/retrieval/rpc_tests_part_01_tests.rs | 4 ++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/openhuman/memory/binding.rs b/src/openhuman/memory/binding.rs index 1601763ddd..49c329ec30 100644 --- a/src/openhuman/memory/binding.rs +++ b/src/openhuman/memory/binding.rs @@ -370,17 +370,20 @@ fn module_provider( ) } +/// The configuration every test-build memory binding loads its module through. +/// +/// Unit tests do not run the full boot sequence that publishes the module +/// policy. A native module is loaded once per process and therefore captures +/// the first workspace it receives. Pin every test binding to the same +/// workspace as the process-global test client so concurrent tests cannot +/// win module initialization with an unrelated tempdir and split guarded +/// writes from legacy read-back calls. +/// +/// Named rather than inlined into [`module_provider`] so a test can await this +/// module's resolution through the same configuration the binding will use — +/// see [`crate::openhuman::memory::test_support::settle_memory_module`]. #[cfg(all(feature = "modules", test))] -fn module_provider( - _workspace_dir: &Path, - memory_subdir: &str, -) -> (Arc, DriverClass) { - // Unit tests do not run the full boot sequence that publishes the module - // policy. A native module is loaded once per process and therefore captures - // the first workspace it receives. Pin every test binding to the same - // workspace as the process-global test client so concurrent tests cannot - // win module initialization with an unrelated tempdir and split guarded - // writes from legacy read-back calls. +pub(crate) fn test_module_config() -> crate::openhuman::config::Config { let workspace_dir = crate::openhuman::memory::ops::shared_memory_test_workspace(); let mut config = crate::openhuman::config::Config::default(); config.workspace_dir = workspace_dir.clone(); @@ -394,10 +397,20 @@ fn module_provider( path: path.to_string_lossy().into_owned(), }); } + config +} + +#[cfg(all(feature = "modules", test))] +fn module_provider( + _workspace_dir: &Path, + memory_subdir: &str, +) -> (Arc, DriverClass) { ( Arc::new( - crate::openhuman::modules::memory::ModuleMemoryProvider::new(Arc::new(config)) - .in_subdir(memory_subdir), + crate::openhuman::modules::memory::ModuleMemoryProvider::new(Arc::new( + test_module_config(), + )) + .in_subdir(memory_subdir), ), DriverClass::Module, ) diff --git a/src/openhuman/memory/ops/provider_tests.rs b/src/openhuman/memory/ops/provider_tests.rs index 0d3c9f2f18..16ff10512a 100644 --- a/src/openhuman/memory/ops/provider_tests.rs +++ b/src/openhuman/memory/ops/provider_tests.rs @@ -20,6 +20,10 @@ async fn status_without_a_context_reports_an_unresolved_slot() { #[tokio::test] async fn bound_driver_status_reports_id_class_contract_and_capabilities() { + // `health` below is `degraded` for as long as the module is loading, and + // the load is process-wide: without this the assertion races whichever + // sibling test asked for the module first (openhuman#6172). + crate::openhuman::memory::test_support::settle_memory_module().await; let workspace = tempfile::tempdir().expect("tempdir"); let cfg = crate::openhuman::config::schema::MemorySubsystemConfig::default(); let binding = crate::openhuman::memory::binding::for_workspace(workspace.path(), &cfg) diff --git a/src/openhuman/memory/test_support/mod.rs b/src/openhuman/memory/test_support/mod.rs index 1e943a3739..f32a18428d 100644 --- a/src/openhuman/memory/test_support/mod.rs +++ b/src/openhuman/memory/test_support/mod.rs @@ -256,3 +256,40 @@ impl tinymemory_api::traits::Memory for RetainingMemory { pub(crate) fn retaining_memory() -> Arc { Arc::new(RetainingMemory::default()) } + +/// Wait out any in-flight load of the memory module. +/// +/// A module is loaded once per process, by whichever caller asks first, and +/// `modules::ops::state_of` reports `Loading` for the whole of it. Two correct +/// answers to that transient are indistinguishable from a regression: +/// `MemoryProvider::health` returns `degraded("the memory module is loading")`, +/// and a handler that reaches the driver answers "memory is still starting". +/// A test asserting a settled value therefore races whichever sibling triggered +/// the load — invisible when it runs alone, and lost about as often as not when +/// `cargo test --lib -- openhuman::memory` runs nine hundred of them in one +/// process (openhuman#6172). +/// +/// Awaiting the resolution is what makes this race-free, where polling +/// `state_of` would only narrow the window: `Ready` and `Failed` are both +/// terminal — tinybus keeps a refused library mapped, so a resolution is never +/// retried — which means the state cannot return to `Loading` after this +/// returns. The wait is bounded because a caller that gives up leaves the +/// resolution running rather than cancelling it. +/// +/// The outcome is deliberately ignored: a host with no artifact for its +/// platform resolves to `Failed`, which is settled too, and what the caller +/// asserts about that is the caller's business. +#[cfg(feature = "modules")] +pub(crate) async fn settle_memory_module() { + let _ = crate::openhuman::modules::ops::ensure_loaded_within( + &crate::openhuman::memory::binding::test_module_config(), + crate::openhuman::memory::binding::MODULE_ID, + Some(std::time::Duration::from_secs(30)), + ) + .await; +} + +/// Without the `modules` feature nothing loads a module, so nothing can be +/// caught mid-load. +#[cfg(not(feature = "modules"))] +pub(crate) async fn settle_memory_module() {} diff --git a/src/openhuman/memory/tree/retrieval/rpc_tests_part_01_tests.rs b/src/openhuman/memory/tree/retrieval/rpc_tests_part_01_tests.rs index 3a7db7fa31..23ea50bc6b 100644 --- a/src/openhuman/memory/tree/retrieval/rpc_tests_part_01_tests.rs +++ b/src/openhuman/memory/tree/retrieval/rpc_tests_part_01_tests.rs @@ -268,6 +268,10 @@ async fn search_entities_rpc_parses_valid_kinds_list() { /// index instead. #[tokio::test] async fn search_entities_rpc_rejects_unknown_entity_kind() { + // The handler reaches the bound driver, which refuses with "memory is + // still starting" for as long as the module is loading — a process-wide + // transient this assertion would otherwise race (openhuman#6172). + crate::openhuman::memory::test_support::settle_memory_module().await; let (_tmp, cfg) = test_config(); let req = SearchEntitiesRequest { query: "x".into(), From bd3806c3f060c4bfa385b327cf36eba9eaa936fb Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 04:31:59 +0530 Subject: [PATCH 5/6] test(memory): fail the settle helper when the module does not settle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ensure_loaded_within` has three outcomes, and discarding the result treated all three as success. `StillLoading` — the bound elapsing with the resolution still in flight — is the one that leaves the caller in exactly the state this helper exists to rule out: it would return, the assertion would reach the driver mid-load, and the test would fail on the transient it was supposed to have waited out, now with thirty seconds of latency and no hint of why. It panics instead, naming the cause. `Failed` stays ignored, because a host with no artifact for its platform is settled, and what a caller asserts about a driver that could not load is the caller's business. Filter run unchanged: 900 passed, 0 failed. --- src/openhuman/memory/test_support/mod.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/openhuman/memory/test_support/mod.rs b/src/openhuman/memory/test_support/mod.rs index f32a18428d..d8ad589980 100644 --- a/src/openhuman/memory/test_support/mod.rs +++ b/src/openhuman/memory/test_support/mod.rs @@ -276,17 +276,30 @@ pub(crate) fn retaining_memory() -> Arc { /// returns. The wait is bounded because a caller that gives up leaves the /// resolution running rather than cancelling it. /// -/// The outcome is deliberately ignored: a host with no artifact for its -/// platform resolves to `Failed`, which is settled too, and what the caller -/// asserts about that is the caller's business. +/// `Failed` is ignored: a host with no artifact for its platform resolves to +/// it, that is settled too, and what the caller asserts about a driver that +/// could not load is the caller's business. `StillLoading` is not ignored — +/// it is the one outcome that leaves the caller in exactly the state this +/// function exists to rule out, so it panics rather than handing back an +/// unsettled module and letting the caller fail on the transient it was +/// supposed to have waited out. #[cfg(feature = "modules")] pub(crate) async fn settle_memory_module() { - let _ = crate::openhuman::modules::ops::ensure_loaded_within( + use crate::openhuman::modules::ops::LoadError; + + match crate::openhuman::modules::ops::ensure_loaded_within( &crate::openhuman::memory::binding::test_module_config(), crate::openhuman::memory::binding::MODULE_ID, Some(std::time::Duration::from_secs(30)), ) - .await; + .await + { + Ok(()) | Err(LoadError::Failed(_)) => {} + Err(LoadError::StillLoading) => panic!( + "the memory module did not settle within 30s; every assertion after \ + this point would race its load" + ), + } } /// Without the `modules` feature nothing loads a module, so nothing can be From e563a0586ecf3c50e374e941d77984c6fc356236 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 10 Sep 2026 05:34:41 +0530 Subject: [PATCH 6/6] fix(tests): give the integration-test parent contexts the field #6162 added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ParentExecutionContext` gained `visible_tool_specs` in #6162 (d93180a53). Nine struct literals under `src/` moved with it; ten more live under `tests/` and did not, so every target that builds an integration test fails to compile on main: error[E0063]: missing field `visible_tool_specs` in initializer of `ParentExecutionContext` --> tests/agent_harness_public.rs:95:5 `cargo test --lib` does not build `tests/*.rs`, which is why the gap survived local verification, and #6162's own coverage lane selects its targets from the changed paths, none of which named these files. Every stub gets `Arc::new(Vec::new())`, which is behaviour-preserving rather than merely compiling: `render_parent_tool_catalog` reads `all_tool_specs` whenever the visible set is empty, so an empty field means "the same surface as `all_tool_specs`" — exactly what these contexts described before the parent's visible set became a field of its own. Each site says so, because an empty collection otherwise reads as an oversight. `cargo check --all-targets --features "$(bash scripts/ci/product-features.sh)"` is clean. --- tests/agent_harness_public.rs | 4 ++++ tests/calendar_grounding_e2e.rs | 4 ++++ tests/composio_list_tools_stack_overflow_regression.rs | 4 ++++ .../agent_archivist_debug_round21_raw_coverage_e2e.rs | 4 ++++ .../raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs | 4 ++++ tests/raw_coverage/agent_harness_raw_coverage_e2e.rs | 4 ++++ tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs | 4 ++++ tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs | 4 ++++ tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs | 4 ++++ .../tools_agent_credentials_state_raw_coverage_e2e.rs | 4 ++++ 10 files changed, 40 insertions(+) diff --git a/tests/agent_harness_public.rs b/tests/agent_harness_public.rs index a1ca5e5c94..cffea8ef30 100644 --- a/tests/agent_harness_public.rs +++ b/tests/agent_harness_public.rs @@ -103,6 +103,10 @@ fn stub_parent_context() -> ParentExecutionContext { )), all_tools: Arc::new(vec![]), all_tool_specs: Arc::new(vec![]), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "stub-model".into(), diff --git a/tests/calendar_grounding_e2e.rs b/tests/calendar_grounding_e2e.rs index e9d08e0c6c..f8f4a36d96 100644 --- a/tests/calendar_grounding_e2e.rs +++ b/tests/calendar_grounding_e2e.rs @@ -163,6 +163,10 @@ async fn test_integrations_agent_has_current_date_context() -> Result<()> { openhuman_core::openhuman::agent::tinyagents::TurnModelSource::from_model(model), all_tools: Arc::new(vec![Box::new(MockCalendarTool)]), all_tool_specs: Arc::new(vec![MockCalendarTool.spec()]), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "test-model".into(), diff --git a/tests/composio_list_tools_stack_overflow_regression.rs b/tests/composio_list_tools_stack_overflow_regression.rs index 0d7b455325..90cf872b4a 100644 --- a/tests/composio_list_tools_stack_overflow_regression.rs +++ b/tests/composio_list_tools_stack_overflow_regression.rs @@ -343,6 +343,10 @@ async fn drive_subagent() { openhuman_core::openhuman::agent::tinyagents::TurnModelSource::from_model(model), all_tools: Arc::new(vec![]), all_tool_specs: Arc::new(vec![]), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "test-model".into(), diff --git a/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs b/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs index 30a74039fc..bdfa0dec8c 100644 --- a/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs @@ -262,6 +262,10 @@ fn parent_context(workspace: &Path, model: Arc) -> ParentExecutio ), all_tools: Arc::new(tools), all_tool_specs: Arc::new(specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "round21-parent-model".to_string(), diff --git a/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs b/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs index e0a0dcb937..2047876ad6 100644 --- a/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs @@ -367,6 +367,10 @@ fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExe openhuman_core::openhuman::agent::tinyagents::TurnModelSource::from_model(provider), all_tools: Arc::new(tools), all_tool_specs: Arc::new(specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "round19-parent".to_string(), diff --git a/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs b/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs index 82c6c1600b..32ec8e46a8 100644 --- a/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs @@ -276,6 +276,10 @@ fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExe ), all_tools: Arc::new(tools), all_tool_specs: Arc::new(tool_specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "coverage-model".to_string(), diff --git a/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs b/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs index 0a0daeafb7..929d86ee43 100644 --- a/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs @@ -326,6 +326,10 @@ fn parent(workspace_dir: PathBuf, model: Arc) -> ParentExecutionC ), all_tools: Arc::new(tools), all_tool_specs: Arc::new(specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "round25-parent-model".to_string(), diff --git a/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs b/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs index 09b642189d..b1feaa8b24 100644 --- a/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs @@ -288,6 +288,10 @@ fn parent(workspace: PathBuf, model: Arc) -> ParentExecutionConte ), all_tools: Arc::new(tools), all_tool_specs: Arc::new(specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "round18-model".to_string(), diff --git a/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs b/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs index f6b0328e62..ac2c73d91b 100644 --- a/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs @@ -1115,6 +1115,10 @@ async fn subagent_runner_parent_context_filters_tools_caps_output_and_reports_er ), all_tools: Arc::new(all_tools), all_tool_specs: Arc::new(all_specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "parent-model".to_string(), diff --git a/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs b/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs index cf80f067d7..8654ab9ba8 100644 --- a/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs +++ b/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs @@ -363,6 +363,10 @@ fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExe ), all_tools: Arc::new(tools), all_tool_specs: Arc::new(tool_specs), + // #6145: empty means "same surface as `all_tool_specs`" — the + // catalogue falls back to it, so these stubs keep the behaviour + // they had before the parent's visible set became its own field. + visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), model_name: "round16-model".to_string(),