diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index 9f45a123ec..562c124874 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +- MCP `list_*` tools now write `count`, `has_more`, `unmatched_fields`, and + any other metadata ahead of the items in their text result, so these keys + survive when an agent harness truncates a large result from the end. + `structuredContent` is unchanged. - `config update` without field flags no longer prints `Nothing to update.` and exits 0. In a terminal, it now opens an interactive editor. Outside a terminal, it exits with status 2. Scripts that call it with no flags now @@ -46,6 +50,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). model only that text, such as Claude Code, never saw the data, and agents retried `preview_rule` expecting results. + ## [v0.7.5] - October 19, 2026 ### What's New diff --git a/rust/crates/sift_mcp/CLAUDE.md b/rust/crates/sift_mcp/CLAUDE.md index f4e738e96a..c5932a170c 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -683,6 +683,8 @@ Run through this before declaring the tool done: match the current proto comments. - [ ] Any tool taking a `filter` closes its `filter` bullet with the search block from Step 5, adapted to that resource's own text fields and phrased as a direct instruction. +- [ ] Any `list_*` tool builds its body with `list_body` and returns it through + `list_result`, so its metadata is written ahead of the items. - [ ] `read_only_hint` is correct, and write tools set `destructive_hint` / `idempotent_hint` per the Step 4 mapping. Write tools confirm the destination via `next_step`. - [ ] Every write handler gates itself as the first line: `self.require_create()?` for diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index 2f3bc4007d..1f405c379c 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/mod.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/mod.rs @@ -14,7 +14,7 @@ use sift_rs::{ use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, - tool::common::{MetadataEntry, list_body, url_clause, with_urls}, + tool::common::{MetadataEntry, list_body, list_result, url_clause, with_urls}, }; #[cfg(test)] @@ -198,12 +198,11 @@ impl SiftMcpServer { self.url_service.build_annotation_url(&a.annotation_id).ok() })?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("annotations", annotations, fields, page.has_more), "annotations", - annotations, - fields, - page.has_more, - ))) + None, + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs index 1eb34cb934..80097ead28 100644 --- a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs +++ b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs @@ -16,7 +16,7 @@ use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, service::artifacts::{ArtifactView, CreateArtifactInput, UpdateArtifactInput, UpdatePath}, - tool::common::{MetadataEntry, list_body, to_values, url_clause}, + tool::common::{MetadataEntry, list_body, list_result, to_values, url_clause}, }; #[cfg(test)] @@ -442,12 +442,11 @@ impl SiftMcpServer { }) .collect::, ErrorData>>()?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("artifacts", artifacts, fields, page.has_more), "artifacts", - artifacts, - fields, - page.has_more, - ))) + None, + )) } #[tool( @@ -531,7 +530,7 @@ impl SiftMcpServer { map.insert("artifact_url".to_string(), serde_json::json!(artifact_url)); } - Ok(CallToolResult::structured(body)) + Ok(list_result(body, "artifact_versions", None)) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/artifacts/test.rs b/rust/crates/sift_mcp/src/tool/artifacts/test.rs index 0f2d7ebc8e..2ece2c9480 100644 --- a/rust/crates/sift_mcp/src/tool/artifacts/test.rs +++ b/rust/crates/sift_mcp/src/tool/artifacts/test.rs @@ -29,7 +29,7 @@ use crate::{ artifacts::ArtifactListParams, common::{ MetadataEntry, MetadataScalar, - test_support::{structured, structured_field}, + test_support::{assert_keys_before, content_texts, structured, structured_field}, }, }, }; @@ -387,6 +387,11 @@ async fn list_artifact_versions_returns_rows_newest_first() { })) .await .expect("list versions"); + assert_keys_before( + &content_texts(&resp)[0], + &["artifact_url", "count", "has_more"], + "artifact_versions", + ); let body = structured(resp); let rows = body["artifact_versions"].as_array().expect("rows"); assert_eq!(rows.len(), 2); diff --git a/rust/crates/sift_mcp/src/tool/assets/mod.rs b/rust/crates/sift_mcp/src/tool/assets/mod.rs index b3e9babe71..ac5c8a3f58 100644 --- a/rust/crates/sift_mcp/src/tool/assets/mod.rs +++ b/rust/crates/sift_mcp/src/tool/assets/mod.rs @@ -11,7 +11,7 @@ use sift_rs::metadata::v1::MetadataValue; use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, - tool::common::{ListParams, MetadataEntry, list_body, url_clause, with_urls}, + tool::common::{ListParams, MetadataEntry, list_body, list_result, url_clause, with_urls}, }; #[derive(Debug, Deserialize, JsonSchema)] @@ -96,12 +96,11 @@ impl SiftMcpServer { self.url_service.build_asset_url(&a.asset_id).ok() })?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("assets", assets, fields, page.has_more), "assets", - assets, - fields, - page.has_more, - ))) + None, + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs b/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs index be40e87ec9..45e42c5e41 100644 --- a/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs @@ -16,7 +16,7 @@ use crate::{ service::calculated_channels::{ CalculatedChannelUpdate, CalculatedChannelWrite, NewCalculatedChannel, }, - tool::common::{ListParams, MetadataEntry, list_body, read_result, to_values}, + tool::common::{ListParams, MetadataEntry, list_body, list_result, to_values}, }; #[cfg(test)] @@ -152,12 +152,11 @@ impl SiftMcpServer { let channels = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("calculated_channels", channels, fields, page.has_more), "calculated_channels", - channels, - fields, - page.has_more, - ))) + None, + )) } #[tool( @@ -252,7 +251,11 @@ impl SiftMcpServer { ); body["next_step"] = serde_json::Value::String(next_step.clone()); - Ok(read_result(body, next_step)) + Ok(list_result( + body, + "calculated_channel_versions", + Some(next_step), + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/calculated_channels/test.rs b/rust/crates/sift_mcp/src/tool/calculated_channels/test.rs index 50254b05d0..e42e37340a 100644 --- a/rust/crates/sift_mcp/src/tool/calculated_channels/test.rs +++ b/rust/crates/sift_mcp/src/tool/calculated_channels/test.rs @@ -17,7 +17,9 @@ use super::{ }; use crate::{ server::SiftMcpServer, - tool::common::test_support::{content_texts, list_params, list_params_with_fields, structured}, + tool::common::test_support::{ + assert_keys_before, content_texts, list_params, list_params_with_fields, structured, + }, }; const REFERENCES_JSON: &str = r#"[ @@ -187,7 +189,12 @@ async fn list_calculated_channel_versions_returns_versions() { let texts = content_texts(&resp); assert_eq!(texts.len(), 2); - assert!(texts[1].contains("calculated_channel_versions")); + assert!(texts[0].starts_with("Listed 1 calculated channel versions.")); + assert_keys_before( + &texts[1], + &["count", "has_more", "next_step"], + "calculated_channel_versions", + ); let body = structured(resp); let versions = body["calculated_channel_versions"] diff --git a/rust/crates/sift_mcp/src/tool/channels/mod.rs b/rust/crates/sift_mcp/src/tool/channels/mod.rs index 985b4cc72b..4b417be1fe 100644 --- a/rust/crates/sift_mcp/src/tool/channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/channels/mod.rs @@ -1,9 +1,9 @@ -use rmcp::{handler::server::wrapper::Parameters, model::CallToolResult, tool, tool_router}; +use rmcp::{handler::server::wrapper::Parameters, tool, tool_router}; use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, - tool::common::{ListParams, list_body, to_values}, + tool::common::{ListParams, list_body, list_result, to_values}, }; #[cfg(test)] @@ -80,11 +80,10 @@ impl SiftMcpServer { let channels = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("channels", channels, fields, page.has_more), "channels", - channels, - fields, - page.has_more, - ))) + None, + )) } } diff --git a/rust/crates/sift_mcp/src/tool/channels/test.rs b/rust/crates/sift_mcp/src/tool/channels/test.rs index f748ed63c4..9563d71520 100644 --- a/rust/crates/sift_mcp/src/tool/channels/test.rs +++ b/rust/crates/sift_mcp/src/tool/channels/test.rs @@ -10,7 +10,8 @@ use crate::{ server::SiftMcpServer, service::common::DEFAULT_LIMIT, tool::common::test_support::{ - list_params, list_params_with_fields, structured, structured_field, + assert_keys_before, content_texts, list_params, list_params_with_fields, structured, + structured_field, }, }; @@ -240,6 +241,16 @@ async fn list_channels_reports_a_field_that_matched_nothing() { .await .expect("list_channels failed"); + // `channels` sorts before every metadata key, so this fails if the handler + // returns the body without `list_result`. + let texts = content_texts(&resp); + assert_eq!(texts.len(), 1); + assert_keys_before( + &texts[0], + &["count", "has_more", "unmatched_fields"], + "channels", + ); + let body = structured(resp); assert_eq!( body["channels"], diff --git a/rust/crates/sift_mcp/src/tool/common/mod.rs b/rust/crates/sift_mcp/src/tool/common/mod.rs index cbdd11bfcf..cbd63188da 100644 --- a/rust/crates/sift_mcp/src/tool/common/mod.rs +++ b/rust/crates/sift_mcp/src/tool/common/mod.rs @@ -1,6 +1,7 @@ use rmcp::ErrorData; use rmcp::model::{CallToolResult, ContentBlock}; use rmcp::schemars::{self, JsonSchema}; +use serde::ser::SerializeMap; use serde::{Deserialize, Serialize}; use serde_json::Value; use sift_rs::metadata::v1::{ @@ -176,10 +177,14 @@ pub(crate) fn project_fields(items: Vec, fields: &[String]) -> (Vec, +) -> CallToolResult { + let text = ItemsLast { + body: &body, + items_key, + }; + let text = serde_json::to_string(&text).unwrap_or_else(|_| body.to_string()); + let mut content = Vec::with_capacity(2); + if let Some(next_step) = next_step { + content.push(ContentBlock::text(next_step)); + } + content.push(ContentBlock::text(text)); + + let mut result = CallToolResult::structured(body); + result.content = content; + result +} + +/// Serializes a JSON object with one key moved to the end. `SerializeMap` +/// writes entries in the order they are given, which `serde_json::Map` would +/// otherwise re-sort. +struct ItemsLast<'a> { + body: &'a Value, + items_key: &'a str, +} + +impl Serialize for ItemsLast<'_> { + fn serialize(&self, serializer: S) -> Result { + let Value::Object(map) = self.body else { + return self.body.serialize(serializer); + }; + let mut out = serializer.serialize_map(Some(map.len()))?; + for (key, value) in map.iter().filter(|(key, _)| *key != self.items_key) { + out.serialize_entry(key, value)?; + } + if let Some(items) = map.get(self.items_key) { + out.serialize_entry(self.items_key, items)?; + } + out.end() + } +} + /// Shape a read tool's result so the calling agent sees both `next_step` and /// the payload. `CallToolResult::structured` already mirrors `body` into a text /// content block; replacing `content` with `next_step` alone, as write tools @@ -304,6 +365,23 @@ pub(crate) mod test_support { .collect() } + /// Assert that every key in `earlier` is written before `later` in `text`. + /// List tools write their metadata ahead of the items so it survives an + /// agent harness truncating the result from the end. + pub(crate) fn assert_keys_before(text: &str, earlier: &[&str], later: &str) { + let offset = |key: &str| { + text.find(&format!("\"{key}\":")) + .unwrap_or_else(|| panic!("missing key `{key}` in {text}")) + }; + let later_at = offset(later); + for key in earlier { + assert!( + offset(key) < later_at, + "`{key}` must come before `{later}` in {text}" + ); + } + } + /// Extract a single field from a tool result's structured JSON body. pub(crate) fn structured_field(result: CallToolResult, key: &str) -> Value { let mut value = structured(result); diff --git a/rust/crates/sift_mcp/src/tool/common/test.rs b/rust/crates/sift_mcp/src/tool/common/test.rs index c40ad6abf9..9a1149aee9 100644 --- a/rust/crates/sift_mcp/src/tool/common/test.rs +++ b/rust/crates/sift_mcp/src/tool/common/test.rs @@ -1,5 +1,7 @@ use super::{ - ListParams, list_body, project_fields, read_result, test_support::content_texts, url_clause, + ListParams, list_body, list_result, project_fields, read_result, + test_support::{assert_keys_before, content_texts}, + url_clause, }; use serde_json::{Value, json}; @@ -262,6 +264,94 @@ fn list_body_omits_unmatched_fields_when_the_page_is_empty() { ); } +/// Every items key the list tools use. Most sort before `count`, which is why +/// the text block cannot rely on `serde_json::Map` ordering. +const ITEMS_KEYS: [&str; 14] = [ + "annotations", + "artifacts", + "artifact_versions", + "assets", + "calculated_channels", + "calculated_channel_versions", + "channels", + "rules", + "runs", + "test_reports", + "test_steps", + "test_measurements", + "user_defined_functions", + "user_defined_function_versions", +]; + +#[test] +fn list_result_writes_metadata_before_the_items() { + // Agent harnesses cut oversized tool results from the end. The metadata has + // to be written ahead of the items or it is the first thing lost, on + // exactly the large responses where it matters. + for key in ITEMS_KEYS { + let items = vec![json!({ "name": "throttle" })]; + let body = list_body(key, items, Some(vec!["nmae".into()]), true); + + let texts = content_texts(&list_result(body, key, None)); + + assert_eq!(texts.len(), 1, "{key}"); + assert_keys_before(&texts[0], &["count", "has_more", "unmatched_fields"], key); + } +} + +#[test] +fn list_result_metadata_survives_head_truncation() { + let items: Vec = (0..500) + .map(|i| json!({ "name": format!("channel-{i}") })) + .collect(); + let body = list_body("channels", items, Some(vec!["nmae".into()]), true); + + let text = content_texts(&list_result(body, "channels", None)).remove(0); + let head: String = text.chars().take(200).collect(); + + for expected in [ + "\"count\":500", + "\"has_more\":true", + "\"unmatched_fields\":[\"nmae\"]", + ] { + assert!( + head.contains(expected), + "{expected} must sit in the first 200 characters: {head}" + ); + } +} + +#[test] +fn list_result_text_is_the_structured_body_reordered() { + // Consumers that parse the text must get exactly the structured payload; + // only key order may differ. + let items = vec![json!({ "name": "throttle", "channelId": "c1" })]; + let body = list_body( + "channels", + items, + Some(vec!["name".into(), "nmae".into()]), + false, + ); + + let result = list_result(body.clone(), "channels", None); + + let texts = content_texts(&result); + let parsed: Value = serde_json::from_str(&texts[0]).expect("text should be JSON"); + assert_eq!(parsed, body); + assert_eq!(result.structured_content, Some(body)); +} + +#[test] +fn list_result_puts_next_step_in_its_own_leading_block() { + let body = list_body("channels", vec![json!({ "name": "a" })], None, false); + + let texts = content_texts(&list_result(body, "channels", Some("Listed 1.".into()))); + + assert_eq!(texts.len(), 2); + assert_eq!(texts[0], "Listed 1."); + assert_keys_before(&texts[1], &["count", "has_more"], "channels"); +} + /// A read tool's payload must reach `content`, not only `structuredContent`: /// Claude Code shows the model `content`, so a payload left out of it is /// invisible to the agent. diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index 2d9902e324..46dc89a738 100644 --- a/rust/crates/sift_mcp/src/tool/rules/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rules/mod.rs @@ -12,7 +12,7 @@ use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, service::rules::RuleUpdate, - tool::common::{ListParams, list_body, read_result, url_clause, with_urls}, + tool::common::{ListParams, list_body, list_result, read_result, url_clause, with_urls}, }; #[cfg(test)] @@ -130,12 +130,11 @@ impl SiftMcpServer { self.url_service.build_rule_url(&r.rule_id).ok() })?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("rules", rules, fields, page.has_more), "rules", - rules, - fields, - page.has_more, - ))) + None, + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/runs/mod.rs b/rust/crates/sift_mcp/src/tool/runs/mod.rs index 42b3237a23..c8d6bdbc0a 100644 --- a/rust/crates/sift_mcp/src/tool/runs/mod.rs +++ b/rust/crates/sift_mcp/src/tool/runs/mod.rs @@ -11,7 +11,7 @@ use sift_rs::metadata::v1::MetadataValue; use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, - tool::common::{ListParams, MetadataEntry, list_body, url_clause, with_urls}, + tool::common::{ListParams, MetadataEntry, list_body, list_result, url_clause, with_urls}, }; #[cfg(test)] @@ -113,12 +113,11 @@ impl SiftMcpServer { self.url_service.build_run_url(&r.run_id).ok() })?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("runs", runs, fields, page.has_more), "runs", - runs, - fields, - page.has_more, - ))) + None, + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs index 1ecbd354c1..cc0639f8a7 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs @@ -11,7 +11,7 @@ use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, service::test_reports::spec::{self, ReportSpec}, - tool::common::{ListParams, list_body, to_values, url_clause}, + tool::common::{ListParams, list_body, list_result, to_values, url_clause}, }; #[cfg(test)] @@ -118,12 +118,11 @@ impl SiftMcpServer { let test_reports = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("test_reports", test_reports, fields, page.has_more), "test_reports", - test_reports, - fields, - page.has_more, - ))) + None, + )) } #[tool( @@ -200,12 +199,11 @@ impl SiftMcpServer { let test_steps = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("test_steps", test_steps, fields, page.has_more), "test_steps", - test_steps, - fields, - page.has_more, - ))) + None, + )) } #[tool( @@ -282,12 +280,16 @@ impl SiftMcpServer { let test_measurements = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body( + "test_measurements", + test_measurements, + fields, + page.has_more, + ), "test_measurements", - test_measurements, - fields, - page.has_more, - ))) + None, + )) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs b/rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs index 789f453c20..cc7c1dfc2c 100644 --- a/rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs @@ -15,7 +15,7 @@ use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, service::user_defined_functions::UdfUpdate, - tool::common::{ListParams, MetadataEntry, list_body, to_values}, + tool::common::{ListParams, MetadataEntry, list_body, list_result, to_values}, }; #[cfg(test)] @@ -149,12 +149,11 @@ impl SiftMcpServer { let functions = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body("user_defined_functions", functions, fields, page.has_more), "user_defined_functions", - functions, - fields, - page.has_more, - ))) + None, + )) } #[tool( @@ -244,12 +243,16 @@ impl SiftMcpServer { let versions = to_values(&page.items)?; - Ok(CallToolResult::structured(list_body( + Ok(list_result( + list_body( + "user_defined_function_versions", + versions, + fields, + page.has_more, + ), "user_defined_function_versions", - versions, - fields, - page.has_more, - ))) + None, + )) } #[tool(