From 2d7d7808caabd0567f7ebedd811551873a1b35a1 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Tue, 18 Aug 2026 22:14:29 -0700 Subject: [PATCH 1/5] feat: add preview_rule MCP tool Add a preview_rule tool over the rule evaluation service's dry-run RPC. Supports evaluating a saved rule (by id or name, resolved through the existing rules service) or an ad-hoc draft rule config against a run, without persisting anything. --- rust/crates/sift_mcp/src/server/mod.rs | 10 +- rust/crates/sift_mcp/src/service/mod.rs | 1 + .../src/service/rule_evaluation/mod.rs | 91 ++++++ .../src/service/rule_evaluation/test.rs | 146 +++++++++ rust/crates/sift_mcp/src/tool/mod.rs | 1 + .../sift_mcp/src/tool/rule_evaluation/mod.rs | 197 ++++++++++++ .../sift_mcp/src/tool/rule_evaluation/test.rs | 290 ++++++++++++++++++ rust/crates/sift_mcp/src/tool_events.json | 1 + 8 files changed, 735 insertions(+), 2 deletions(-) create mode 100644 rust/crates/sift_mcp/src/service/rule_evaluation/mod.rs create mode 100644 rust/crates/sift_mcp/src/service/rule_evaluation/test.rs create mode 100644 rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs create mode 100644 rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs diff --git a/rust/crates/sift_mcp/src/server/mod.rs b/rust/crates/sift_mcp/src/server/mod.rs index 81f26bff7..5aafa0ca5 100644 --- a/rust/crates/sift_mcp/src/server/mod.rs +++ b/rust/crates/sift_mcp/src/server/mod.rs @@ -42,8 +42,9 @@ use crate::service::{ annotations::AnnotationService, assets::AssetService, calculated_channels::CalculatedChannelService, channels::ChannelService, data::DataService, docs::DocsService, ingest::IngestService, ping::PingService, - report_templates::ReportTemplateService, reports::ReportService, rules::RuleService, - runs::RunService, url::UrlService, users::UserService, + report_templates::ReportTemplateService, reports::ReportService, + rule_evaluation::RuleEvaluationService, rules::RuleService, runs::RunService, url::UrlService, + users::UserService, }; #[derive(Clone)] @@ -63,6 +64,7 @@ pub struct SiftMcpServer { pub report_service: ReportService, pub report_template_service: ReportTemplateService, pub rule_service: RuleService, + pub rule_evaluation_service: RuleEvaluationService, #[cfg(feature = "test-reports")] pub test_report_service: TestReportService, pub docs_service: DocsService, @@ -188,6 +190,7 @@ impl SiftMcpServer { tool_router.merge(Self::explore_router()); tool_router.merge(Self::ping_router()); tool_router.merge(Self::rules_router()); + tool_router.merge(Self::rule_evaluation_router()); tool_router.merge(Self::annotations_router()); #[cfg(feature = "test-reports")] tool_router.merge(Self::test_reports_router()); @@ -215,6 +218,8 @@ impl SiftMcpServer { let report_template_service = ReportTemplateService::new(channel.clone(), retry_policy.clone()); let rule_service = RuleService::new(channel.clone(), retry_policy.clone()); + let rule_evaluation_service = + RuleEvaluationService::new(channel.clone(), retry_policy.clone()); #[cfg(feature = "test-reports")] let test_report_service = TestReportService::new(channel.clone(), retry_policy.clone()); let docs_service = DocsService::new(channel.clone(), retry_policy.clone()); @@ -233,6 +238,7 @@ impl SiftMcpServer { report_service, report_template_service, rule_service, + rule_evaluation_service, #[cfg(feature = "test-reports")] test_report_service, docs_service, diff --git a/rust/crates/sift_mcp/src/service/mod.rs b/rust/crates/sift_mcp/src/service/mod.rs index fe778f3e8..6eaccfc4d 100644 --- a/rust/crates/sift_mcp/src/service/mod.rs +++ b/rust/crates/sift_mcp/src/service/mod.rs @@ -8,6 +8,7 @@ pub mod ingest; pub mod ping; pub mod report_templates; pub mod reports; +pub mod rule_evaluation; pub mod rules; pub mod runs; #[cfg(feature = "test-reports")] diff --git a/rust/crates/sift_mcp/src/service/rule_evaluation/mod.rs b/rust/crates/sift_mcp/src/service/rule_evaluation/mod.rs new file mode 100644 index 000000000..869058c01 --- /dev/null +++ b/rust/crates/sift_mcp/src/service/rule_evaluation/mod.rs @@ -0,0 +1,91 @@ +use crate::policy::{RetryPolicy, with_retry}; +use anyhow::{Context, Result}; +use sift_rs::{ + SiftChannel, + common::r#type::v1::{ + Ids, ResourceIdentifier, ResourceIdentifiers, resource_identifier, resource_identifiers, + }, + rule_evaluation::v1::{ + EvaluateRulesFromCurrentRuleVersions, EvaluateRulesFromRuleConfigs, + EvaluateRulesPreviewRequest, EvaluateRulesPreviewResponse, evaluate_rules_preview_request, + rule_evaluation_service_client::RuleEvaluationServiceClient, + }, + rules::v1::UpdateRuleRequest, +}; + +#[cfg(test)] +mod test; + +/// Which rule to dry-run: a saved rule identified by id (already resolved from +/// a name upstream, if that's how the caller identified it), or an ad-hoc +/// draft definition that is never persisted anywhere. +pub enum PreviewRuleSource { + SavedRuleId(String), + DraftRuleConfig(Box), +} + +#[derive(Clone)] +pub struct RuleEvaluationService { + channel: SiftChannel, + policy: RetryPolicy, +} + +impl RuleEvaluationService { + pub fn new(channel: SiftChannel, policy: RetryPolicy) -> Self { + Self { channel, policy } + } + + /// Dry-runs rule evaluation against a run via `EvaluateRulesPreview`. Nothing + /// is persisted by this RPC: no report, annotation, or rule version is + /// created. Returns the count of annotations that would be created and their + /// would-be details. + pub async fn preview_rule( + &self, + run_id: String, + source: PreviewRuleSource, + organization_id: Option, + ) -> Result { + let mode = match source { + PreviewRuleSource::SavedRuleId(rule_id) => { + evaluate_rules_preview_request::Mode::Rules(EvaluateRulesFromCurrentRuleVersions { + rules: Some(ResourceIdentifiers { + identifiers: Some(resource_identifiers::Identifiers::Ids(Ids { + ids: vec![rule_id], + })), + }), + }) + } + PreviewRuleSource::DraftRuleConfig(config) => { + evaluate_rules_preview_request::Mode::RuleConfigs(EvaluateRulesFromRuleConfigs { + configs: vec![*config], + }) + } + }; + + let request = EvaluateRulesPreviewRequest { + organization_id: organization_id.unwrap_or_default(), + time: Some(evaluate_rules_preview_request::Time::Run( + ResourceIdentifier { + identifier: Some(resource_identifier::Identifier::Id(run_id)), + }, + )), + mode: Some(mode), + ..Default::default() + }; + + let channel = self.channel.clone(); + with_retry(&self.policy, move || { + let channel = channel.clone(); + let request = request.clone(); + async move { + let mut client = RuleEvaluationServiceClient::new(channel); + client + .evaluate_rules_preview(request) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to preview rule evaluation") + } +} diff --git a/rust/crates/sift_mcp/src/service/rule_evaluation/test.rs b/rust/crates/sift_mcp/src/service/rule_evaluation/test.rs new file mode 100644 index 000000000..35b9142bd --- /dev/null +++ b/rust/crates/sift_mcp/src/service/rule_evaluation/test.rs @@ -0,0 +1,146 @@ +use sift_rs::{ + common::r#type::v1::{resource_identifier, resource_identifiers}, + rule_evaluation::v1::{ + EvaluateRulesPreviewResponse, evaluate_rules_preview_request, + rule_evaluation_service_server::RuleEvaluationServiceServer, + }, + rules::v1::{DryRunAnnotation, UpdateRuleRequest}, +}; +use sift_test_util::{ + grpc::memory_sift_channel, mock::rule_evaluation::v1::MockRuleEvaluationServiceImpl, +}; +use tokio::task::JoinHandle; +use tonic::{Response, Status, transport::Server}; + +use super::{PreviewRuleSource, RuleEvaluationService}; +use crate::policy::RetryPolicy; + +async fn service_with_mock( + mock: MockRuleEvaluationServiceImpl, +) -> (RuleEvaluationService, JoinHandle<()>) { + let (client, server) = tokio::io::duplex(1024); + let channel = memory_sift_channel(client).await; + + let handle = tokio::spawn(async move { + Server::builder() + .add_service(RuleEvaluationServiceServer::new(mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) + .await + .unwrap(); + }); + + ( + RuleEvaluationService::new(channel, RetryPolicy::default()), + handle, + ) +} + +#[tokio::test] +async fn preview_rule_builds_saved_rule_request() { + let mut mock = MockRuleEvaluationServiceImpl::new(); + mock.expect_evaluate_rules_preview() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let run_matches = matches!( + &req.time, + Some(evaluate_rules_preview_request::Time::Run(id)) + if matches!(&id.identifier, Some(resource_identifier::Identifier::Id(v)) if v == "run-1") + ); + let mode_matches = matches!( + &req.mode, + Some(evaluate_rules_preview_request::Mode::Rules(r)) + if matches!( + r.rules.as_ref().and_then(|r| r.identifiers.as_ref()), + Some(resource_identifiers::Identifiers::Ids(ids)) if ids.ids == vec!["rule-1".to_string()] + ) + ); + run_matches && mode_matches + }) + .returning(|_| { + Ok(Response::new(EvaluateRulesPreviewResponse { + created_annotation_count: 2, + dry_run_annotations: vec![], + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let resp = service + .preview_rule( + "run-1".to_string(), + PreviewRuleSource::SavedRuleId("rule-1".to_string()), + None, + ) + .await + .expect("preview_rule failed"); + + assert_eq!(resp.created_annotation_count, 2); +} + +#[tokio::test] +async fn preview_rule_builds_draft_rule_config_request() { + let mut mock = MockRuleEvaluationServiceImpl::new(); + mock.expect_evaluate_rules_preview() + .times(1) + .withf(|req| { + let req = req.get_ref(); + matches!( + &req.mode, + Some(evaluate_rules_preview_request::Mode::RuleConfigs(cfg)) + if cfg.configs.len() == 1 && cfg.configs[0].name == "draft rule" + ) + }) + .returning(|_| { + Ok(Response::new(EvaluateRulesPreviewResponse { + created_annotation_count: 1, + dry_run_annotations: vec![DryRunAnnotation { + condition_id: "cond-1".into(), + name: "draft rule".into(), + ..Default::default() + }], + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let draft = UpdateRuleRequest { + name: "draft rule".into(), + ..Default::default() + }; + + let resp = service + .preview_rule( + "run-1".to_string(), + PreviewRuleSource::DraftRuleConfig(Box::new(draft)), + None, + ) + .await + .expect("preview_rule failed"); + + assert_eq!(resp.created_annotation_count, 1); + assert_eq!(resp.dry_run_annotations.len(), 1); +} + +#[tokio::test] +async fn preview_rule_propagates_grpc_error() { + let mut mock = MockRuleEvaluationServiceImpl::new(); + mock.expect_evaluate_rules_preview() + .returning(|_| Err(Status::invalid_argument("bad rule config"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .preview_rule( + "run-1".to_string(), + PreviewRuleSource::SavedRuleId("rule-1".to_string()), + None, + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to preview rule evaluation") + ); +} diff --git a/rust/crates/sift_mcp/src/tool/mod.rs b/rust/crates/sift_mcp/src/tool/mod.rs index 331d92b16..93d7bc26d 100644 --- a/rust/crates/sift_mcp/src/tool/mod.rs +++ b/rust/crates/sift_mcp/src/tool/mod.rs @@ -9,6 +9,7 @@ pub mod explore; pub mod ping; pub mod report_templates; pub mod reports; +pub mod rule_evaluation; pub mod rules; pub mod runs; #[cfg(feature = "test-reports")] diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs new file mode 100644 index 000000000..d1241adee --- /dev/null +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs @@ -0,0 +1,197 @@ +use rmcp::{ + ErrorData, + handler::server::wrapper::Parameters, + model::{CallToolResult, ContentBlock}, + schemars::{self, JsonSchema}, + tool, tool_router, +}; +use serde::Deserialize; +use sift_rs::rules::v1::UpdateRuleRequest; + +use crate::{ + error::{self, from_anyhow}, + server::SiftMcpServer, + service::rule_evaluation::PreviewRuleSource, +}; + +#[cfg(test)] +mod test; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct PreviewRuleParams { + pub(crate) run_id: String, + pub(crate) rule_id: Option, + pub(crate) rule_name: Option, + pub(crate) draft_rule_config: Option, + pub(crate) organization_id: Option, +} + +/// Which saved-rule selector the caller gave, prior to resolving a name to an id. +enum SavedRuleSelector { + Id(String), + Name(String), +} + +#[tool_router(router = rule_evaluation_router, vis = "pub(crate)")] +impl SiftMcpServer { + #[tool( + name = "preview_rule", + description = " + Dry-run a rule against a run and return the annotations that would be generated. This is a READ-ONLY + action: nothing is persisted (no report, annotation, or rule version is created). To persist an + evaluation, use `create_report` instead once the preview looks right. + + Output: + - `{ \"created_annotation_count\": number, \"dry_run_annotations\": [DryRunAnnotation, ...], + \"next_step\": string }`. Each `DryRunAnnotation` has `condition_id`, `name`, `start_time`, + `end_time`, and `condition_version_id` for one annotation the rule would create. + + Parameters: + - `run_id`: required; the run to evaluate the rule against. + - The rule under test is one of two mutually exclusive shapes — provide exactly one: + - SAVED rule: set `rule_id` or `rule_name` (not both). `rule_name` is resolved to a rule id via an + exact `name ==` lookup; if more than one rule shares the name, an arbitrary match is used, so + prefer `rule_id` when it's known. + - DRAFT rule: set `draft_rule_config`, a JSON string matching + `protos/sift/rules/v1/rules.proto::UpdateRuleRequest` — the same shape `create_rule` takes as + `rule_json`. Nothing is looked up or saved for this shape; the whole definition travels in the + request. Mirror an existing rule retrieved via `list_rules` rather than authoring blind. + - `organization_id`: optional. Required only when the caller belongs to multiple organizations. + + Errors: + - `INVALID_PARAMS` if `run_id` is empty; if neither a saved-rule selector nor `draft_rule_config` is + given, or both are; if both `rule_id` and `rule_name` are given; or if `draft_rule_config` is not + valid JSON matching the rule schema. + - `RESOURCE_NOT_FOUND` if `rule_name` matches no rule, or `run_id` does not exist. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - This is read-only — no user confirmation is needed before calling it, unlike `create_report`. + - After a good preview, offer the user the next step: `create_report` to persist the evaluation as a + report, or `create_rule`/`update_rule` to save a draft as a real rule. + ", + annotations(title = "rule_evaluation/preview_rule", read_only_hint = true) + )] + pub async fn preview_rule(&self, params: Parameters) -> error::McpResult { + let Parameters(PreviewRuleParams { + run_id, + rule_id, + rule_name, + draft_rule_config, + organization_id, + }) = params; + + if run_id.is_empty() { + return Err(ErrorData::invalid_params( + "`run_id` must not be empty", + None, + )); + } + + let selector = saved_rule_selector(rule_id, rule_name)?; + + let source = match (selector, draft_rule_config) { + (Some(_), Some(_)) => { + return Err(ErrorData::invalid_params( + "provide either a saved rule (`rule_id`/`rule_name`) or `draft_rule_config`, not both", + None, + )); + } + (None, None) => { + return Err(ErrorData::invalid_params( + "provide either a saved rule (`rule_id`/`rule_name`) or `draft_rule_config`", + None, + )); + } + (Some(selector), None) => { + let rule_id = self.resolve_saved_rule_id(selector).await?; + PreviewRuleSource::SavedRuleId(rule_id) + } + (None, Some(draft_rule_config)) => { + let config = parse_draft_rule_config(&draft_rule_config)?; + PreviewRuleSource::DraftRuleConfig(Box::new(config)) + } + }; + + let result = self + .rule_evaluation_service + .preview_rule(run_id, source, organization_id) + .await + .map_err(from_anyhow)?; + + let next_step = format!( + "Preview evaluated {} annotation(s) that would be created. Nothing was persisted (no report, \ + annotation, or rule was saved). Review the dry-run results with the user; if they want to keep \ + them, use `create_report` to persist an evaluation over this run.", + result.created_annotation_count, + ); + + let mut result = CallToolResult::structured(serde_json::json!({ + "created_annotation_count": result.created_annotation_count, + "dry_run_annotations": result.dry_run_annotations, + "next_step": next_step, + })); + result.content = vec![ContentBlock::text(next_step)]; + Ok(result) + } +} + +/// Resolve the mutually exclusive `(rule_id, rule_name)` params into an +/// optional selector. `None` means the caller gave neither (the "draft rule" +/// case is expected instead); both set is an error. +fn saved_rule_selector( + rule_id: Option, + rule_name: Option, +) -> Result, ErrorData> { + match (rule_id, rule_name) { + (Some(id), None) => Ok(Some(SavedRuleSelector::Id(id))), + (None, Some(name)) => Ok(Some(SavedRuleSelector::Name(name))), + (None, None) => Ok(None), + (Some(_), Some(_)) => Err(ErrorData::invalid_params( + "provide at most one of `rule_id` or `rule_name`, not both", + None, + )), + } +} + +/// Deserialize a draft rule config JSON string into an `UpdateRuleRequest`, +/// mapping any parse error to `INVALID_PARAMS` so the agent can correct it. +fn parse_draft_rule_config(draft_rule_config: &str) -> Result { + serde_json::from_str::(draft_rule_config).map_err(|e| { + ErrorData::invalid_params( + format!("`draft_rule_config` is not a valid rule definition: {e}"), + None, + ) + }) +} + +impl SiftMcpServer { + /// Resolve a saved-rule selector to a `rule_id`, going through the existing + /// rules service for name lookups so this doesn't duplicate rule lookup logic. + async fn resolve_saved_rule_id( + &self, + selector: SavedRuleSelector, + ) -> Result { + match selector { + SavedRuleSelector::Id(id) => Ok(id), + SavedRuleSelector::Name(name) => { + let filter = format!("name == \"{}\"", cel_escape(&name)); + let rule = self + .rule_service + .list_rules(filter, None, Some(1)) + .await + .map_err(from_anyhow)? + .into_iter() + .next() + .ok_or_else(|| { + ErrorData::resource_not_found(format!("rule '{name}' not found"), None) + })?; + Ok(rule.rule_id) + } + } + } +} + +fn cel_escape(s: &str) -> String { + s.replace('\\', "\\\\").replace('"', "\\\"") +} diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs new file mode 100644 index 000000000..6b0067bf2 --- /dev/null +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs @@ -0,0 +1,290 @@ +use rmcp::{handler::server::wrapper::Parameters, model::ErrorCode}; +use sift_rs::{ + rule_evaluation::v1::{ + EvaluateRulesPreviewResponse, rule_evaluation_service_server::RuleEvaluationServiceServer, + }, + rules::v1::{ + DryRunAnnotation, ListRulesResponse, Rule, rule_service_server::RuleServiceServer, + }, +}; +use sift_test_util::{ + grpc::memory_sift_channel, + mock::{rule_evaluation::v1::MockRuleEvaluationServiceImpl, rules::v1::MockRuleServiceImpl}, +}; +use tokio::task::JoinHandle; +use tonic::{Response, Status, transport::Server}; + +use super::PreviewRuleParams; +use crate::{server::SiftMcpServer, tool::common::test_support::structured_field}; + +fn preview_rule_params() -> PreviewRuleParams { + PreviewRuleParams { + run_id: "run-1".into(), + rule_id: None, + rule_name: None, + draft_rule_config: None, + organization_id: None, + } +} + +/// Registers only the evaluation mock. Sufficient for tests that never need to +/// resolve a rule name (rule_id-only or draft-config paths). +async fn server_with_eval_mock( + mock: MockRuleEvaluationServiceImpl, +) -> (SiftMcpServer, JoinHandle<()>) { + let (client, server) = tokio::io::duplex(1024); + let channel = memory_sift_channel(client).await; + + let handle = tokio::spawn(async move { + Server::builder() + .add_service(RuleEvaluationServiceServer::new(mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) + .await + .unwrap(); + }); + + ( + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), + handle, + ) +} + +/// Registers both the rule and evaluation mocks, for the rule-name-resolution path. +async fn server_with_dual_mocks( + rule_mock: MockRuleServiceImpl, + eval_mock: MockRuleEvaluationServiceImpl, +) -> (SiftMcpServer, JoinHandle<()>) { + let (client, server) = tokio::io::duplex(1024); + let channel = memory_sift_channel(client).await; + + let handle = tokio::spawn(async move { + Server::builder() + .add_service(RuleServiceServer::new(rule_mock)) + .add_service(RuleEvaluationServiceServer::new(eval_mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) + .await + .unwrap(); + }); + + ( + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), + handle, + ) +} + +#[tokio::test] +async fn preview_rule_saved_by_id_happy_path() { + let mut eval_mock = MockRuleEvaluationServiceImpl::new(); + eval_mock.expect_evaluate_rules_preview().returning(|_| { + Ok(Response::new(EvaluateRulesPreviewResponse { + created_annotation_count: 2, + dry_run_annotations: vec![DryRunAnnotation { + condition_id: "cond-1".into(), + name: "overtemp".into(), + ..Default::default() + }], + })) + }); + + let (server, _h) = server_with_eval_mock(eval_mock).await; + + let mut params = preview_rule_params(); + params.rule_id = Some("rule-1".into()); + + let resp = server + .preview_rule(Parameters(params)) + .await + .expect("preview_rule failed"); + + let count = structured_field(resp.clone(), "created_annotation_count"); + assert_eq!(count, 2); + let annotations = structured_field(resp, "dry_run_annotations"); + assert_eq!(annotations.as_array().unwrap().len(), 1); +} + +#[tokio::test] +async fn preview_rule_saved_by_name_resolves_rule_id() { + let mut rule_mock = MockRuleServiceImpl::new(); + rule_mock + .expect_list_rules() + .withf(|req| req.get_ref().filter == "name == \"overtemp\"") + .returning(|_| { + Ok(Response::new(ListRulesResponse { + rules: vec![Rule { + rule_id: "rule-resolved".into(), + name: "overtemp".into(), + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let mut eval_mock = MockRuleEvaluationServiceImpl::new(); + eval_mock.expect_evaluate_rules_preview().returning(|_| { + Ok(Response::new(EvaluateRulesPreviewResponse { + created_annotation_count: 0, + dry_run_annotations: vec![], + })) + }); + + let (server, _h) = server_with_dual_mocks(rule_mock, eval_mock).await; + + let mut params = preview_rule_params(); + params.rule_name = Some("overtemp".into()); + + let resp = server + .preview_rule(Parameters(params)) + .await + .expect("preview_rule failed"); + + let count = structured_field(resp, "created_annotation_count"); + assert_eq!(count, 0); +} + +#[tokio::test] +async fn preview_rule_saved_by_name_not_found() { + let mut rule_mock = MockRuleServiceImpl::new(); + rule_mock.expect_list_rules().returning(|_| { + Ok(Response::new(ListRulesResponse { + rules: vec![], + next_page_token: String::new(), + })) + }); + + let (server, _h) = + server_with_dual_mocks(rule_mock, MockRuleEvaluationServiceImpl::new()).await; + + let mut params = preview_rule_params(); + params.rule_name = Some("does-not-exist".into()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::RESOURCE_NOT_FOUND); +} + +#[tokio::test] +async fn preview_rule_draft_config_happy_path() { + let mut eval_mock = MockRuleEvaluationServiceImpl::new(); + eval_mock.expect_evaluate_rules_preview().returning(|_| { + Ok(Response::new(EvaluateRulesPreviewResponse { + created_annotation_count: 1, + dry_run_annotations: vec![DryRunAnnotation { + condition_id: "cond-draft".into(), + name: "draft rule".into(), + ..Default::default() + }], + })) + }); + + let (server, _h) = server_with_eval_mock(eval_mock).await; + + let mut params = preview_rule_params(); + params.draft_rule_config = + Some(r#"{ "name": "draft rule", "description": "ad-hoc" }"#.to_string()); + + let resp = server + .preview_rule(Parameters(params)) + .await + .expect("preview_rule failed"); + + let count = structured_field(resp, "created_annotation_count"); + assert_eq!(count, 1); +} + +#[tokio::test] +async fn preview_rule_rejects_malformed_draft_json() { + let (server, _h) = server_with_eval_mock(MockRuleEvaluationServiceImpl::new()).await; + + let mut params = preview_rule_params(); + params.draft_rule_config = Some("not json".to_string()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn preview_rule_rejects_both_saved_and_draft() { + let (server, _h) = server_with_eval_mock(MockRuleEvaluationServiceImpl::new()).await; + + let mut params = preview_rule_params(); + params.rule_id = Some("rule-1".into()); + params.draft_rule_config = Some(r#"{ "name": "x", "description": "y" }"#.to_string()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn preview_rule_rejects_neither_saved_nor_draft() { + let (server, _h) = server_with_eval_mock(MockRuleEvaluationServiceImpl::new()).await; + + let err = server + .preview_rule(Parameters(preview_rule_params())) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn preview_rule_rejects_both_rule_id_and_rule_name() { + let (server, _h) = server_with_eval_mock(MockRuleEvaluationServiceImpl::new()).await; + + let mut params = preview_rule_params(); + params.rule_id = Some("rule-1".into()); + params.rule_name = Some("overtemp".into()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn preview_rule_rejects_empty_run_id() { + let (server, _h) = server_with_eval_mock(MockRuleEvaluationServiceImpl::new()).await; + + let mut params = preview_rule_params(); + params.run_id = String::new(); + params.rule_id = Some("rule-1".into()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn preview_rule_propagates_grpc_error() { + let mut eval_mock = MockRuleEvaluationServiceImpl::new(); + eval_mock + .expect_evaluate_rules_preview() + .returning(|_| Err(Status::not_found("run missing"))); + + let (server, _h) = server_with_eval_mock(eval_mock).await; + + let mut params = preview_rule_params(); + params.rule_id = Some("rule-1".into()); + + let err = server + .preview_rule(Parameters(params)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::RESOURCE_NOT_FOUND); +} diff --git a/rust/crates/sift_mcp/src/tool_events.json b/rust/crates/sift_mcp/src/tool_events.json index c6b3fb8ca..0706bf055 100644 --- a/rust/crates/sift_mcp/src/tool_events.json +++ b/rust/crates/sift_mcp/src/tool_events.json @@ -29,6 +29,7 @@ "list_test_steps": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_TEST_STEPS", "list_users": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_USERS", "ping": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_PING", + "preview_rule": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_PREVIEW_RULE", "search_docs": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_SEARCH_DOCS", "sql": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_SQL", "unarchive_calculated_channel": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UNARCHIVE_CALCULATED_CHANNEL", From c3e4c6bb68f6eb8dab83c4d2c2f5f850c04a2307 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 19 Aug 2026 00:21:56 -0700 Subject: [PATCH 2/5] refactor: use the shared cel_escape in rule evaluation --- rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs index d1241adee..bc1dfb115 100644 --- a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs @@ -11,7 +11,7 @@ use sift_rs::rules::v1::UpdateRuleRequest; use crate::{ error::{self, from_anyhow}, server::SiftMcpServer, - service::rule_evaluation::PreviewRuleSource, + service::{common::cel_escape, rule_evaluation::PreviewRuleSource}, }; #[cfg(test)] @@ -191,7 +191,3 @@ impl SiftMcpServer { } } } - -fn cel_escape(s: &str) -> String { - s.replace('\\', "\\\\").replace('"', "\\\"") -} From 5adf404fd25f534f574a319a23bcb489e5054ffa Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 00:46:25 -0700 Subject: [PATCH 3/5] fix: adapt preview_rule name resolution to paged list results --- rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs index bc1dfb115..a675038b1 100644 --- a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs @@ -181,6 +181,7 @@ impl SiftMcpServer { .list_rules(filter, None, Some(1)) .await .map_err(from_anyhow)? + .items .into_iter() .next() .ok_or_else(|| { From f09118b5c867faa918f64c7f9edafd004ca52171 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 01:12:04 -0700 Subject: [PATCH 4/5] fix: exclude archived rules from preview_rule name resolution --- rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs | 2 +- rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs index a675038b1..6eef4bc4b 100644 --- a/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/mod.rs @@ -175,7 +175,7 @@ impl SiftMcpServer { match selector { SavedRuleSelector::Id(id) => Ok(id), SavedRuleSelector::Name(name) => { - let filter = format!("name == \"{}\"", cel_escape(&name)); + let filter = format!("is_archived == false && name == \"{}\"", cel_escape(&name)); let rule = self .rule_service .list_rules(filter, None, Some(1)) diff --git a/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs b/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs index 6b0067bf2..ac1034f9c 100644 --- a/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs +++ b/rust/crates/sift_mcp/src/tool/rule_evaluation/test.rs @@ -107,7 +107,7 @@ async fn preview_rule_saved_by_name_resolves_rule_id() { let mut rule_mock = MockRuleServiceImpl::new(); rule_mock .expect_list_rules() - .withf(|req| req.get_ref().filter == "name == \"overtemp\"") + .withf(|req| req.get_ref().filter == "is_archived == false && name == \"overtemp\"") .returning(|_| { Ok(Response::new(ListRulesResponse { rules: vec![Rule { From bfdb514eaf8215b19fa04243c47b79d5ab959301 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang <159062208+evan-sift@users.noreply.github.com> Date: Thu, 27 Aug 2026 12:39:38 -0700 Subject: [PATCH 5/5] rust(feat): user defined function MCP tools (#739) Co-authored-by: Liam Neville --- rust/crates/sift_cli/CHANGELOG.md | 26 + rust/crates/sift_cli/Cargo.toml | 2 +- .../sift_cli/assets/skills/sift/SKILL.md | 93 +- rust/crates/sift_mcp/Cargo.toml | 1 + rust/crates/sift_mcp/src/server/mod.rs | 7 +- .../sift_mcp/src/service/annotations/mod.rs | 267 +++++- .../sift_mcp/src/service/annotations/test.rs | 185 +++- rust/crates/sift_mcp/src/service/mod.rs | 1 + .../src/service/user_defined_functions/mod.rs | 377 ++++++++ .../service/user_defined_functions/test.rs | 860 ++++++++++++++++++ .../sift_mcp/src/tool/annotations/mod.rs | 179 +++- .../sift_mcp/src/tool/annotations/test.rs | 480 +++++++++- rust/crates/sift_mcp/src/tool/mod.rs | 1 + .../src/tool/user_defined_functions/mod.rs | 719 +++++++++++++++ .../src/tool/user_defined_functions/test.rs | 799 ++++++++++++++++ rust/crates/sift_mcp/src/tool_events.json | 6 + rust/crates/sift_test_util/src/mock/mod.rs | 1 + .../src/mock/user_defined_functions/mod.rs | 1 + .../src/mock/user_defined_functions/v1.rs | 94 ++ 19 files changed, 4024 insertions(+), 75 deletions(-) create mode 100644 rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs create mode 100644 rust/crates/sift_mcp/src/service/user_defined_functions/test.rs create mode 100644 rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs create mode 100644 rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs create mode 100644 rust/crates/sift_test_util/src/mock/user_defined_functions/mod.rs create mode 100644 rust/crates/sift_test_util/src/mock/user_defined_functions/v1.rs diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index d8d0acbce..090d08c0f 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,32 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +## [v0.5.0] - August 26, 2026 + +### What's New + +- Added MCP tools for managing calculated channels: `list_calculated_channels`, + `list_calculated_channel_versions`, `create_calculated_channel`, + `update_calculated_channel`, `archive_calculated_channel`, and + `unarchive_calculated_channel`. +- `get_data` now serves saved calculated channels. A name in `channel_names` + with no raw-channel match resolves as an active saved calculated channel for + the asset and run; unresolvable names are reported explicitly. +- `get_data` now accepts `asset_id` as an alternative to `asset_name`; exactly + one must be set. +- Added `preview_rule`, which dry-runs a saved rule or an ad-hoc draft rule + config against a run without persisting anything. +- Added MCP tools for managing user-defined functions: + `list_user_defined_functions`, `list_user_defined_function_versions`, + `create_user_defined_function`, `update_user_defined_function`, + `archive_user_defined_function`, and `unarchive_user_defined_function`. +- `update_annotation` now requires `annotation_ids` instead of `annotation_id`, + a breaking change for existing callers; pass a one-element list for one + annotation. It updates 1 to 1000 annotations per call with per-ID failure + reporting, and its new `is_archived` parameter archives or unarchives + annotations. +- Refreshed the bundled Sift agent skill to cover the expanded MCP tool surface. + ## [v0.4.4] - August 24, 2026 ### What's New diff --git a/rust/crates/sift_cli/Cargo.toml b/rust/crates/sift_cli/Cargo.toml index 933525bd9..f1b096a04 100644 --- a/rust/crates/sift_cli/Cargo.toml +++ b/rust/crates/sift_cli/Cargo.toml @@ -3,7 +3,7 @@ test-reports = ["sift_mcp/test-reports"] [package] name = "sift_cli" -version = "0.4.4" +version = "0.5.0" authors.workspace = true edition.workspace = true categories.workspace = true diff --git a/rust/crates/sift_cli/assets/skills/sift/SKILL.md b/rust/crates/sift_cli/assets/skills/sift/SKILL.md index de9eab083..47f56b3cb 100644 --- a/rust/crates/sift_cli/assets/skills/sift/SKILL.md +++ b/rust/crates/sift_cli/assets/skills/sift/SKILL.md @@ -1,19 +1,21 @@ --- name: sift description: >- - Use when working with Sift: ingesting or importing time-series data, - querying assets/runs/channels/users, exporting data, decimating or running - SQL over data, opening a view in the Sift Explore web app, writing code that - integrates with Sift, installing, updating, or diagnosing the Sift agent - integration, or looking up how Sift works in its product and API - documentation. Covers the Sift MCP server (started by `sift-cli mcp`), the - `sift-cli` itself, the Sift REST API over cURL, the Sift Python library - (`sift_client`), and the Sift Rust streaming library (`sift_stream`). + Use for Sift tasks: ingesting or importing time-series data, querying + assets/runs/channels/users, managing calculated channels, rules, and + user-defined functions, exporting or decimating data, running SQL over data, + opening a view in Sift Explore, writing code that integrates with Sift, + installing, updating, or diagnosing the Sift agent integration, or looking + up how Sift works in its product and API documentation. Covers the Sift MCP + server (started by `sift-cli mcp`), `sift-cli`, the Sift REST API over cURL, + the Sift Python library (`sift_client`), and the Sift Rust streaming library + (`sift_stream`). Triggers include phrases like "import this file into Sift", "stream data to Sift", "list assets/runs/channels", "runs I created", "runs a teammate - created", "export a run", "query Sift", "graph", "plot", "visualize", "open - in Explore", "write code to integrate with Sift", "how does X work in Sift", - "what does this endpoint do", or "look up the Sift API reference". + created", "export a run", "query Sift", "graph", "plot", "visualize", "open in + Explore", "write code to integrate with Sift", "how does X work in Sift", + "what does this endpoint do", "list calculated channels", "preview a rule", + or "look up the Sift API reference". ---