diff --git a/rust/crates/sift_mcp/CLAUDE.md b/rust/crates/sift_mcp/CLAUDE.md index f8a368b7f6..6644dee492 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -492,7 +492,7 @@ When you add or update a list tool: your own words risks drift; quoting from the proto keeps the tool aligned with the API. 3. **Map every wrapped field to a bullet** under `Parameters:`, following Step 4's structure: - Filter: list every filterable field named in the proto's `filter` comment. Preserve - metadata-key syntax (`metadata.{key}`) and CEL helpers (`duration(...)`). + metadata-key syntax (`metadata[""]`) and CEL helpers (`duration(...)`). - Order-by: list every orderable field, the default sort when the field is empty (assets and runs default to `created_date desc`; channels defaults to `created_date` ascending — these differ, do not assume), and the `\"FIELD_NAME[ desc],...\"` format. diff --git a/rust/crates/sift_mcp/src/server/test.rs b/rust/crates/sift_mcp/src/server/test.rs index 8ae0230193..0124f90a3d 100644 --- a/rust/crates/sift_mcp/src/server/test.rs +++ b/rust/crates/sift_mcp/src/server/test.rs @@ -193,6 +193,58 @@ async fn feature_flag_gates_test_report_tools() { enabled_handle.abort(); } +/// The `filter` parameter bullet of a tool description, up to the next bullet or blank line. +fn filter_bullet(description: &str) -> String { + let mut lines = description + .lines() + .skip_while(|line| !line.trim_start().starts_with("- `filter`:")); + let Some(first) = lines.next() else { + return String::new(); + }; + let rest = lines.take_while(|line| { + let line = line.trim_start(); + !line.is_empty() && !line.starts_with("- ") + }); + std::iter::once(first) + .chain(rest) + .collect::>() + .join("\n") +} + +/// The filter service only declares metadata keys as `metadata[""]` and rejects +/// `metadata.`. An agent that copies a dot-form example concludes metadata can't +/// be filtered, so every `filter` that takes metadata must show the bracket form and no +/// description may show a dot-form key. +#[tokio::test] +async fn tool_descriptions_use_bracket_metadata_syntax() { + let (server, handle) = + server_with_feature_flags(None, 0, ClientEventReporter::default(), all_feature_flags()) + .await; + let mut metadata_filters = 0; + for tool in server.tool_router.list_all() { + let description = tool.description.as_deref().unwrap_or_default(); + let filter = filter_bullet(description); + if filter.contains("metadata") { + metadata_filters += 1; + assert!( + filter.contains(r#"`metadata[""]`"#), + "{} filters on metadata without showing the bracket syntax", + tool.name + ); + } + for (i, _) in description.match_indices("metadata.") { + let next = description[i + "metadata.".len()..].chars().next(); + assert!( + !next.is_some_and(|c| c == '{' || c == '_' || c.is_ascii_alphanumeric()), + "{} shows the unsupported dot metadata syntax", + tool.name + ); + } + } + assert!(metadata_filters > 0, "no `filter` bullet mentions metadata"); + handle.abort(); +} + #[tokio::test] async fn each_feature_flag_gates_only_its_own_tools() { for &(_, flag) in TOOL_FEATURE_FLAGS { diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index 7196565706..2f3bc4007d 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/mod.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/mod.rs @@ -143,7 +143,7 @@ impl SiftMcpServer { `description`, `state`, `created_by_user_id`, `created_by_rule_condition_version_id`, `rule_id`, `annotation_type`, `tag_name`, `report_id`, `asset_id`, `asset_name`, `pending`, `assignee`, `campaign_reports`, `metadata`, `archived_date`, `is_archived`. Reference metadata entries as - `metadata.{key}` (e.g. `metadata.severity == \"high\"`). + `metadata[\"\"]` (e.g. `metadata[\"severity\"] == \"high\"`). Compare `state` with `ANNOTATION_STATE_OPEN`, `ANNOTATION_STATE_FLAGGED`, or `ANNOTATION_STATE_RESOLVED`. Compare `annotation_type` with `ANNOTATION_TYPE_DATA_REVIEW` or `ANNOTATION_TYPE_PHASE`. Use these exact string enum values in quoted filter operands. diff --git a/rust/crates/sift_mcp/src/tool/assets/mod.rs b/rust/crates/sift_mcp/src/tool/assets/mod.rs index 9095cb820a..b3e9babe71 100644 --- a/rust/crates/sift_mcp/src/tool/assets/mod.rs +++ b/rust/crates/sift_mcp/src/tool/assets/mod.rs @@ -47,7 +47,7 @@ impl SiftMcpServer { - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: `asset_id`, `name`, `name_lower`, `tag_id`, `tag_name`, `created_date`, `modified_date`, `archived_date`, `is_archived`, `created_by_user_id`, `modified_by_user_id`, `metadata`. - Reference metadata entries as `metadata.{key}` (e.g. `metadata.vehicle_type == \"rover\"`). + Reference metadata entries as `metadata[\"\"]` (e.g. `metadata[\"vehicle_type\"] == \"rover\"`). When searching by name, use `name_lower.contains(\"rover\")`: `name_lower` is an indexed lowercase copy of `name` and exists only on assets. On any other text field use `name.matches(\"(?i)rover\")`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are diff --git a/rust/crates/sift_mcp/src/tool/report_templates/mod.rs b/rust/crates/sift_mcp/src/tool/report_templates/mod.rs index 3eacf0f958..863bb294f2 100644 --- a/rust/crates/sift_mcp/src/tool/report_templates/mod.rs +++ b/rust/crates/sift_mcp/src/tool/report_templates/mod.rs @@ -70,7 +70,7 @@ impl SiftMcpServer { Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: `report_template_id`, `tag_id`, `tag_name`, `client_key`, `metadata`, `name`, `is_archived`. - Reference metadata entries as `metadata.{key}` (e.g. `metadata.owner == \"qa-team\"`). + Reference metadata entries as `metadata[\"\"]` (e.g. `metadata[\"owner\"] == \"qa-team\"`). When filtering or searching, use `name.matches(\"(?i)avionics\")`, not `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Avionics\")` silently misses `avionics-power-limit`. diff --git a/rust/crates/sift_mcp/src/tool/reports/mod.rs b/rust/crates/sift_mcp/src/tool/reports/mod.rs index 6569adf00c..4b73e3bc3e 100644 --- a/rust/crates/sift_mcp/src/tool/reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/reports/mod.rs @@ -73,7 +73,7 @@ impl SiftMcpServer { `report_id`, `report_template_id`, `tag_name`, `name`, `run_id`, `report_type`, `is_archived`, `archived_date`, `created_date`, `created_by_user_id`, `metadata`, `modified_date`, `modified_by_user_id`. - Reference metadata entries as `metadata.{key}` (e.g. `metadata.batch == \"nightly\"`). + Reference metadata entries as `metadata[\"\"]` (e.g. `metadata[\"batch\"] == \"nightly\"`). When filtering or searching, use `name.matches(\"(?i)nightly\")`, not `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Nightly\")` silently misses `nightly-regression`. diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index eddbfa2ac6..a82f632490 100644 --- a/rust/crates/sift_mcp/src/tool/rules/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rules/mod.rs @@ -78,7 +78,7 @@ impl SiftMcpServer { `rule_id`, `client_key`, `name`, `description`, `is_external`, `asset_id`, `tag_id`, `created_date`, `created_by_user_id`, `metadata`, `modified_date`, `modified_by_user_id`, `deleted_date`, `is_archived`, `archived_date`, `is_live_evaluation_enabled`. - Reference metadata entries as `metadata.{key}` (e.g. `metadata.severity == \"high\"`). + Reference metadata entries as `metadata[\"\"]` (e.g. `metadata[\"severity\"] == \"high\"`). When filtering or searching, use `name.matches(\"(?i)avionics\")`, not `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Avionics\")` silently misses `avionics-power-limit`. diff --git a/rust/crates/sift_mcp/src/tool/runs/mod.rs b/rust/crates/sift_mcp/src/tool/runs/mod.rs index b1f230ecfc..6f19244460 100644 --- a/rust/crates/sift_mcp/src/tool/runs/mod.rs +++ b/rust/crates/sift_mcp/src/tool/runs/mod.rs @@ -56,7 +56,7 @@ impl SiftMcpServer { `start_time`, `stop_time`, `duration`, `duration_string`, `tag_id`, `asset_tag_id`, `annotation_comments_count`, `annotation_state`, `created_date`, `modified_date`, `archived_date`, `is_archived`, `created_by_user_id`, `modified_by_user_id`, `metadata`. - Reference metadata entries as `metadata.{key}`. + Reference metadata entries as `metadata[\"\"]`. `duration` is elapsed seconds (numeric). `duration_string` accepts `h`/`m`/`s`/`ms` suffixes via the `duration(...)` helper, e.g. `duration_string > duration('10h')`. When filtering or searching, use `name.matches(\"(?i)rover\")`, not `==`. Use `==` only for an 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 b64898f77e..1ecbd354c1 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs @@ -68,7 +68,7 @@ impl SiftMcpServer { `test_report_id`, `status`, `name`, `test_system_name`, `test_case`, `start_time`, `end_time`, `serial_number`, `part_number`, `system_operator`, `run_id`, `archived_date`, `created_by_user_id`, `modified_by_user_id`, `metadata`. Reference metadata entries as - `metadata.{key}`. `status` matches the `TestStatus` enum: `TEST_STATUS_DRAFT`, + `metadata[\"\"]`. `status` matches the `TestStatus` enum: `TEST_STATUS_DRAFT`, `TEST_STATUS_PASSED`, `TEST_STATUS_FAILED`, `TEST_STATUS_ABORTED`, `TEST_STATUS_ERROR`, `TEST_STATUS_IN_PROGRESS`, `TEST_STATUS_SKIPPED`. Filtering `test_report_id == \"...\"` fetches one report (there is no separate get tool). @@ -150,9 +150,10 @@ impl SiftMcpServer { - `filter`: CEL expression. Pass an empty string to list everything (rarely useful; almost always scope by `test_report_id`). Filterable fields: `test_step_id`, `test_report_id`, `parent_step_id`, `name`, `description`, `step_type`, `step_path`, `status`, `start_time`, `end_time`, `error_code`, - `error_message`, `created_date`, `modified_date`, `metadata`. `step_type` matches the - `TestStepType` enum: `TEST_STEP_TYPE_SEQUENCE`, `TEST_STEP_TYPE_GROUP`, `TEST_STEP_TYPE_ACTION`, - `TEST_STEP_TYPE_FLOW_CONTROL`. `status` matches `TestStatus` (see `list_test_reports`). + `error_message`, `created_date`, `modified_date`, `metadata`. Reference metadata entries as + `metadata[\"\"]`. `step_type` matches the `TestStepType` enum: `TEST_STEP_TYPE_SEQUENCE`, + `TEST_STEP_TYPE_GROUP`, `TEST_STEP_TYPE_ACTION`, `TEST_STEP_TYPE_FLOW_CONTROL`. `status` matches + `TestStatus` (see `list_test_reports`). When filtering or searching `name`, `description`, or `error_message`, use `name.matches(\"(?i)power\")`, not `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Power\")` silently misses @@ -232,9 +233,10 @@ impl SiftMcpServer { - `filter`: CEL expression. Pass an empty string to list everything (almost always scope by `test_report_id` or `test_step_id`). Filterable fields: `measurement_id`, `measurement_type`, `name`, `test_step_id`, `test_report_id`, `numeric_value`, `string_value`, `boolean_value`, - `passed`, `timestamp`, `created_date`, `modified_date`, `metadata`. `measurement_type` matches the - `TestMeasurementType` enum: `TEST_MEASUREMENT_TYPE_DOUBLE`, `TEST_MEASUREMENT_TYPE_STRING`, - `TEST_MEASUREMENT_TYPE_BOOLEAN`, `TEST_MEASUREMENT_TYPE_LIMIT`. + `passed`, `timestamp`, `created_date`, `modified_date`, `metadata`. Reference metadata entries as + `metadata[\"\"]`. `measurement_type` matches the `TestMeasurementType` enum: + `TEST_MEASUREMENT_TYPE_DOUBLE`, `TEST_MEASUREMENT_TYPE_STRING`, `TEST_MEASUREMENT_TYPE_BOOLEAN`, + `TEST_MEASUREMENT_TYPE_LIMIT`. When filtering or searching `name`, use `name.matches(\"(?i)voltage\")`, not `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Voltage\")` silently misses `voltage_bus_a`. @@ -302,10 +304,10 @@ impl SiftMcpServer { - `filter`: CEL expression. Pass an empty string to count all steps. Filterable fields are identical to `list_test_steps`: `test_step_id`, `test_report_id`, `parent_step_id`, `name`, `description`, `step_type`, `step_path`, `status`, `start_time`, `end_time`, `error_code`, `error_message`, - `created_date`, `modified_date`, `metadata`. When filtering `name`, `description`, or - `error_message`, use `name.matches(\"(?i)power\")`, not `==`. Use `==` only for an exact value from - a prior result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Power\")` - silently misses `power-on`. + `created_date`, `modified_date`, `metadata`. Reference metadata entries as `metadata[\"\"]`. + When filtering `name`, `description`, or `error_message`, use `name.matches(\"(?i)power\")`, not + `==`. Use `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are + case-SENSITIVE: `contains(\"Power\")` silently misses `power-on`. Errors: - `INVALID_PARAMS` if `filter` is not a valid CEL expression. @@ -340,10 +342,10 @@ impl SiftMcpServer { - `filter`: CEL expression. Pass an empty string to count all measurements. Filterable fields are identical to `list_test_measurements`: `measurement_id`, `measurement_type`, `name`, `test_step_id`, `test_report_id`, `numeric_value`, `string_value`, `boolean_value`, `passed`, - `timestamp`, `created_date`, `modified_date`, `metadata`. When filtering `name`, use - `name.matches(\"(?i)voltage\")`, not `==`. Use `==` only for an exact value from a prior result. - `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"Voltage\")` silently misses - `voltage_bus_a`. + `timestamp`, `created_date`, `modified_date`, `metadata`. Reference metadata entries as + `metadata[\"\"]`. When filtering `name`, use `name.matches(\"(?i)voltage\")`, not `==`. Use + `==` only for an exact value from a prior result. `contains`/`startsWith`/`endsWith` are + case-SENSITIVE: `contains(\"Voltage\")` silently misses `voltage_bus_a`. Errors: - `INVALID_PARAMS` if `filter` is not a valid CEL expression.