From 93cede87122ac944c5d47c09f3c6ce1e3ab44fab Mon Sep 17 00:00:00 2001 From: OceanLi Date: Sat, 12 Sep 2026 09:09:22 -0400 Subject: [PATCH 1/2] fix(kg): enforce the object and range contracts the params declare A parameter's declared type is rendered into the JSON schema a caller is handed and was never compared against the argument that arrived, so two documented contracts were promises with nothing behind them. properties is declared as an object and accepted a bare string, which persisted. Every later reader then found a string where the schema said map. The success return is the real harm: an agent that mis-serializes a nested argument gets ok back and proceeds believing the write landed in the shape it intended, with no signal to correct on. Absent and explicit null stay legal, since null is how the update path clears the field. min_score is documented as a 0.0 to 1.0 floor and neither end was enforced. A floor above the range was honoured and returned an empty result, which a caller cannot tell from no such record, and a negative floor was silently clamped to zero, so the value passed was not the value that ran. Both are now refused with the range and the value named. Tests carry the arms that make them load-bearing. The object test has a control in the same test, the identical call with a real object, so the refusal is about the shape rather than the field; it also asserts a batch refusal names which item. The range test creates the rows and finds them at a sane floor first, so the empty result at an out-of-range floor cannot be read as an empty corpus. --- crates/khive-pack-kg/src/handlers/common.rs | 64 +++++++++++ crates/khive-pack-kg/src/handlers/create.rs | 5 + crates/khive-pack-kg/src/handlers/search.rs | 16 ++- crates/khive-pack-kg/src/handlers/update.rs | 2 + crates/khive-pack-kg/tests/integration.rs | 114 ++++++++++++++++++++ 5 files changed, 200 insertions(+), 1 deletion(-) diff --git a/crates/khive-pack-kg/src/handlers/common.rs b/crates/khive-pack-kg/src/handlers/common.rs index 59f02f68a..0299c0823 100644 --- a/crates/khive-pack-kg/src/handlers/common.rs +++ b/crates/khive-pack-kg/src/handlers/common.rs @@ -993,3 +993,67 @@ pub(crate) fn render_query_result(result: QueryResult) -> Value { out.insert("truncated".to_string(), json!(result.truncated)); Value::Object(out) } + +/// Name a JSON value's type the way a caller's schema names it. +fn json_type_name(value: &Value) -> &'static str { + match value { + Value::Null => "null", + Value::Bool(_) => "boolean", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Array(_) => "array", + Value::Object(_) => "object", + } +} + +/// Refuse a value for a parameter this pack declares as an object. +/// +/// `param_type` is a promise to the caller and nothing was checking it. It is +/// rendered into the JSON schema handed to a model and then never compared +/// against the argument that arrives, so `properties: "not-an-object"` was +/// accepted and persisted, and every later reader found a string where the +/// schema said map. For an agent that is worse than a refusal: a success +/// return gives it nothing to correct on, so it proceeds believing the write +/// landed in the shape it intended. +/// +/// Absent and explicit null are not type errors. Null is how a caller clears +/// the field on the update path, and absent means unchanged. +pub(crate) fn require_object_param(value: Option<&Value>, param: &str) -> Result<(), RuntimeError> { + match value { + None | Some(Value::Null) | Some(Value::Object(_)) => Ok(()), + Some(other) => Err(RuntimeError::InvalidInput(format!( + "{param} must be an object; got {}", + json_type_name(other) + ))), + } +} + +#[cfg(test)] +mod param_contract_tests { + use super::*; + + #[test] + fn an_object_parameter_refuses_every_non_object_and_names_what_it_got() { + // The shape that was accepted and persisted. + let err = require_object_param(Some(&json!("not-an-object")), "properties") + .expect_err("a string is not an object"); + let message = err.to_string(); + assert!(message.contains("properties"), "{message}"); + assert!(message.contains("string"), "names what arrived: {message}"); + + // An array is the near miss a caller is most likely to send next, so it + // must be refused by type rather than by a map-specific probe. + assert!(require_object_param(Some(&json!([1, 2])), "properties").is_err()); + assert!(require_object_param(Some(&json!(7)), "properties").is_err()); + assert!(require_object_param(Some(&json!(true)), "properties").is_err()); + } + + #[test] + fn absent_and_explicit_null_are_not_type_errors() { + // Absent means unchanged and null is how the update path clears a field; + // an implementation that refuses anything that is not an object breaks both. + require_object_param(None, "properties").expect("absent is allowed"); + require_object_param(Some(&Value::Null), "properties").expect("null is allowed"); + require_object_param(Some(&json!({"a": 1})), "properties").expect("an object is allowed"); + } +} diff --git a/crates/khive-pack-kg/src/handlers/create.rs b/crates/khive-pack-kg/src/handlers/create.rs index 1d757e780..b490982e0 100644 --- a/crates/khive-pack-kg/src/handlers/create.rs +++ b/crates/khive-pack-kg/src/handlers/create.rs @@ -155,6 +155,10 @@ impl KgPack { let mut specs: Vec = Vec::with_capacity(attempted); let mut entity_type_normalized: Vec = Vec::new(); for (idx, entry) in entries.into_iter().enumerate() { + super::common::require_object_param( + entry.properties.as_ref(), + &format!("items[{idx}].properties"), + )?; // Resolve the item's own kind. let item_kind_spec = resolve_kind_spec(&entry.kind, registry).map_err(|e| { RuntimeError::InvalidInput(format!("items[{idx}].kind: {e}")) @@ -378,6 +382,7 @@ impl KgPack { } let p: CreateParams = deser(params.clone())?; + super::common::require_object_param(p.properties.as_ref(), "properties")?; if p.kind != "note" && (p.key.is_some() || p.embed.is_some() || p.fence.is_some()) { return Err(RuntimeError::InvalidInput( "key, embed and fence apply only to notes".into(), diff --git a/crates/khive-pack-kg/src/handlers/search.rs b/crates/khive-pack-kg/src/handlers/search.rs index 44c4d0652..1c3a7e1f4 100644 --- a/crates/khive-pack-kg/src/handlers/search.rs +++ b/crates/khive-pack-kg/src/handlers/search.rs @@ -59,6 +59,7 @@ impl ValidatedSearchRequest { /// Parse and validate the canonical KG search wire contract. pub fn from_value(params: Value, registry: &VerbRegistry) -> Result { let p: SearchParams = deser(params)?; + super::common::require_object_param(p.properties.as_ref(), "properties")?; let kind_raw = p .kind .as_deref() @@ -74,7 +75,20 @@ impl ValidatedSearchRequest { }; let tags = p.tags.unwrap_or_default(); let limit = p.limit.unwrap_or(10).min(100); - let min_score = p.min_score.unwrap_or(0.0).max(0.0); + // The declared range is 0.0 to 1.0 and neither end was enforced. A floor + // above 1.0 was honoured and returned an empty result, which a caller cannot + // tell from no such record; a negative floor was silently clamped to 0.0, so + // the value the caller passed was not the value that ran. Refuse both and + // name the range, the way the other input refusals on this surface do. + let min_score = match p.min_score { + None => 0.0, + Some(value) if value.is_finite() && (0.0..=1.0).contains(&value) => value, + Some(value) => { + return Err(RuntimeError::InvalidInput(format!( + "min_score must be between 0.0 and 1.0; got {value}" + ))) + } + }; let source = match p.source.as_deref() { None => None, Some("text") => Some(SearchSource::Text), diff --git a/crates/khive-pack-kg/src/handlers/update.rs b/crates/khive-pack-kg/src/handlers/update.rs index b2f3a08b7..12ac2ebd0 100644 --- a/crates/khive-pack-kg/src/handlers/update.rs +++ b/crates/khive-pack-kg/src/handlers/update.rs @@ -182,6 +182,7 @@ impl KgPack { registry: &VerbRegistry, ) -> Result { let p: UpdateParams = deser(params.clone())?; + super::common::require_object_param(p.properties.as_ref(), "properties")?; if p.entity_kind.is_some() { return Err(RuntimeError::InvalidInput( "entity_kind is immutable; to change kind, delete then re-create the entity, or use merge() if this is a deduplication correction".into(), @@ -300,6 +301,7 @@ impl KgPack { .prepare_note_update_hook(&self.runtime, token, ¬e, &mut params) .await?; let p: UpdateParams = deser(params)?; + super::common::require_object_param(p.properties.as_ref(), "properties")?; let patch = NotePatch::new( optional_string_patch(p.name, "name")?, p.content, diff --git a/crates/khive-pack-kg/tests/integration.rs b/crates/khive-pack-kg/tests/integration.rs index 4e811ccee..8624b8581 100644 --- a/crates/khive-pack-kg/tests/integration.rs +++ b/crates/khive-pack-kg/tests/integration.rs @@ -14933,3 +14933,117 @@ async fn update_empty_string_property_survives_agent_echo_and_readback() { "Agent readback must retain the same empty-string value: {agent_readback}" ); } + +/// A parameter this pack declares as an object must refuse a scalar rather than +/// store it. The declared type is rendered into the schema a caller is handed and +/// was never compared against the argument that arrived, so a string persisted and +/// every later reader found a string where the schema promised a map. The success +/// return is the harm: an agent has nothing to correct on. +#[tokio::test] +async fn create_refuses_a_scalar_where_properties_declares_an_object() { + let pack = pack(); + + let error = pack + .dispatch( + "create", + json!({ + "kind": "entity", + "entity_kind": "concept", + "name": "ObjectParamScalar", + "properties": "not-an-object" + }), + ) + .await + .expect_err("a string properties must be refused, not stored"); + assert!( + is_invalid_input(&error), + "must be an input refusal, got {error:?}" + ); + let message = error.to_string(); + assert!( + message.contains("properties") && message.contains("object"), + "the refusal must name the parameter and the expected shape: {message}" + ); + + // Control in the same test: the identical call with a real object succeeds, so + // the refusal is about the shape and not about the field being present at all. + pack.dispatch( + "create", + json!({ + "kind": "entity", + "entity_kind": "concept", + "name": "ObjectParamControl", + "properties": {"domain": "inference"} + }), + ) + .await + .expect("an object properties must still be accepted"); + + // A batch names the offending item rather than the batch, since one error is + // returned for N records. + let error = pack + .dispatch( + "create", + json!({"items": [ + {"kind": "entity", "entity_kind": "concept", "name": "BatchOk", + "properties": {"domain": "inference"}}, + {"kind": "entity", "entity_kind": "concept", "name": "BatchBad", + "properties": "not-an-object"} + ]}), + ) + .await + .expect_err("a scalar properties inside a batch must be refused"); + assert!( + error.to_string().contains("items[1]"), + "the refusal must name which item: {error}" + ); +} + +/// `min_score` is documented as a 0.0-1.0 floor and neither end was enforced: a +/// floor above the range was honoured and returned an empty result a caller +/// cannot tell from "no such record", and a negative floor was silently clamped, +/// so the value passed was not the value that ran. +#[tokio::test] +async fn search_refuses_a_score_floor_outside_the_declared_range() { + let pack = pack(); + for name in ["ScoreFloorOne", "ScoreFloorTwo"] { + pack.dispatch( + "create", + json!({"kind": "entity", "entity_kind": "concept", "name": name}), + ) + .await + .unwrap(); + } + + // Load-bearing control: the rows ARE findable at a sane floor. Without this the + // empty result at an out-of-range floor could be an empty corpus. + let hits = pack + .dispatch( + "search", + json!({"kind": "entity", "query": "ScoreFloor", "min_score": 0.0, "limit": 10}), + ) + .await + .expect("a floor inside the range must be accepted"); + assert!( + !hits.as_array().expect("array").is_empty(), + "control: the rows must be findable at a sane floor" + ); + + for floor in [json!(7), json!(-3), json!(1.5)] { + let error = pack + .dispatch( + "search", + json!({"kind": "entity", "query": "ScoreFloor", "min_score": floor, "limit": 10}), + ) + .await + .expect_err("a floor outside 0.0-1.0 must be refused, not honoured"); + assert!( + is_invalid_input(&error), + "must be an input refusal, got {error:?}" + ); + assert!( + error.to_string().contains("min_score"), + "the refusal must name the parameter: {error}" + ); + } +} From ae5f1340472956dd965ec7f6cd2ce2d615c9b4dc Mon Sep 17 00:00:00 2001 From: OceanLi Date: Sat, 12 Sep 2026 10:01:34 -0400 Subject: [PATCH 2/2] test(contract): use an in-range score floor, matching the Rust source --- .../tests/test_coordinator_fanout.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/khive-contract/tests/test_coordinator_fanout.py b/tests/khive-contract/tests/test_coordinator_fanout.py index 1c9078766..3ef2f2769 100644 --- a/tests/khive-contract/tests/test_coordinator_fanout.py +++ b/tests/khive-contract/tests/test_coordinator_fanout.py @@ -321,16 +321,21 @@ def test_search_min_score_filters_all_below_threshold( khive_session: KhiveMcpSession, temp_namespace: str, ) -> None: - """search(kind="entity", min_score=2.0) returns empty results for any real entity. + """search(kind="entity", min_score=1.0) returns empty results for any real entity. Source: crates/kkernel/src/coordinator/tests.rs t7c_multi_backend_search_min_score_applied - RRF scores for any real hit are always <= 1/(60+1) ~= 0.016. A min_score - of 2.0 is above any achievable RRF score. If the coordinator or handler - ignores min_score, the seeded entity would be returned and this test fails. - An empty result proves min_score is applied (search.rs line 138, - score_floor = p.min_score.unwrap_or(0.0).max(0.0)). + RRF scores for any real hit are always <= 1/(60+1) ~= 0.016, so a floor of + 1.0 is above any achievable score. If the coordinator or handler ignores + min_score, the seeded entity would be returned and this test fails; an empty + result proves the floor is applied. + + This port previously used 2.0, which is outside the documented 0.0-1.0 range + and was only accepted because the range was unenforced. Its own Rust source + uses 1.0. The intent, a floor above every achievable score, is expressible + inside the contract, so the out-of-range value bought nothing and hid the + fact that the range was a promise with nothing behind it. """ ns = temp_namespace @@ -355,13 +360,13 @@ def test_search_min_score_filters_all_below_threshold( hits = khive_session.verb("search", { "kind": "entity", "query": "cft7c_minscore_probe", - "min_score": 2.0, + "min_score": 1.0, "namespace": ns, }) assert isinstance(hits, list), f"search must return a list; got {type(hits)}" assert hits == [], ( - "min_score=2.0 must exclude all results (no real RRF score can reach 2.0); " + "min_score=1.0 must exclude all results (no real RRF score can reach 1.0); " f"got {len(hits)} hit(s): {hits}" )