Skip to content

Restore host coverage for three memory RPC handlers dropped with the engine #6172

Description

@YellowSnnowmann

Summary

Three live memory RPC handlers have no test naming them anywhere in this crate. They are not leftovers from a removed domain — all three are registered controllers the desktop app calls today, one of them on a 5-second poll. They lost their coverage as collateral in #6161, which removed the memory engine from the test build.

Handler RPC method Reached from
memory::sources::rpc::add_rpc openhuman.memory_sources_add app/src/services/memorySourcesService.ts:93
memory::sync::sync_status::rpc::status_list_rpc openhuman.memory_sync_status_list the sync status table
memory::sync::composio::providers::slack::rpc::sync_status_rpc Slack sync status the sync status surface

Why the coverage went

All three were exercised only by tests/raw_coverage/memory_threads_raw_coverage_e2e.rs and memory_sync_tree_round21_raw_coverage_e2e.rs — 4,800-line targets that drove ~40 subjects at once through an in-process engine. #6161 deleted those targets with the engine, on the correct ground that most of what they asserted was the engine's behaviour and is covered upstream in tinymemory.

What went with them, and should not have, is the part that was never the engine's: this host's own policy in these handlers. No upstream work covers it, because upstream does not have these handlers.

A sweep of every deleted test file for host RPC handlers left with no surviving reference found 13. Seven are 5–17 line passthroughs to the driver and are legitimately upstream's to cover. Three were restored in #6170 (top_entities_rpc, chunks_for_entity_rpc, delete_chunk_rpc). These are the remaining three.

What is untested, per handler

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. The handler's comment states this is inherited behaviour kept deliberately, because "this surface renders a status table, and every caller of it today treats 'no rows' as 'nothing syncing'". A later tightening of any of the three into an RPC error would turn a 5-second poll into a visible failure on the Memory Sources screen, and nothing would catch it.

sync_status_rpc — filters to ACTIVE slack connections only, and answers a specific degraded zero-value shape when per-connection detail is no longer readable: per_channel_cursors == "{}", synced_ids_count == 0, requests_used_today == 0, daily_request_limit == 0, plus a log line explaining the degraded read. The deleted test asserted exactly this and named it slack_sync_status_rpc_reports_the_degraded_zero_value_shape.

add_rpc — generates the source id (src_<uuid>), maps the request into a MemorySourceEntry, and calls apply_kind_defaults so an add that leaves caps unset gets conservative per-kind defaults rather than zeros. apply_kind_defaults itself lives in tinymemory-sources and is upstream's to test; what belongs here is that this handler invokes it, and that the id and field mapping are what the registry receives.

Scope

Sibling *_tests.rs files already exist at all three sites, so this is adding test functions, not new modules or new production code:

  • src/openhuman/memory/sources/rpc_tests.rs
  • src/openhuman/memory/sync/sync_status/rpc_tests.rs
  • src/openhuman/memory/sync/composio/providers/slack/rpc_tests.rs

Follow the pattern memory/read_rpc/admin_tests.rs established and memory/read_rpc/entities_tests.rs (added in #6170) extends: bind a driver with binding::install_for_test, then assert how this host answers, not what a store contains. NullMemoryProvider covers the family-absent paths; a small local double covers the ones that need a driver to answer a specific way.

Not in scope

  • The seven thin passthroughs (get_rpc, update_rpc, remove_rpc, chunk_score_rpc, entity_index_for_rpc, list_items_rpc, read_item_rpc). Each is 5–17 lines of binding → member → map, and the contract behaviour underneath is covered by tinymemory's conformance suite.
  • Anything in tinymemory. The engine side has no gap here — every engine module the deleted tests exercised has upstream coverage.

Acceptance criteria

  • Each of the three handlers has at least one test naming it.
  • status_list_rpc: all three degradation paths assert Ok with an empty list, not an error.
  • sync_status_rpc: ACTIVE-only filtering and the four zero-value fields are asserted.
  • add_rpc: an add with caps unset reaches the registry with defaults applied and a generated id.
  • Every new assertion is verified able to fail — break the behaviour, watch the test go red, restore it. build(memory): drop the memory engine from the test build #6170 shipped three assertions that could not fail (two passed without the driver being reached at all), and they were only caught because a fourth assertion in the same file failed for an unrelated reason. An empty-list assertion in particular is satisfied by a driver that was never called, so pair it with evidence the handler reached the driver.
  • cargo clippy -p openhuman --features "$(bash scripts/ci/product-features.sh)" -- -D warnings clean.

Context


Correction (while implementing)

Two same-named status_list_rpc handlers exist, and the table above originally named the wrong one:

  • memory/sources/rpc_part_02.rs::status_list_rpc() serves memory_sources_status_list — the 5-second UI poll — and is a 5-line passthrough to status::status_list. Not worth host coverage; it belongs with the seven thin ones listed as out of scope.
  • memory/sync/sync_status/rpc.rs::status_list_rpc(config) serves memory_sync_status_list and carries the documented three-path degradation. That is the one covered.

Also: this issue reported sync_status_rpc as having one test file naming it. That was a false positive — the only hit is a comment at slack/rpc_tests.rs:22. Zero real coverage, as claimed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

priority: p2Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions