From 1414884043859cc35b916ff5401dd4d1df52d463 Mon Sep 17 00:00:00 2001 From: Alexa Agnew Date: Tue, 20 Oct 2026 11:30:14 -0700 Subject: [PATCH 1/4] rust(fix): keep unmatched fields visible when list output is truncated Rename the list tools' unmatched_fields key to _unmatched_fields so it sorts ahead of the items and survives head truncation of large responses. Document the key in each list tool's Output section and add tests for the ordering and the descriptions. --- rust/crates/sift_cli/CHANGELOG.md | 7 +++ rust/crates/sift_mcp/CLAUDE.md | 2 + rust/crates/sift_mcp/src/server/test.rs | 47 ++++++++++++++++ .../sift_mcp/src/tool/annotations/mod.rs | 5 +- .../crates/sift_mcp/src/tool/artifacts/mod.rs | 10 +++- rust/crates/sift_mcp/src/tool/assets/mod.rs | 5 +- .../src/tool/calculated_channels/mod.rs | 10 +++- rust/crates/sift_mcp/src/tool/channels/mod.rs | 5 +- .../crates/sift_mcp/src/tool/channels/test.rs | 4 +- rust/crates/sift_mcp/src/tool/common/mod.rs | 15 ++++-- rust/crates/sift_mcp/src/tool/common/test.rs | 54 ++++++++++++++++++- rust/crates/sift_mcp/src/tool/data/mod.rs | 2 +- rust/crates/sift_mcp/src/tool/rules/mod.rs | 5 +- rust/crates/sift_mcp/src/tool/runs/mod.rs | 5 +- rust/crates/sift_mcp/src/tool/runs/test.rs | 2 +- .../sift_mcp/src/tool/test_reports/mod.rs | 15 ++++-- .../src/tool/user_defined_functions/mod.rs | 10 +++- 17 files changed, 179 insertions(+), 24 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index aa8360bf25..56cdccb162 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,13 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +- MCP `list_*` tools now list any field requested in `fields` that matches no + key on the returned items in `_unmatched_fields`, renamed from + `unmatched_fields`. The new name sorts + ahead of the items, so the warning comes first in the response and survives + when an agent harness truncates a large result. Each list tool's Output + section now documents the key. + ## [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 6644dee492..6bbcddcb1c 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -676,6 +676,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 taking `fields` builds its body with `list_body` and documents + `_unmatched_fields` in both its `fields` bullet and its Output section. - [ ] `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/server/test.rs b/rust/crates/sift_mcp/src/server/test.rs index 0124f90a3d..b820c9bae3 100644 --- a/rust/crates/sift_mcp/src/server/test.rs +++ b/rust/crates/sift_mcp/src/server/test.rs @@ -712,6 +712,53 @@ async fn every_registered_tool_has_a_client_event() { grpc_handle.abort(); } +/// Agents learn the projection warning's name only from the tool description. +/// Every tool that accepts `fields` must name `_unmatched_fields` in both its +/// `fields` bullet and its Output section, and never the pre-rename key, or the +/// agent will look for a key the response no longer carries. +#[tokio::test] +async fn every_fields_tool_documents_unmatched_fields() { + let current = UpdateCheck::Current { + current_version: "0.4.0".to_string(), + latest_version: "0.4.0".to_string(), + }; + let (server, grpc_handle) = server_with_feature_flags( + Some(receiver(current)), + 0, + ClientEventReporter::default(), + all_feature_flags(), + ) + .await; + let fields_tools: Vec<_> = server + .tool_router + .list_all() + .into_iter() + .filter(|tool| { + tool.input_schema + .get("properties") + .and_then(|properties| properties.get("fields")) + .is_some() + }) + .collect(); + let undocumented: Vec<_> = fields_tools + .iter() + .filter(|tool| { + let description = tool.description.as_deref().unwrap_or_default(); + description.matches("`_unmatched_fields`").count() < 2 + || description.contains("`unmatched_fields`") + }) + .map(|tool| tool.name.clone()) + .collect(); + + assert!(!fields_tools.is_empty(), "no tools accept `fields`"); + assert!( + undocumented.is_empty(), + "tools that don't document `_unmatched_fields`: {undocumented:?}" + ); + + grpc_handle.abort(); +} + #[tokio::test] async fn client_event_failure_does_not_change_the_tool_result() { let reporter = ClientEventReporter::new( diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index 2f3bc4007d..b54df158b5 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/mod.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/mod.rs @@ -136,6 +136,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -160,7 +163,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs index 1eb34cb934..ee6e629f33 100644 --- a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs +++ b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs @@ -350,6 +350,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — raise `limit` or scope with `conversation_id` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `conversation_id`: optional. When set, only artifacts linked to that conversation. When omitted, @@ -381,7 +384,7 @@ impl SiftMcpServer { `[\"title\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `artifact_id`, `artifactId` and `artifact-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -471,6 +474,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with versions left over, so this page is not the whole history. Never report `count` as a total while `has_more` is `true` — raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `artifact_id`: required stable container id. Resolve it with `list_artifacts` first if you only @@ -481,7 +487,7 @@ impl SiftMcpServer { `[\"version\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `artifact_version_id`, `artifactVersionId` and `artifact-version-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/assets/mod.rs b/rust/crates/sift_mcp/src/tool/assets/mod.rs index b3e9babe71..0fecae0ae6 100644 --- a/rust/crates/sift_mcp/src/tool/assets/mod.rs +++ b/rust/crates/sift_mcp/src/tool/assets/mod.rs @@ -42,6 +42,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -61,7 +64,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. 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 8b01db110c..6cea63e1da 100644 --- a/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs @@ -89,6 +89,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -111,7 +114,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -177,6 +180,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `calculated_channel_id`: required. The calculated channel whose versions to list. Resolve it with @@ -197,7 +203,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/channels/mod.rs b/rust/crates/sift_mcp/src/tool/channels/mod.rs index 985b4cc72b..c102834e78 100644 --- a/rust/crates/sift_mcp/src/tool/channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/channels/mod.rs @@ -27,6 +27,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -45,7 +48,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/channels/test.rs b/rust/crates/sift_mcp/src/tool/channels/test.rs index f748ed63c4..858207043e 100644 --- a/rust/crates/sift_mcp/src/tool/channels/test.rs +++ b/rust/crates/sift_mcp/src/tool/channels/test.rs @@ -245,7 +245,7 @@ async fn list_channels_reports_a_field_that_matched_nothing() { body["channels"], serde_json::json!([{ "name": "throttle" }]) ); - assert_eq!(body["unmatched_fields"], serde_json::json!(["nmae"])); + assert_eq!(body["_unmatched_fields"], serde_json::json!(["nmae"])); } #[test] @@ -307,5 +307,5 @@ async fn list_channels_projects_asset_id_and_unit_id_but_not_asset_name() { body["channels"], serde_json::json!([{ "assetId": "a1", "unitId": "u1" }]) ); - assert_eq!(body["unmatched_fields"], serde_json::json!(["asset_name"])); + assert_eq!(body["_unmatched_fields"], serde_json::json!(["asset_name"])); } diff --git a/rust/crates/sift_mcp/src/tool/common/mod.rs b/rust/crates/sift_mcp/src/tool/common/mod.rs index c59964d1bf..50478f6ddf 100644 --- a/rust/crates/sift_mcp/src/tool/common/mod.rs +++ b/rust/crates/sift_mcp/src/tool/common/mod.rs @@ -175,10 +175,17 @@ pub(crate) fn project_fields(items: Vec, fields: &[String]) -> (Vec = (0..500) + .map(|i| json!({ "name": format!("channel-{i}") })) + .collect(); + + let text = list_body("channels", items, Some(vec!["nmae".into()]), false).to_string(); + let head: String = text.chars().take(200).collect(); + + assert!( + head.contains("\"_unmatched_fields\":[\"nmae\"]"), + "warning must sit in the first 200 characters: {head}" + ); +} diff --git a/rust/crates/sift_mcp/src/tool/data/mod.rs b/rust/crates/sift_mcp/src/tool/data/mod.rs index 631e9afd89..e6dbb3d0e7 100644 --- a/rust/crates/sift_mcp/src/tool/data/mod.rs +++ b/rust/crates/sift_mcp/src/tool/data/mod.rs @@ -736,7 +736,7 @@ impl SiftMcpServer { // Always present, even when empty. Omitting them would leave "every // channel arrived" and "this tool never checked" looking identical, and // the first is the assurance a caller needs before trusting the file. - // Deliberately unlike `unmatched_fields` on the list tools, where + // Deliberately unlike `_unmatched_fields` on the list tools, where // silence costs nothing. body.insert( "unmatched_channel_names".to_string(), diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index a82f632490..a146279e08 100644 --- a/rust/crates/sift_mcp/src/tool/rules/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rules/mod.rs @@ -72,6 +72,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -94,7 +97,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/runs/mod.rs b/rust/crates/sift_mcp/src/tool/runs/mod.rs index 42b3237a23..25cdb1627b 100644 --- a/rust/crates/sift_mcp/src/tool/runs/mod.rs +++ b/rust/crates/sift_mcp/src/tool/runs/mod.rs @@ -51,6 +51,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -75,7 +78,7 @@ impl SiftMcpServer { `asset-ids` all work. Name output fields, not filter fields: `asset_id`, `asset_name`, and `annotation_state` filter runs but are not on the item. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. diff --git a/rust/crates/sift_mcp/src/tool/runs/test.rs b/rust/crates/sift_mcp/src/tool/runs/test.rs index cc33564d86..6cf5d374b8 100644 --- a/rust/crates/sift_mcp/src/tool/runs/test.rs +++ b/rust/crates/sift_mcp/src/tool/runs/test.rs @@ -312,7 +312,7 @@ async fn list_runs_projects_asset_ids_but_not_filter_only_asset_fields() { let body = structured(resp); assert_eq!(body["runs"][0]["assetIds"], serde_json::json!(["a1", "a2"])); assert_eq!( - body["unmatched_fields"], + body["_unmatched_fields"], serde_json::json!(["asset_id", "asset_name"]) ); } 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..7d6265200e 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs @@ -62,6 +62,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -85,7 +88,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -145,6 +148,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (rarely useful; almost always @@ -169,7 +175,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -228,6 +234,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (almost always scope by @@ -251,7 +260,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. 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..6e7d51ab0d 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 @@ -91,6 +91,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -108,7 +111,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -175,6 +178,9 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. + - `_unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It is the first key in the response. Fix the + field name and call again; never report that field as empty or missing. Parameters: - `user_defined_function_id`: optional. The id of the function whose versions to list. @@ -195,7 +201,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `unmatched_fields`; an empty page reports none, since it + is listed in `_unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. From 19f1720064a0cedc0bfa897bdb6d68fefdd09928 Mon Sep 17 00:00:00 2001 From: Alexa Agnew Date: Mon, 26 Oct 2026 13:02:40 -0700 Subject: [PATCH 2/4] rust(fix): close the truncation test left open by the main merge Co-Authored-By: Claude Opus 5.5 --- rust/crates/sift_cli/CHANGELOG.md | 7 +++---- rust/crates/sift_mcp/src/tool/common/test.rs | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index eaf446772b..6b3c847d02 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -9,10 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). - MCP `list_*` tools now list any field requested in `fields` that matches no key on the returned items in `_unmatched_fields`, renamed from - `unmatched_fields`. The new name sorts - ahead of the items, so the warning comes first in the response and survives - when an agent harness truncates a large result. Each list tool's Output - section now documents the key. + `unmatched_fields`. The new name sorts ahead of the items, so the warning + comes first in the response and survives when an agent harness truncates a + large result. Each list tool's Output section now documents the key. ## [v0.7.6] - October 26, 2026 diff --git a/rust/crates/sift_mcp/src/tool/common/test.rs b/rust/crates/sift_mcp/src/tool/common/test.rs index 54f8a07fbc..cc3e15067b 100644 --- a/rust/crates/sift_mcp/src/tool/common/test.rs +++ b/rust/crates/sift_mcp/src/tool/common/test.rs @@ -310,6 +310,7 @@ fn list_body_unmatched_fields_survive_head_truncation() { head.contains("\"_unmatched_fields\":[\"nmae\"]"), "warning must sit in the first 200 characters: {head}" ); +} /// 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 From c0d8be4df8feffab230762a7e1f5f8870f2ec3e3 Mon Sep 17 00:00:00 2001 From: Alexa Agnew Date: Wed, 28 Oct 2026 21:00:10 -0700 Subject: [PATCH 3/4] rust(fix): write list metadata ahead of the items in the text result Replace the `_unmatched_fields` rename with a `list_result` helper. The text block agents read now writes `count`, `has_more`, `unmatched_fields` and other metadata before the items, so they survive head truncation in every list tool, including the 7 whose items key sorts before `count`. `structuredContent` is unchanged, and the key keeps its original `unmatched_fields` name. ENG-15730 Co-Authored-By: Claude Opus 5.5 --- rust/crates/sift_mcp/CLAUDE.md | 5 +- rust/crates/sift_mcp/src/server/test.rs | 11 +- .../sift_mcp/src/tool/annotations/mod.rs | 20 +-- .../crates/sift_mcp/src/tool/artifacts/mod.rs | 31 ++--- .../sift_mcp/src/tool/artifacts/test.rs | 7 +- rust/crates/sift_mcp/src/tool/assets/mod.rs | 20 +-- .../src/tool/calculated_channels/mod.rs | 35 +++--- .../src/tool/calculated_channels/test.rs | 11 +- rust/crates/sift_mcp/src/tool/channels/mod.rs | 22 ++-- .../crates/sift_mcp/src/tool/channels/test.rs | 17 ++- rust/crates/sift_mcp/src/tool/common/mod.rs | 81 ++++++++++++- rust/crates/sift_mcp/src/tool/common/test.rs | 114 ++++++++++++------ rust/crates/sift_mcp/src/tool/data/mod.rs | 2 +- rust/crates/sift_mcp/src/tool/rules/mod.rs | 20 +-- rust/crates/sift_mcp/src/tool/runs/mod.rs | 20 +-- rust/crates/sift_mcp/src/tool/runs/test.rs | 2 +- .../sift_mcp/src/tool/test_reports/mod.rs | 61 +++++----- .../src/tool/user_defined_functions/mod.rs | 43 ++++--- 18 files changed, 336 insertions(+), 186 deletions(-) diff --git a/rust/crates/sift_mcp/CLAUDE.md b/rust/crates/sift_mcp/CLAUDE.md index 776ce9f3cf..06d8e46f56 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -683,8 +683,9 @@ 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 taking `fields` builds its body with `list_body` and documents - `_unmatched_fields` in both its `fields` bullet and its Output section. +- [ ] Any `list_*` tool taking `fields` builds its body with `list_body`, returns it through + `list_result`, and documents `unmatched_fields` in both its `fields` bullet and its + Output section. - [ ] `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/server/test.rs b/rust/crates/sift_mcp/src/server/test.rs index b820c9bae3..26b8326241 100644 --- a/rust/crates/sift_mcp/src/server/test.rs +++ b/rust/crates/sift_mcp/src/server/test.rs @@ -713,9 +713,9 @@ async fn every_registered_tool_has_a_client_event() { } /// Agents learn the projection warning's name only from the tool description. -/// Every tool that accepts `fields` must name `_unmatched_fields` in both its -/// `fields` bullet and its Output section, and never the pre-rename key, or the -/// agent will look for a key the response no longer carries. +/// Every tool that accepts `fields` must name `unmatched_fields` in both its +/// `fields` bullet and its Output section, or the agent has no way to know a +/// mistyped field name is reported at all. #[tokio::test] async fn every_fields_tool_documents_unmatched_fields() { let current = UpdateCheck::Current { @@ -744,8 +744,7 @@ async fn every_fields_tool_documents_unmatched_fields() { .iter() .filter(|tool| { let description = tool.description.as_deref().unwrap_or_default(); - description.matches("`_unmatched_fields`").count() < 2 - || description.contains("`unmatched_fields`") + description.matches("`unmatched_fields`").count() < 2 }) .map(|tool| tool.name.clone()) .collect(); @@ -753,7 +752,7 @@ async fn every_fields_tool_documents_unmatched_fields() { assert!(!fields_tools.is_empty(), "no tools accept `fields`"); assert!( undocumented.is_empty(), - "tools that don't document `_unmatched_fields`: {undocumented:?}" + "tools that don't document `unmatched_fields`: {undocumented:?}" ); grpc_handle.abort(); diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index b54df158b5..e41f2c5a4d 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)] @@ -136,9 +136,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -163,7 +164,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -201,12 +202,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 ee6e629f33..fd65d46dc6 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)] @@ -350,9 +350,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — raise `limit` or scope with `conversation_id` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `conversation_id`: optional. When set, only artifacts linked to that conversation. When omitted, @@ -384,7 +385,7 @@ impl SiftMcpServer { `[\"title\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `artifact_id`, `artifactId` and `artifact-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -445,12 +446,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( @@ -474,9 +474,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with versions left over, so this page is not the whole history. Never report `count` as a total while `has_more` is `true` — raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `artifact_id`: required stable container id. Resolve it with `list_artifacts` first if you only @@ -487,7 +488,7 @@ impl SiftMcpServer { `[\"version\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `artifact_version_id`, `artifactVersionId` and `artifact-version-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -537,7 +538,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 0fecae0ae6..fb33ad9ad0 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)] @@ -42,9 +42,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -64,7 +65,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -99,12 +100,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 1aa7b6f7af..ce5d410d74 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)] @@ -89,9 +89,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -114,7 +115,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -155,12 +156,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( @@ -180,9 +180,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `calculated_channel_id`: required. The calculated channel whose versions to list. Resolve it with @@ -203,7 +204,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -258,7 +259,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 c102834e78..58d32299e2 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)] @@ -27,9 +27,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -48,7 +49,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -83,11 +84,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 858207043e..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,12 +241,22 @@ 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"], serde_json::json!([{ "name": "throttle" }]) ); - assert_eq!(body["_unmatched_fields"], serde_json::json!(["nmae"])); + assert_eq!(body["unmatched_fields"], serde_json::json!(["nmae"])); } #[test] @@ -307,5 +318,5 @@ async fn list_channels_projects_asset_id_and_unit_id_but_not_asset_name() { body["channels"], serde_json::json!([{ "assetId": "a1", "unitId": "u1" }]) ); - assert_eq!(body["_unmatched_fields"], serde_json::json!(["asset_name"])); + assert_eq!(body["unmatched_fields"], serde_json::json!(["asset_name"])); } diff --git a/rust/crates/sift_mcp/src/tool/common/mod.rs b/rust/crates/sift_mcp/src/tool/common/mod.rs index 8c79f8bf98..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::{ @@ -177,11 +178,8 @@ 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 @@ -311,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 cc3e15067b..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}; @@ -170,7 +172,7 @@ fn list_body_surfaces_unmatched_fields_alongside_the_items() { "channels": [{ "name": "throttle" }], "count": 1, "has_more": false, - "_unmatched_fields": ["nope"], + "unmatched_fields": ["nope"], }) ); } @@ -257,59 +259,97 @@ fn list_body_omits_unmatched_fields_when_the_page_is_empty() { assert_eq!(body["count"], json!(0)); assert!( - body.get("_unmatched_fields").is_none(), + body.get("unmatched_fields").is_none(), "an empty page must not look like a spelling problem: {body}" ); } -#[test] -fn list_body_writes_unmatched_fields_before_the_items() { - // Agent harnesses cut oversized tool results from the end. The warning has - // to be serialized ahead of the items or it is the first thing lost, on - // exactly the large responses where a typo is hardest to spot. - let keys = [ - "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", - ]; +/// 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", +]; - for key in keys { +#[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 text = list_body(key, items, Some(vec!["nmae".into()]), false).to_string(); + let body = list_body(key, items, Some(vec!["nmae".into()]), true); - let warning = text.find("\"_unmatched_fields\"").expect("warning present"); - let items_at = text.find(&format!("\"{key}\"")).expect("items present"); - assert!( - warning < items_at, - "{key}: warning must precede the items in {text}" - ); + 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_body_unmatched_fields_survive_head_truncation() { +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 = list_body("channels", items, Some(vec!["nmae".into()]), false).to_string(); + let text = content_texts(&list_result(body, "channels", None)).remove(0); let head: String = text.chars().take(200).collect(); - assert!( - head.contains("\"_unmatched_fields\":[\"nmae\"]"), - "warning must sit in the first 200 characters: {head}" + 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`: diff --git a/rust/crates/sift_mcp/src/tool/data/mod.rs b/rust/crates/sift_mcp/src/tool/data/mod.rs index e6dbb3d0e7..631e9afd89 100644 --- a/rust/crates/sift_mcp/src/tool/data/mod.rs +++ b/rust/crates/sift_mcp/src/tool/data/mod.rs @@ -736,7 +736,7 @@ impl SiftMcpServer { // Always present, even when empty. Omitting them would leave "every // channel arrived" and "this tool never checked" looking identical, and // the first is the assurance a caller needs before trusting the file. - // Deliberately unlike `_unmatched_fields` on the list tools, where + // Deliberately unlike `unmatched_fields` on the list tools, where // silence costs nothing. body.insert( "unmatched_channel_names".to_string(), diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index 021d7ea76c..4ef57aac4e 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)] @@ -72,9 +72,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -97,7 +98,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -133,12 +134,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 25cdb1627b..e89f19bfd3 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)] @@ -51,9 +51,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -78,7 +79,7 @@ impl SiftMcpServer { `asset-ids` all work. Name output fields, not filter fields: `asset_id`, `asset_name`, and `annotation_state` filter runs but are not on the item. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -116,12 +117,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/runs/test.rs b/rust/crates/sift_mcp/src/tool/runs/test.rs index 6cf5d374b8..cc33564d86 100644 --- a/rust/crates/sift_mcp/src/tool/runs/test.rs +++ b/rust/crates/sift_mcp/src/tool/runs/test.rs @@ -312,7 +312,7 @@ async fn list_runs_projects_asset_ids_but_not_filter_only_asset_fields() { let body = structured(resp); assert_eq!(body["runs"][0]["assetIds"], serde_json::json!(["a1", "a2"])); assert_eq!( - body["_unmatched_fields"], + body["unmatched_fields"], serde_json::json!(["asset_id", "asset_name"]) ); } 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 7d6265200e..a90556e77d 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)] @@ -62,9 +62,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -88,7 +89,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -121,12 +122,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( @@ -148,9 +148,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (rarely useful; almost always @@ -175,7 +176,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -206,12 +207,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( @@ -234,9 +234,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (almost always scope by @@ -260,7 +261,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -291,12 +292,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 6e7d51ab0d..416f8b1061 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)] @@ -91,9 +91,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -111,7 +112,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -152,12 +153,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( @@ -178,9 +178,10 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `_unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It is the first key in the response. Fix the - field name and call again; never report that field as empty or missing. + - `unmatched_fields`: present only when a field you listed in `fields` matched + no key on any returned item. It comes before the items in the response. + Fix the field name and call again; never report that field as empty or + missing. Parameters: - `user_defined_function_id`: optional. The id of the function whose versions to list. @@ -201,7 +202,7 @@ impl SiftMcpServer { `[\"name\"]`. Omit it for the full object. Names match case-insensitively and ignore underscores and hyphens, so `asset_id`, `assetId` and `asset-id` all work. Any name that matched nothing on any returned item - is listed in `_unmatched_fields`; an empty page reports none, since it + is listed in `unmatched_fields`; an empty page reports none, since it says nothing about whether a name was spelled right. Reach for this whenever you need only a few fields: full objects are wide, and a large listing can exceed the response size limit without it. @@ -250,12 +251,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( From 561f709ae68c5184e7567a8434e3674147158e4e Mon Sep 17 00:00:00 2001 From: Alexa Agnew Date: Thu, 29 Oct 2026 12:34:08 -0700 Subject: [PATCH 4/4] rust(chore): drop the unmatched_fields Output bullets The bullets were added so agents would learn the `_unmatched_fields` rename. With the rename reverted, each tool's `fields` bullet already names the key, so remove the bullets, their docs guard test, and the CHANGELOG mention. ENG-15730 Co-Authored-By: Claude Opus 5.5 --- rust/crates/sift_cli/CHANGELOG.md | 3 +- rust/crates/sift_mcp/CLAUDE.md | 5 +- rust/crates/sift_mcp/src/server/test.rs | 46 ------------------- .../sift_mcp/src/tool/annotations/mod.rs | 4 -- .../crates/sift_mcp/src/tool/artifacts/mod.rs | 8 ---- rust/crates/sift_mcp/src/tool/assets/mod.rs | 4 -- .../src/tool/calculated_channels/mod.rs | 8 ---- rust/crates/sift_mcp/src/tool/channels/mod.rs | 4 -- rust/crates/sift_mcp/src/tool/rules/mod.rs | 4 -- rust/crates/sift_mcp/src/tool/runs/mod.rs | 4 -- .../sift_mcp/src/tool/test_reports/mod.rs | 12 ----- .../src/tool/user_defined_functions/mod.rs | 8 ---- 12 files changed, 3 insertions(+), 107 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index eb13159074..562c124874 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -10,8 +10,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - 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. Each list tool's Output section now - documents `unmatched_fields`. + `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 diff --git a/rust/crates/sift_mcp/CLAUDE.md b/rust/crates/sift_mcp/CLAUDE.md index 06d8e46f56..c5932a170c 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -683,9 +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 taking `fields` builds its body with `list_body`, returns it through - `list_result`, and documents `unmatched_fields` in both its `fields` bullet and its - Output section. +- [ ] 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/server/test.rs b/rust/crates/sift_mcp/src/server/test.rs index 26b8326241..0124f90a3d 100644 --- a/rust/crates/sift_mcp/src/server/test.rs +++ b/rust/crates/sift_mcp/src/server/test.rs @@ -712,52 +712,6 @@ async fn every_registered_tool_has_a_client_event() { grpc_handle.abort(); } -/// Agents learn the projection warning's name only from the tool description. -/// Every tool that accepts `fields` must name `unmatched_fields` in both its -/// `fields` bullet and its Output section, or the agent has no way to know a -/// mistyped field name is reported at all. -#[tokio::test] -async fn every_fields_tool_documents_unmatched_fields() { - let current = UpdateCheck::Current { - current_version: "0.4.0".to_string(), - latest_version: "0.4.0".to_string(), - }; - let (server, grpc_handle) = server_with_feature_flags( - Some(receiver(current)), - 0, - ClientEventReporter::default(), - all_feature_flags(), - ) - .await; - let fields_tools: Vec<_> = server - .tool_router - .list_all() - .into_iter() - .filter(|tool| { - tool.input_schema - .get("properties") - .and_then(|properties| properties.get("fields")) - .is_some() - }) - .collect(); - let undocumented: Vec<_> = fields_tools - .iter() - .filter(|tool| { - let description = tool.description.as_deref().unwrap_or_default(); - description.matches("`unmatched_fields`").count() < 2 - }) - .map(|tool| tool.name.clone()) - .collect(); - - assert!(!fields_tools.is_empty(), "no tools accept `fields`"); - assert!( - undocumented.is_empty(), - "tools that don't document `unmatched_fields`: {undocumented:?}" - ); - - grpc_handle.abort(); -} - #[tokio::test] async fn client_event_failure_does_not_change_the_tool_result() { let reporter = ClientEventReporter::new( diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index e41f2c5a4d..1f405c379c 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/mod.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/mod.rs @@ -136,10 +136,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: diff --git a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs index fd65d46dc6..80097ead28 100644 --- a/rust/crates/sift_mcp/src/tool/artifacts/mod.rs +++ b/rust/crates/sift_mcp/src/tool/artifacts/mod.rs @@ -350,10 +350,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — raise `limit` or scope with `conversation_id` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `conversation_id`: optional. When set, only artifacts linked to that conversation. When omitted, @@ -474,10 +470,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with versions left over, so this page is not the whole history. Never report `count` as a total while `has_more` is `true` — raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `artifact_id`: required stable container id. Resolve it with `list_artifacts` first if you only diff --git a/rust/crates/sift_mcp/src/tool/assets/mod.rs b/rust/crates/sift_mcp/src/tool/assets/mod.rs index fb33ad9ad0..ac5c8a3f58 100644 --- a/rust/crates/sift_mcp/src/tool/assets/mod.rs +++ b/rust/crates/sift_mcp/src/tool/assets/mod.rs @@ -42,10 +42,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: 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 ce5d410d74..45e42c5e41 100644 --- a/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/calculated_channels/mod.rs @@ -89,10 +89,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -180,10 +176,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `calculated_channel_id`: required. The calculated channel whose versions to list. Resolve it with diff --git a/rust/crates/sift_mcp/src/tool/channels/mod.rs b/rust/crates/sift_mcp/src/tool/channels/mod.rs index 58d32299e2..4b417be1fe 100644 --- a/rust/crates/sift_mcp/src/tool/channels/mod.rs +++ b/rust/crates/sift_mcp/src/tool/channels/mod.rs @@ -27,10 +27,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index 4ef57aac4e..46dc89a738 100644 --- a/rust/crates/sift_mcp/src/tool/rules/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rules/mod.rs @@ -72,10 +72,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: diff --git a/rust/crates/sift_mcp/src/tool/runs/mod.rs b/rust/crates/sift_mcp/src/tool/runs/mod.rs index e89f19bfd3..c8d6bdbc0a 100644 --- a/rust/crates/sift_mcp/src/tool/runs/mod.rs +++ b/rust/crates/sift_mcp/src/tool/runs/mod.rs @@ -51,10 +51,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: 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 a90556e77d..cc0639f8a7 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs @@ -62,10 +62,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -148,10 +144,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (rarely useful; almost always @@ -234,10 +226,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything (almost always scope by 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 416f8b1061..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 @@ -91,10 +91,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -178,10 +174,6 @@ impl SiftMcpServer { - `has_more`: `true` when the service hit `limit` with matches left over, so this page is not the whole set. Never report `count` as a total while `has_more` is `true` — narrow `filter` or raise `limit` and ask again. - - `unmatched_fields`: present only when a field you listed in `fields` matched - no key on any returned item. It comes before the items in the response. - Fix the field name and call again; never report that field as empty or - missing. Parameters: - `user_defined_function_id`: optional. The id of the function whose versions to list.