Skip to content

Commit 737c38b

Browse files
committed
test(abi): the MCP call log, proven durable through a real dlopen and a reload
store-valkey had ZERO over-the-ABI coverage of the four call-log methods it ships on this branch. Every existing test of append_mcp_call/list_mcp_calls/ list_mcp_call_principals/purge_mcp_calls_before calls ValkeyStore DIRECTLY, in-process — and none of them can see the failure that actually matters, because in production this backend is ONLY ever reached as a plugin (conformance boots the in-process RAM store, so the plugin seam is the only path a deployment takes, and it was the one path with no coverage at all). busbar_api::Store DEFAULTS all ten task/call-log methods to accept-and-keep- nothing. A seam that does not RELAY them substitutes those defaults silently: every append returns Ok, every list answers empty, and the deployment loses every tool-call record while reporting success. Not hypothetical — the ABI once carried four store methods while the trait carried ten, and exactly this happened. A unit test passing while the ABI drops every write is the shape this test exists to make impossible. RED BEFORE GREEN, proven not argued: with the SDK's store_dispatch answering Unsupported for the ten task/call-log variants (which is precisely what a plugin predating them does, and what makes the loader substitute the trait defaults), this test fails — assertion `left == right` failed: the per-principal call chain must survive the unload/reload over the plugin ABI in chain order; got 0 record(s) back, which is the accept-and-keep-nothing shape of the trait default an unrelayed seam substitutes left: [] right: [1, 2, 3] — while store-valkey's own 39 unit tests stay green throughout. That gap is the whole point. With the relay restored the test passes. The test dlopens the cdylib, writes at arity > 1 (three chained records for one principal, one for a second), DROPS the handle so busbar_close runs and the library is UNLOADED, then dlopens again over the same file and reads everything back: a restart is what proves durability, and a multi-row round trip across one cannot be faked by a lucky trait default the way a single-row same-session read could. A third leg re-reads through the plain ValkeyStore, so a plugin answering from its own in-process cache still fails. Exact counts are meaningful because the test starts from an empty log: list_mcp_call_principals and purge_mcp_calls_before are GLOBAL, not per- principal, so a leftover chain from an earlier run against a re-used Valkey would make both assertions vacuous. Per-run principal ids on top of that, and two of them — one principal's chain leaking into another's is a real defect class this repo has already hit once. Cargo.lock: committed as it now resolves. It had drifted uncommitted against the sibling busbarAI checkout (busbar-plugin-abi picked up tracing/tracing-core, libloading 0.8.9 -> 0.9.0), which is a `cargo --locked` failure waiting to happen in CI.
1 parent 366af65 commit 737c38b

2 files changed

Lines changed: 190 additions & 3 deletions

File tree

Cargo.lock

Lines changed: 11 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

store-valkey-plugin/tests/e2e.rs

Lines changed: 179 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
//! `VALKEY_URL` is a HARD FAILURE, never a silent skip, so the only over-the-ABI coverage of the
2626
//! durable Valkey store path cannot quietly vanish.
2727
28-
use busbar_api::{ModelTokens, Store, TierTokens, UsageLedger, VirtualKey};
28+
use busbar_api::{McpCallRecord, ModelTokens, Store, TierTokens, UsageLedger, VirtualKey};
2929
use busbar_plugin_loader::{load_store, plugin_library_filename};
3030
use busbar_store_valkey::ValkeyStore;
3131
use std::path::PathBuf;
@@ -758,3 +758,181 @@ fn admin_api_installs_the_valkey_plugin_and_writes_land_in_real_valkey() {
758758
let _ = Store::delete_key(&direct, &key_id);
759759
let _ = std::fs::remove_dir_all(&work);
760760
}
761+
762+
/// THE DURABILITY PROOF FOR THE FOUR MCP CALL-LOG METHODS, OVER THE REAL PLUGIN PATH.
763+
///
764+
/// This repo ships `feat/durable-mcp-call-log` — `append_mcp_call`/`list_mcp_calls`/
765+
/// `list_mcp_call_principals`/`purge_mcp_calls_before` against a real Valkey. Every existing test of
766+
/// those four calls `ValkeyStore` DIRECTLY, in-process, and NONE of them can see the failure that
767+
/// actually matters in production, because in production this backend is ONLY ever reached as a
768+
/// plugin: conformance boots the in-process RAM store, so the plugin seam is the only path a real
769+
/// deployment takes and was, until this test, the one path with zero coverage of these methods.
770+
///
771+
/// `busbar_api::Store` DEFAULTS all ten task/call-log methods to accept-and-keep-nothing. A plugin
772+
/// seam that does not RELAY them silently substitutes those defaults: every `append_mcp_call`
773+
/// returns `Ok`, every `list_mcp_calls` answers empty, and a deployment loses every tool-call record
774+
/// while reporting success. That is not hypothetical — the ABI once carried four store methods while
775+
/// the trait carried ten, so exactly this happened. A unit test passing while the ABI drops every
776+
/// write is the precise shape this test exists to make impossible.
777+
///
778+
/// So it goes through `busbar_plugin_loader::load_store`: a REAL `dlopen` of the built cdylib, the
779+
/// real C ABI, the real `DynStore`. It writes AT ARITY > 1 (three chained records for one principal
780+
/// and one for a second), DROPS the handle — which runs `busbar_close` and UNLOADS the library, so
781+
/// nothing this process still holds can answer the reads — then `dlopen`s AGAIN over the same file
782+
/// and reads everything back. A restart is what proves durability; a single-row same-session round
783+
/// trip would not distinguish a relayed method from a lucky trait default, and a multi-row one
784+
/// across an unload/reload cannot be faked by either.
785+
///
786+
/// A third leg reads the same rows through the plain `ValkeyStore`, never touching the cdylib, the
787+
/// C ABI or the loader — so a plugin that answered from its own in-process cache still fails here.
788+
#[test]
789+
fn mcp_call_log_survives_an_unload_and_reload_over_the_real_plugin_abi() {
790+
let path = plugin_path();
791+
let Some(url) = valkey_url() else {
792+
return;
793+
};
794+
let cfg = serde_json::json!({ "url": url }).to_string();
795+
796+
// Start from an EMPTY call log. `list_mcp_call_principals` and `purge_mcp_calls_before` are
797+
// GLOBAL, not per-principal, so against a re-used Valkey a leftover chain from an earlier run
798+
// would make both of their exact assertions below meaningless. `purge_mcp_calls_before(MAX)` is
799+
// the store's own contract-level wipe (it also drops each emptied principal from the
800+
// enumeration — see `ValkeyStore::purge_mcp_calls_before`), so this needs no key-pattern
801+
// guesswork. No other test in this file touches the `busbar:mcp:*` namespace.
802+
let direct = ValkeyStore::connect(&url).expect("connect directly to clean up and verify");
803+
Store::purge_mcp_calls_before(&direct, u64::MAX).expect("wipe the call log before this run");
804+
805+
// Per-run principal ids, for the same reason the key test uses one: a read that only THIS run's
806+
// writes can answer. Two of them, because one principal's chain leaking into another's is a real
807+
// defect class (this repo fixed a case-folding instance of it) and a single-principal test is
808+
// blind to it.
809+
let stamp = format!(
810+
"{}_{}",
811+
std::process::id(),
812+
std::time::SystemTime::now()
813+
.duration_since(std::time::UNIX_EPOCH)
814+
.unwrap()
815+
.as_nanos()
816+
);
817+
let p_main = format!("vk_abi_main_{stamp}");
818+
let p_other = format!("vk_abi_other_{stamp}");
819+
820+
let call = |principal: &str, seq: u64, prev: &str, hash: &str| McpCallRecord {
821+
principal: principal.to_string(),
822+
seq,
823+
ts: 2_000 + seq,
824+
server: "srv".to_string(),
825+
tool: "srv_read_file".to_string(),
826+
outcome: "dispatched".to_string(),
827+
reason: String::new(),
828+
tool_digest: format!("sha256:tool{seq}"),
829+
pin_generation: 3,
830+
request_id: format!("req-{seq}"),
831+
prev_hash: prev.to_string(),
832+
hash: hash.to_string(),
833+
};
834+
835+
{
836+
// BOOT 1 — a real dlopen of the cdylib; every call below crosses the C ABI.
837+
let store = load_store(&path, &cfg).expect("the valkey plugin must load over the real ABI");
838+
for (seq, prev, hash) in [(1_u64, "", "h1"), (2, "h1", "h2"), (3, "h2", "h3")] {
839+
store
840+
.append_mcp_call(&call(&p_main, seq, prev, hash))
841+
.expect("append_mcp_call over the ABI");
842+
}
843+
store
844+
.append_mcp_call(&call(&p_other, 1, "", "o1"))
845+
.expect("append_mcp_call over the ABI");
846+
// Dropping the boxed store drops the loader's `Library` handle: `busbar_close` runs and the
847+
// dylib is UNLOADED. Nothing this process still holds can be answering the reads below.
848+
drop(store);
849+
}
850+
851+
// BOOT 2 — a second, independent dlopen over the same file, a fresh `busbar_open`, a fresh
852+
// connection inside the plugin.
853+
let store = load_store(&path, &cfg).expect("the valkey plugin must load again over the real ABI");
854+
855+
let calls = store.list_mcp_calls(&p_main).expect("list_mcp_calls");
856+
assert_eq!(
857+
calls.iter().map(|c| c.seq).collect::<Vec<_>>(),
858+
vec![1, 2, 3],
859+
"the per-principal call chain must survive the unload/reload over the plugin ABI in chain \
860+
order; got {} record(s) back, which is the accept-and-keep-nothing shape of the trait \
861+
default an unrelayed seam substitutes",
862+
calls.len()
863+
);
864+
for w in calls.windows(2) {
865+
assert_eq!(
866+
w[1].prev_hash, w[0].hash,
867+
"the chain must still link after the reload: seq {} carries prev_hash {:?} but seq {} \
868+
persisted hash {:?}",
869+
w[1].seq, w[1].prev_hash, w[0].seq, w[0].hash
870+
);
871+
}
872+
// Every non-indexed field rides the `body` blob; a relay that dropped it would still satisfy a
873+
// seq-only check.
874+
assert_eq!(calls[2].tool_digest, "sha256:tool3");
875+
assert_eq!(calls[2].request_id, "req-3");
876+
assert_eq!(calls[2].tool, "srv_read_file");
877+
assert_eq!(calls[2].outcome, "dispatched");
878+
assert_eq!(calls[1].pin_generation, 3);
879+
assert_eq!(
880+
store
881+
.list_mcp_calls(&p_other)
882+
.expect("list_mcp_calls")
883+
.len(),
884+
1,
885+
"one principal's chain must not carry another's records"
886+
);
887+
888+
let principals = store
889+
.list_mcp_call_principals()
890+
.expect("list_mcp_call_principals");
891+
assert_eq!(
892+
principals,
893+
vec![p_main.clone(), p_other.clone()],
894+
"the boot enumeration must name every principal holding records, exactly once each, sorted"
895+
);
896+
897+
// Retention crosses the ABI too, COUNT AND ALL — checked for the number it ACTUALLY removed,
898+
// because a relay that dropped the return value would read as 0 and look like a no-op sweep.
899+
assert_eq!(
900+
store.purge_mcp_calls_before(2_002).expect("purge"),
901+
2,
902+
"both records at ts 2001 go (one per principal); the one sitting exactly at the cutoff stays"
903+
);
904+
assert_eq!(
905+
store
906+
.list_mcp_calls(&p_main)
907+
.expect("list_mcp_calls")
908+
.len(),
909+
2
910+
);
911+
assert!(store
912+
.list_mcp_calls(&p_other)
913+
.expect("list_mcp_calls")
914+
.is_empty());
915+
assert_eq!(
916+
store
917+
.list_mcp_call_principals()
918+
.expect("list_mcp_call_principals"),
919+
vec![p_main.clone()],
920+
"a principal whose chain the sweep emptied must leave the enumeration, or a boot keeps \
921+
resuming a chain with nothing in it"
922+
);
923+
drop(store);
924+
925+
// LEG 3 — read the surviving rows through the plain `ValkeyStore`, a code path that never
926+
// touches the cdylib, the C ABI or the loader. A plugin answering the reads above out of its
927+
// own in-process state (rather than Valkey) passes both boots and fails here.
928+
let direct_calls =
929+
Store::list_mcp_calls(&direct, &p_main).expect("list_mcp_calls via the direct connection");
930+
assert_eq!(
931+
direct_calls.iter().map(|c| c.seq).collect::<Vec<_>>(),
932+
vec![2, 3],
933+
"the records must be physically present in Valkey, not just cached in-process by the plugin"
934+
);
935+
assert_eq!(direct_calls[1].hash, "h3");
936+
937+
Store::purge_mcp_calls_before(&direct, u64::MAX).expect("clean up this run's records");
938+
}

0 commit comments

Comments
 (0)