From fcfe4aefe11bc409fd0f1c0301f6678cf4e430d6 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Tue, 18 Aug 2026 22:09:57 -0700 Subject: [PATCH 1/8] feat: add user defined function service with mock-backed tests --- rust/crates/sift_mcp/src/service/mod.rs | 1 + .../src/service/user_defined_functions/mod.rs | 319 +++++++ .../service/user_defined_functions/test.rs | 807 ++++++++++++++++++ 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 ++ 6 files changed, 1223 insertions(+) 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_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_mcp/src/service/mod.rs b/rust/crates/sift_mcp/src/service/mod.rs index 6eaccfc4d5..ecad306081 100644 --- a/rust/crates/sift_mcp/src/service/mod.rs +++ b/rust/crates/sift_mcp/src/service/mod.rs @@ -14,6 +14,7 @@ pub mod runs; #[cfg(feature = "test-reports")] pub mod test_reports; pub mod url; +pub mod user_defined_functions; pub mod users; pub(crate) mod common; diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs new file mode 100644 index 0000000000..db59a3c270 --- /dev/null +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs @@ -0,0 +1,319 @@ +use crate::policy::{RetryPolicy, with_retry}; +use crate::service::common; +use anyhow::{Context, Result, anyhow}; +use pbjson_types::FieldMask; +use sift_rs::{ + SiftChannel, + common::r#type::v1::{FunctionInput, UserDefinedFunction}, + metadata::v1::MetadataValue, + user_defined_functions::v1::{ + CreateUserDefinedFunctionRequest, ListUserDefinedFunctionVersionsRequest, + ListUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionsRequest, + ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionRequest, + user_defined_function_service_client::UserDefinedFunctionServiceClient, + }, +}; + +#[cfg(test)] +mod test; + +/// A partial set of changes to apply to an existing user defined function. +/// `None` means "leave unchanged" — only the fields set here reach the update +/// mask. Archive state has its own entry point +/// ([`UserDefinedFunctionService::set_user_defined_function_archived`]) so the +/// archive flip stays a separate, gated operation. +#[derive(Debug, Default)] +pub struct UdfUpdate { + pub name: Option, + pub description: Option, + pub expression: Option, + pub function_inputs: Option>, + pub metadata: Option>, +} + +#[derive(Clone)] +pub struct UserDefinedFunctionService { + channel: SiftChannel, + policy: RetryPolicy, +} + +impl UserDefinedFunctionService { + pub fn new(channel: SiftChannel, policy: RetryPolicy) -> Self { + Self { channel, policy } + } + + /// Lists the latest version of each user defined function. + pub async fn list_user_defined_functions( + &self, + filter: String, + order_by: Option, + limit: Option, + ) -> Result> { + let (page_size, record_limit) = common::paging(limit); + + let mut page_token = String::new(); + let mut results = Vec::new(); + + let order_by = order_by.unwrap_or_default(); + + loop { + let channel = self.channel.clone(); + let filter = filter.clone(); + let order_by = order_by.clone(); + let token = page_token.clone(); + + let resp = with_retry(&self.policy, move || { + let channel = channel.clone(); + let filter = filter.clone(); + let order_by = order_by.clone(); + let token = token.clone(); + async move { + let mut client = UserDefinedFunctionServiceClient::new(channel); + client + .list_user_defined_functions(ListUserDefinedFunctionsRequest { + page_size, + page_token: token, + filter, + order_by, + }) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to query user defined functions")?; + + let ListUserDefinedFunctionsResponse { + user_defined_functions, + next_page_token, + } = resp; + if user_defined_functions.is_empty() { + break; + } + results.extend(user_defined_functions); + + if results.len() >= record_limit || next_page_token.is_empty() { + break; + } + page_token = next_page_token; + } + + results.truncate(record_limit); + + Ok(results) + } + + /// Lists the version history of one user defined function. The caller + /// guarantees exactly one of `user_defined_function_id` or `name` is + /// non-empty; the proto ignores `name` when the id is present. + pub async fn list_user_defined_function_versions( + &self, + user_defined_function_id: String, + name: String, + filter: String, + order_by: Option, + limit: Option, + ) -> Result> { + let (page_size, record_limit) = common::paging(limit); + + let mut page_token = String::new(); + let mut results = Vec::new(); + + let order_by = order_by.unwrap_or_default(); + + loop { + let channel = self.channel.clone(); + let user_defined_function_id = user_defined_function_id.clone(); + let name = name.clone(); + let filter = filter.clone(); + let order_by = order_by.clone(); + let token = page_token.clone(); + + let resp = with_retry(&self.policy, move || { + let channel = channel.clone(); + let user_defined_function_id = user_defined_function_id.clone(); + let name = name.clone(); + let filter = filter.clone(); + let order_by = order_by.clone(); + let token = token.clone(); + async move { + let mut client = UserDefinedFunctionServiceClient::new(channel); + client + .list_user_defined_function_versions( + ListUserDefinedFunctionVersionsRequest { + user_defined_function_id, + name, + page_size, + page_token: token, + filter, + order_by, + }, + ) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to query user defined function versions")?; + + let ListUserDefinedFunctionVersionsResponse { + user_defined_functions, + next_page_token, + } = resp; + if user_defined_functions.is_empty() { + break; + } + results.extend(user_defined_functions); + + if results.len() >= record_limit || next_page_token.is_empty() { + break; + } + page_token = next_page_token; + } + + results.truncate(record_limit); + + Ok(results) + } + + /// Creates a user defined function at version 1 and returns it. + pub async fn create_user_defined_function( + &self, + name: String, + description: Option, + expression: String, + function_inputs: Vec, + user_notes: Option, + metadata: Vec, + ) -> Result { + let channel = self.channel.clone(); + + let resp = with_retry(&self.policy, move || { + let channel = channel.clone(); + let name = name.clone(); + let description = description.clone(); + let expression = expression.clone(); + let function_inputs = function_inputs.clone(); + let user_notes = user_notes.clone(); + let metadata = metadata.clone(); + async move { + let mut client = UserDefinedFunctionServiceClient::new(channel); + client + .create_user_defined_function(CreateUserDefinedFunctionRequest { + name, + description, + expression, + function_inputs, + user_notes, + metadata, + }) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to create user defined function")?; + + resp.user_defined_function.ok_or_else(|| { + anyhow!("create_user_defined_function response missing user defined function") + }) + } + + /// Updates a user defined function through the field mask. Per + /// `protos/sift/user_defined_functions/v1/user_defined_functions.proto::UpdateUserDefinedFunctionRequest` + /// the updatable fields are `name`, `archived_date`, `is_archived`, + /// `description`, `expression`, `function_inputs`, and `metadata`; archive + /// state goes through [`Self::set_user_defined_function_archived`]. + /// + /// The RPC has no version precondition. Every accepted update creates a new + /// version and returns it, so the request carries only the id and the masked + /// fields — never a version read earlier by the caller. + pub async fn update_user_defined_function( + &self, + user_defined_function_id: String, + changes: UdfUpdate, + ) -> Result { + let mut function = UserDefinedFunction { + user_defined_function_id, + ..Default::default() + }; + let mut paths = Vec::new(); + + let UdfUpdate { + name, + description, + expression, + function_inputs, + metadata, + } = changes; + + if let Some(v) = name { + function.name = v; + paths.push("name".to_string()); + } + if let Some(v) = description { + function.description = v; + paths.push("description".to_string()); + } + if let Some(v) = expression { + function.expression = v; + paths.push("expression".to_string()); + } + if let Some(v) = function_inputs { + function.function_inputs = v; + paths.push("function_inputs".to_string()); + } + if let Some(v) = metadata { + function.metadata = v; + paths.push("metadata".to_string()); + } + + self.send_update(function, paths).await + } + + /// Archives or unarchives a user defined function. There is no dedicated + /// archive RPC: the proto sets `is_archived` through the update mask. + pub async fn set_user_defined_function_archived( + &self, + user_defined_function_id: String, + is_archived: bool, + ) -> Result { + let function = UserDefinedFunction { + user_defined_function_id, + is_archived, + ..Default::default() + }; + + self.send_update(function, vec!["is_archived".to_string()]) + .await + } + + async fn send_update( + &self, + function: UserDefinedFunction, + paths: Vec, + ) -> Result { + let channel = self.channel.clone(); + let resp = with_retry(&self.policy, move || { + let channel = channel.clone(); + let function = function.clone(); + let paths = paths.clone(); + async move { + let mut client = UserDefinedFunctionServiceClient::new(channel); + client + .update_user_defined_function(UpdateUserDefinedFunctionRequest { + user_defined_function: Some(function), + update_mask: Some(FieldMask { paths }), + }) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to update user defined function")?; + + resp.user_defined_function.ok_or_else(|| { + anyhow!("update_user_defined_function response missing user defined function") + }) + } +} diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs new file mode 100644 index 0000000000..30a9f0d394 --- /dev/null +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs @@ -0,0 +1,807 @@ +use sift_rs::{ + common::r#type::v1::{FunctionDataType, FunctionInput, UserDefinedFunction}, + metadata::v1::{ + MetadataKey, MetadataKeyType, MetadataValue, metadata_value::Value as MetadataValueInner, + }, + user_defined_functions::v1::{ + CreateUserDefinedFunctionResponse, ListUserDefinedFunctionVersionsResponse, + ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionResponse, + user_defined_function_service_server::UserDefinedFunctionServiceServer, + }, +}; +use sift_test_util::{ + grpc::memory_sift_channel, + mock::user_defined_functions::v1::MockUserDefinedFunctionServiceImpl, +}; +use tokio::task::JoinHandle; +use tonic::{Response, Status, transport::Server}; + +use super::{UdfUpdate, UserDefinedFunctionService}; +use crate::policy::RetryPolicy; +use crate::service::common::DEFAULT_LIMIT; + +fn string_metadata(name: &str, value: &str) -> MetadataValue { + MetadataValue { + key: Some(MetadataKey { + name: name.into(), + r#type: MetadataKeyType::String.into(), + ..Default::default() + }), + value: Some(MetadataValueInner::StringValue(value.into())), + ..Default::default() + } +} + +fn numeric_input(identifier: &str) -> FunctionInput { + FunctionInput { + identifier: identifier.into(), + data_type: FunctionDataType::Numeric.into(), + constant: false, + } +} + +fn udf(id: &str, name: &str) -> UserDefinedFunction { + UserDefinedFunction { + user_defined_function_id: id.into(), + name: name.into(), + ..Default::default() + } +} + +async fn service_with_mock( + mock: MockUserDefinedFunctionServiceImpl, +) -> (UserDefinedFunctionService, 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(UserDefinedFunctionServiceServer::new(mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) + .await + .unwrap(); + }); + + ( + UserDefinedFunctionService::new(channel, RetryPolicy::default()), + handle, + ) +} + +#[tokio::test] +async fn list_user_defined_functions_returns_single_page() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.filter == "name.matches(\"(?i)rms\")" + && req.order_by == "name" + && req.page_size == DEFAULT_LIMIT + && req.page_token.is_empty() + }) + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: vec![udf("f1", "rms"), udf("f2", "rms_windowed")], + next_page_token: String::new(), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let functions = service + .list_user_defined_functions( + "name.matches(\"(?i)rms\")".to_string(), + Some("name".to_string()), + None, + ) + .await + .expect("list_user_defined_functions failed"); + + assert_eq!(functions.len(), 2); + assert_eq!(functions[0].user_defined_function_id, "f1"); + assert_eq!(functions[1].user_defined_function_id, "f2"); +} + +#[tokio::test] +async fn list_user_defined_functions_paginates_until_token_empty() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions().returning(|req| { + let req = req.into_inner(); + assert_eq!(req.page_size, DEFAULT_LIMIT); + let (functions, next) = match req.page_token.as_str() { + "" => (vec![udf("f1", "a")], "page-2".to_string()), + "page-2" => (vec![udf("f2", "b")], "page-3".to_string()), + "page-3" => (vec![udf("f3", "c")], String::new()), + other => return Err(Status::invalid_argument(format!("bad token: {other}"))), + }; + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: functions, + next_page_token: next, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let functions = service + .list_user_defined_functions(String::new(), None, None) + .await + .expect("list_user_defined_functions failed"); + + let ids: Vec<&str> = functions + .iter() + .map(|f| f.user_defined_function_id.as_str()) + .collect(); + assert_eq!(ids, vec!["f1", "f2", "f3"]); +} + +#[tokio::test] +async fn list_user_defined_functions_truncates_to_limit_across_pages() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions().returning(|req| { + let req = req.into_inner(); + assert_eq!(req.page_size, 3); + let (functions, next) = match req.page_token.as_str() { + "" => (vec![udf("f1", "a"), udf("f2", "b")], "page-2".to_string()), + "page-2" => (vec![udf("f3", "c"), udf("f4", "d")], String::new()), + other => return Err(Status::invalid_argument(format!("bad token: {other}"))), + }; + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: functions, + next_page_token: next, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let functions = service + .list_user_defined_functions(String::new(), None, Some(3)) + .await + .expect("list_user_defined_functions failed"); + + let ids: Vec<&str> = functions + .iter() + .map(|f| f.user_defined_function_id.as_str()) + .collect(); + assert_eq!(ids, vec!["f1", "f2", "f3"]); +} + +#[tokio::test] +async fn list_user_defined_functions_clamps_limit_to_page_size() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .times(1) + .withf(|req| req.get_ref().page_size == 200) + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: vec![udf("f1", "a")], + next_page_token: String::new(), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + service + .list_user_defined_functions(String::new(), None, Some(5_000)) + .await + .expect("list_user_defined_functions failed"); +} + +#[tokio::test] +async fn list_user_defined_functions_breaks_on_empty_page() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .times(1) + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: vec![], + next_page_token: "ignored".into(), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let functions = service + .list_user_defined_functions(String::new(), None, None) + .await + .expect("list_user_defined_functions failed"); + + assert!(functions.is_empty()); +} + +#[tokio::test] +async fn list_user_defined_functions_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .returning(|_| Err(Status::invalid_argument("bad filter"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .list_user_defined_functions("nope".to_string(), None, None) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to query user defined functions") + ); +} + +#[tokio::test] +async fn list_versions_sends_id_filter_and_order_by() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.user_defined_function_id == "f1" + && req.name.is_empty() + && req.filter == "version == 2" + && req.order_by == "version desc" + && req.page_size == DEFAULT_LIMIT + && req.page_token.is_empty() + }) + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: vec![UserDefinedFunction { + user_defined_function_id: "f1".into(), + user_defined_function_version_id: "v2".into(), + version: 2, + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let versions = service + .list_user_defined_function_versions( + "f1".to_string(), + String::new(), + "version == 2".to_string(), + Some("version desc".to_string()), + None, + ) + .await + .expect("list_user_defined_function_versions failed"); + + assert_eq!(versions.len(), 1); + assert_eq!(versions[0].version, 2); +} + +#[tokio::test] +async fn list_versions_sends_name_when_id_is_empty() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.user_defined_function_id.is_empty() && req.name == "rms" + }) + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: vec![udf("f1", "rms")], + next_page_token: String::new(), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + service + .list_user_defined_function_versions( + String::new(), + "rms".to_string(), + String::new(), + None, + None, + ) + .await + .expect("list_user_defined_function_versions failed"); +} + +#[tokio::test] +async fn list_versions_paginates_and_truncates_to_limit() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .returning(|req| { + let req = req.into_inner(); + assert_eq!(req.page_size, 2); + let (functions, next) = match req.page_token.as_str() { + "" => ( + vec![ + UserDefinedFunction { + version: 1, + ..Default::default() + }, + UserDefinedFunction { + version: 2, + ..Default::default() + }, + ], + "page-2".to_string(), + ), + "page-2" => ( + vec![UserDefinedFunction { + version: 3, + ..Default::default() + }], + String::new(), + ), + other => return Err(Status::invalid_argument(format!("bad token: {other}"))), + }; + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: functions, + next_page_token: next, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let versions = service + .list_user_defined_function_versions( + "f1".to_string(), + String::new(), + String::new(), + None, + Some(2), + ) + .await + .expect("list_user_defined_function_versions failed"); + + let numbers: Vec = versions.iter().map(|v| v.version).collect(); + assert_eq!(numbers, vec![1, 2]); +} + +#[tokio::test] +async fn list_versions_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .returning(|_| Err(Status::not_found("no such function"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .list_user_defined_function_versions( + "missing".to_string(), + String::new(), + String::new(), + None, + None, + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to query user defined function versions") + ); +} + +#[tokio::test] +async fn create_sends_every_provided_field() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.name == "rms" + && req.description.as_deref() == Some("root mean square") + && req.expression == "sqrt(mean($x * $x))" + && req.function_inputs.len() == 1 + && req.function_inputs[0].identifier == "x" + && req.function_inputs[0].data_type == i32::from(FunctionDataType::Numeric) + && req.user_notes.as_deref() == Some("initial version") + && req.metadata.len() == 1 + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(CreateUserDefinedFunctionResponse { + user_defined_function: Some(UserDefinedFunction { + user_defined_function_id: "f1".into(), + user_defined_function_version_id: "v1".into(), + version: 1, + name: req.name, + expression: req.expression, + function_inputs: req.function_inputs, + ..Default::default() + }), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let created = service + .create_user_defined_function( + "rms".to_string(), + Some("root mean square".to_string()), + "sqrt(mean($x * $x))".to_string(), + vec![numeric_input("x")], + Some("initial version".to_string()), + vec![string_metadata("owner", "avionics")], + ) + .await + .expect("create_user_defined_function failed"); + + assert_eq!(created.user_defined_function_id, "f1"); + assert_eq!(created.version, 1); +} + +#[tokio::test] +async fn create_omits_optional_fields_when_absent() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.description.is_none() && req.user_notes.is_none() && req.metadata.is_empty() + }) + .returning(|_| { + Ok(Response::new(CreateUserDefinedFunctionResponse { + user_defined_function: Some(udf("f1", "rms")), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + service + .create_user_defined_function( + "rms".to_string(), + None, + "$x".to_string(), + vec![numeric_input("x")], + None, + Vec::new(), + ) + .await + .expect("create_user_defined_function failed"); +} + +#[tokio::test] +async fn create_errors_when_response_missing_function() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function().returning(|_| { + Ok(Response::new(CreateUserDefinedFunctionResponse { + user_defined_function: None, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .create_user_defined_function( + "rms".to_string(), + None, + "$x".to_string(), + vec![numeric_input("x")], + None, + Vec::new(), + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("create_user_defined_function response missing user defined function") + ); +} + +#[tokio::test] +async fn create_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function() + .returning(|_| Err(Status::invalid_argument("bad expression"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .create_user_defined_function( + "rms".to_string(), + None, + "nonsense(".to_string(), + vec![], + None, + Vec::new(), + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to create user defined function") + ); +} + +#[tokio::test] +async fn update_masks_only_the_provided_fields() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req.user_defined_function.as_ref().expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.user_defined_function_id == "f1" + && function.description == "updated" + && function.expression.is_empty() + && function.function_inputs.is_empty() + && function.metadata.is_empty() + && mask.paths == vec!["description".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let updated = service + .update_user_defined_function( + "f1".to_string(), + UdfUpdate { + description: Some("updated".to_string()), + ..Default::default() + }, + ) + .await + .expect("update_user_defined_function failed"); + + assert_eq!(updated.user_defined_function_id, "f1"); +} + +#[tokio::test] +async fn update_masks_every_provided_field_in_declaration_order() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req.user_defined_function.as_ref().expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.name == "rms_v2" + && function.description == "updated" + && function.expression == "$x * 2" + && function.function_inputs.len() == 1 + && function.metadata.len() == 1 + && mask.paths + == vec![ + "name".to_string(), + "description".to_string(), + "expression".to_string(), + "function_inputs".to_string(), + "metadata".to_string(), + ] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + service + .update_user_defined_function( + "f1".to_string(), + UdfUpdate { + name: Some("rms_v2".to_string()), + description: Some("updated".to_string()), + expression: Some("$x * 2".to_string()), + function_inputs: Some(vec![numeric_input("x")]), + metadata: Some(vec![string_metadata("owner", "avionics")]), + }, + ) + .await + .expect("update_user_defined_function failed"); +} + +#[tokio::test] +async fn update_sends_no_version_precondition() { + // `UpdateUserDefinedFunctionRequest` has no version precondition field: the + // service always creates a new version and returns it. Assert the request + // carries no stale version identifiers that could be mistaken for one, and + // that the caller sees the new version from the response. + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let function = req + .get_ref() + .user_defined_function + .as_ref() + .expect("function present"); + function.version == 0 + && function.user_defined_function_version_id.is_empty() + && function.function_dependencies.is_empty() + }) + .returning(|req| { + let req = req.into_inner(); + let mut function = req.user_defined_function.expect("function present"); + function.version = 7; + function.user_defined_function_version_id = "v7".into(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: Some(function), + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let updated = service + .update_user_defined_function( + "f1".to_string(), + UdfUpdate { + expression: Some("$x + 1".to_string()), + ..Default::default() + }, + ) + .await + .expect("update_user_defined_function failed"); + + assert_eq!(updated.version, 7); + assert_eq!(updated.user_defined_function_version_id, "v7"); +} + +#[tokio::test] +async fn update_with_no_fields_sends_an_empty_mask() { + // The tool handler rejects this case; the service contract is to send + // exactly what it was given. + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req.user_defined_function.as_ref().expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.user_defined_function_id == "f1" && mask.paths.is_empty() + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + service + .update_user_defined_function("f1".to_string(), UdfUpdate::default()) + .await + .expect("update_user_defined_function failed"); +} + +#[tokio::test] +async fn update_errors_when_response_missing_function() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function().returning(|_| { + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: None, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .update_user_defined_function( + "f1".to_string(), + UdfUpdate { + description: Some("x".to_string()), + ..Default::default() + }, + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("update_user_defined_function response missing user defined function") + ); +} + +#[tokio::test] +async fn update_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .returning(|_| Err(Status::failed_precondition("function has dependents"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .update_user_defined_function( + "f1".to_string(), + UdfUpdate { + function_inputs: Some(vec![numeric_input("x")]), + ..Default::default() + }, + ) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to update user defined function") + ); +} + +#[tokio::test] +async fn archive_sets_is_archived_true_through_the_mask() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req.user_defined_function.as_ref().expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.user_defined_function_id == "f1" + && function.is_archived + && function.archived_date.is_none() + && mask.paths == vec!["is_archived".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let archived = service + .set_user_defined_function_archived("f1".to_string(), true) + .await + .expect("set_user_defined_function_archived failed"); + + assert!(archived.is_archived); +} + +#[tokio::test] +async fn unarchive_sets_is_archived_false_through_the_mask() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req.user_defined_function.as_ref().expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.user_defined_function_id == "f2" + && !function.is_archived + && mask.paths == vec!["is_archived".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (service, _h) = service_with_mock(mock).await; + + let unarchived = service + .set_user_defined_function_archived("f2".to_string(), false) + .await + .expect("set_user_defined_function_archived failed"); + + assert!(!unarchived.is_archived); +} + +#[tokio::test] +async fn set_archived_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .returning(|_| Err(Status::not_found("function missing"))); + + let (service, _h) = service_with_mock(mock).await; + + let err = service + .set_user_defined_function_archived("missing".to_string(), true) + .await + .expect_err("expected error"); + + assert!( + err.to_string() + .contains("failed to update user defined function") + ); +} diff --git a/rust/crates/sift_test_util/src/mock/mod.rs b/rust/crates/sift_test_util/src/mock/mod.rs index a7a7c812c8..8d9c64c105 100644 --- a/rust/crates/sift_test_util/src/mock/mod.rs +++ b/rust/crates/sift_test_util/src/mock/mod.rs @@ -11,6 +11,7 @@ pub mod rule_evaluation; pub mod rules; pub mod runs; pub mod test_reports; +pub mod user_defined_functions; pub mod users; /// A test demonstrating a little bit of everything of how to leverage the mock API. diff --git a/rust/crates/sift_test_util/src/mock/user_defined_functions/mod.rs b/rust/crates/sift_test_util/src/mock/user_defined_functions/mod.rs new file mode 100644 index 0000000000..a3a6d96c3f --- /dev/null +++ b/rust/crates/sift_test_util/src/mock/user_defined_functions/mod.rs @@ -0,0 +1 @@ +pub mod v1; diff --git a/rust/crates/sift_test_util/src/mock/user_defined_functions/v1.rs b/rust/crates/sift_test_util/src/mock/user_defined_functions/v1.rs new file mode 100644 index 0000000000..38333df829 --- /dev/null +++ b/rust/crates/sift_test_util/src/mock/user_defined_functions/v1.rs @@ -0,0 +1,94 @@ +use async_trait::async_trait; +use mockall::mock; +use sift_rs::user_defined_functions::v1::{ + CheckUpdatableFieldsRequest, CheckUpdatableFieldsResponse, CreateUserDefinedFunctionRequest, + CreateUserDefinedFunctionResponse, GetUserDefinedFunctionDependentsRequest, + GetUserDefinedFunctionDependentsResponse, GetUserDefinedFunctionRequest, + GetUserDefinedFunctionResponse, GetUserDefinedFunctionVersionRequest, + GetUserDefinedFunctionVersionResponse, GetUserDefinedFunctionVersionsRequest, + GetUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionVersionsRequest, + ListUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionsRequest, + ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionRequest, + UpdateUserDefinedFunctionResponse, ValidateUserDefinedFunctionRequest, + ValidateUserDefinedFunctionResponse, + user_defined_function_service_server::UserDefinedFunctionService, +}; +use tonic::{Request, Response, Status}; + +mock! { + pub UserDefinedFunctionServiceImpl {} + + #[async_trait] + impl UserDefinedFunctionService for UserDefinedFunctionServiceImpl { + async fn get_user_defined_function( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn get_user_defined_function_version( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn get_user_defined_function_versions( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn get_user_defined_function_dependents( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn create_user_defined_function( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn validate_user_defined_function( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn update_user_defined_function( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn check_updatable_fields( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn list_user_defined_functions( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + async fn list_user_defined_function_versions( + &self, + request: Request, + ) -> std::result::Result< + Response, + Status, + >; + } +} From 5bf5db1d80d394ad51d81e35082fd93f90c73c17 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Tue, 18 Aug 2026 22:19:06 -0700 Subject: [PATCH 2/8] feat: add user defined function MCP tools --- rust/crates/sift_mcp/src/server/mod.rs | 6 + .../service/user_defined_functions/test.rs | 28 +- rust/crates/sift_mcp/src/tool/mod.rs | 1 + .../src/tool/user_defined_functions/mod.rs | 651 ++++++++++++++++++ .../src/tool/user_defined_functions/test.rs | 560 +++++++++++++++ rust/crates/sift_mcp/src/tool_events.json | 6 + 6 files changed, 1245 insertions(+), 7 deletions(-) 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 diff --git a/rust/crates/sift_mcp/src/server/mod.rs b/rust/crates/sift_mcp/src/server/mod.rs index 5aafa0ca58..4a7ebed0a3 100644 --- a/rust/crates/sift_mcp/src/server/mod.rs +++ b/rust/crates/sift_mcp/src/server/mod.rs @@ -44,6 +44,7 @@ use crate::service::{ docs::DocsService, ingest::IngestService, ping::PingService, report_templates::ReportTemplateService, reports::ReportService, rule_evaluation::RuleEvaluationService, rules::RuleService, runs::RunService, url::UrlService, + user_defined_functions::UserDefinedFunctionService, users::UserService, }; @@ -68,6 +69,7 @@ pub struct SiftMcpServer { #[cfg(feature = "test-reports")] pub test_report_service: TestReportService, pub docs_service: DocsService, + pub user_defined_function_service: UserDefinedFunctionService, pub user_service: UserService, pub allow_create: bool, @@ -195,6 +197,7 @@ impl SiftMcpServer { #[cfg(feature = "test-reports")] tool_router.merge(Self::test_reports_router()); tool_router.merge(Self::docs_router()); + tool_router.merge(Self::user_defined_functions_router()); tool_router.merge(Self::users_router()); if update_check.is_some() { tool_router.merge(Self::update_router()); @@ -223,6 +226,8 @@ impl SiftMcpServer { #[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()); + let user_defined_function_service = + UserDefinedFunctionService::new(channel.clone(), retry_policy.clone()); let user_service = UserService::new(channel.clone(), retry_policy); Self { @@ -242,6 +247,7 @@ impl SiftMcpServer { #[cfg(feature = "test-reports")] test_report_service, docs_service, + user_defined_function_service, user_service, tool_router, prompt_router, diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs index 30a9f0d394..0e3e07b41e 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs @@ -10,8 +10,7 @@ use sift_rs::{ }, }; use sift_test_util::{ - grpc::memory_sift_channel, - mock::user_defined_functions::v1::MockUserDefinedFunctionServiceImpl, + grpc::memory_sift_channel, mock::user_defined_functions::v1::MockUserDefinedFunctionServiceImpl, }; use tokio::task::JoinHandle; use tonic::{Response, Status, transport::Server}; @@ -520,7 +519,10 @@ async fn update_masks_only_the_provided_fields() { .times(1) .withf(|req| { let req = req.get_ref(); - let function = req.user_defined_function.as_ref().expect("function present"); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f1" && function.description == "updated" @@ -559,7 +561,10 @@ async fn update_masks_every_provided_field_in_declaration_order() { .times(1) .withf(|req| { let req = req.get_ref(); - let function = req.user_defined_function.as_ref().expect("function present"); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); let mask = req.update_mask.as_ref().expect("mask present"); function.name == "rms_v2" && function.description == "updated" @@ -654,7 +659,10 @@ async fn update_with_no_fields_sends_an_empty_mask() { .times(1) .withf(|req| { let req = req.get_ref(); - let function = req.user_defined_function.as_ref().expect("function present"); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f1" && mask.paths.is_empty() }) @@ -733,7 +741,10 @@ async fn archive_sets_is_archived_true_through_the_mask() { .times(1) .withf(|req| { let req = req.get_ref(); - let function = req.user_defined_function.as_ref().expect("function present"); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f1" && function.is_archived @@ -764,7 +775,10 @@ async fn unarchive_sets_is_archived_false_through_the_mask() { .times(1) .withf(|req| { let req = req.get_ref(); - let function = req.user_defined_function.as_ref().expect("function present"); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f2" && !function.is_archived diff --git a/rust/crates/sift_mcp/src/tool/mod.rs b/rust/crates/sift_mcp/src/tool/mod.rs index 93d7bc26df..dc9858e1fe 100644 --- a/rust/crates/sift_mcp/src/tool/mod.rs +++ b/rust/crates/sift_mcp/src/tool/mod.rs @@ -15,4 +15,5 @@ pub mod runs; #[cfg(feature = "test-reports")] pub mod test_reports; pub mod update; +pub mod user_defined_functions; pub mod users; 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 new file mode 100644 index 0000000000..05deb8d779 --- /dev/null +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs @@ -0,0 +1,651 @@ +use rmcp::{ + ErrorData, + handler::server::wrapper::Parameters, + model::{CallToolResult, ContentBlock}, + schemars::{self, JsonSchema}, + tool, tool_router, +}; +use serde::Deserialize; +use sift_rs::{ + common::r#type::v1::{FunctionDataType, FunctionInput}, + metadata::v1::MetadataValue, +}; + +use crate::{ + error::{self, from_anyhow}, + server::SiftMcpServer, + service::user_defined_functions::UdfUpdate, + tool::common::{ListParams, MetadataEntry}, +}; + +#[cfg(test)] +mod test; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct UserDefinedFunctionVersionListParams { + pub(crate) user_defined_function_id: Option, + pub(crate) name: Option, + pub(crate) filter: Option, + pub(crate) order_by: Option, + pub(crate) limit: Option, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct CreateUserDefinedFunctionParams { + pub(crate) name: String, + pub(crate) expression: String, + pub(crate) function_inputs_json: String, + pub(crate) description: Option, + pub(crate) user_notes: Option, + pub(crate) metadata: Option>, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct UpdateUserDefinedFunctionParams { + pub(crate) user_defined_function_id: String, + pub(crate) name: Option, + pub(crate) description: Option, + pub(crate) expression: Option, + pub(crate) function_inputs_json: Option, + pub(crate) metadata: Option>, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct ArchiveUserDefinedFunctionParams { + pub(crate) user_defined_function_id: String, +} + +/// One entry of the `function_inputs_json` array. The proto's `FunctionInput` is +/// a nested message, so it arrives as a documented JSON string scalar and is +/// parsed here. +#[derive(Debug, Deserialize)] +struct FunctionInputSpec { + identifier: String, + #[serde(alias = "dataType")] + data_type: String, + #[serde(default)] + constant: bool, +} + +#[tool_router(router = user_defined_functions_router, vis = "pub(crate)")] +impl SiftMcpServer { + #[tool( + name = "list_user_defined_functions", + description = " + List the latest version of each user defined function in Sift, optionally filtered by a CEL + expression and ordered by one or more fields. A user defined function is a named, reusable + expression that calculated channels and rules can call. + + Output: + - `{ \"user_defined_functions\": [UserDefinedFunction, ...] }`. Each item carries + `user_defined_function_id`, `name`, `description`, `expression`, `function_inputs` + (`identifier`, `data_type`, `constant`), `function_output_type`, `function_dependencies` + (the version ids of other functions this one calls), `user_defined_function_version_id`, + `version`, `change_message`, `user_notes`, `metadata`, timestamps, author ids, and archive + state. + - Fields at their proto3 default are OMITTED from the JSON: a missing `is_archived` or + `version` key means `false` / `0`, not \"unknown\". + + Parameters: + - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: + `user_defined_function_id`, `name`, `archived_date`, `is_archived`. + `name` is the only free-text field. When filtering or searching it, use + `name.matches(\"(?i)rms\")`, not `==`. Use `==` only for an exact value from a prior + result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"RMS\")` + silently misses `rms_window`. + - `order_by`: optional comma-separated `FIELD_NAME[ desc]` list. Orderable fields: + `created_date`, `modified_date`, `name`. Default sort is `created_date desc` + (newest first). Example: `\"name,modified_date desc\"`. + - `limit`: max items to return. Start at 50 and only raise it if the result is capped + and you still need more. Values are clamped to `1..=200`; omitting it defaults to 50. + + Errors: + - `INVALID_PARAMS` if `filter` is not a valid CEL expression or `order_by` references an + unknown field. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - Call this BEFORE authoring an expression that depends on a user defined function. The + function's exact `name`, its `function_inputs` order, and its `function_output_type` are + what a caller has to match; guessing them produces an expression the server rejects. + - Default add `is_archived == false` to the filter. Include archived functions only when + the user explicitly asks for them. + ", + annotations( + title = "user_defined_functions/list_user_defined_functions", + read_only_hint = true + ) + )] + pub async fn list_user_defined_functions( + &self, + params: Parameters, + ) -> error::McpResult { + let Parameters(ListParams { + filter, + order_by, + limit, + }) = params; + + let functions = self + .user_defined_function_service + .list_user_defined_functions(filter, order_by, limit) + .await + .map_err(from_anyhow)?; + + Ok(CallToolResult::structured( + serde_json::json!({ "user_defined_functions": functions }), + )) + } + + #[tool( + name = "list_user_defined_function_versions", + description = " + List the version history of one user defined function. Every accepted update creates a new + version and leaves the previous one intact, so this is how you read what changed and when. + + Output: + - `{ \"user_defined_function_versions\": [UserDefinedFunction, ...] }`. Each item is one + version with the same shape `list_user_defined_functions` returns, including + `user_defined_function_version_id`, `version`, `expression`, `function_inputs`, + `change_message` (server-generated summary of the change), `user_notes`, and + `modified_by_user_id`. + + Parameters: + - `user_defined_function_id`: optional. The id of the function whose versions to list. + - `name`: optional. The name of the function whose versions to list. + - Exactly one of `user_defined_function_id` or `name` must be set. + - `filter`: optional CEL expression. Omit or pass an empty string to list every version. + Filterable fields: `user_defined_function_id`, `name`, `version`, `archived_date`, + `is_archived`. `name` is the only free-text field; when filtering or searching it, use + `name.matches(\"(?i)rms\")`, not `==`. Use `==` only for an exact value from a prior + result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"RMS\")` + silently misses `rms_window`. + - `order_by`: optional comma-separated `FIELD_NAME[ desc]` list. Orderable fields: + `created_date`, `modified_date`, `name`, `version`. When empty, items come back ordered + by `name` ascending — pass `\"version desc\"` for newest-first. + - `limit`: max items to return. Start at 50 and only raise it if the result is capped + and you still need more. Values are clamped to `1..=200`; omitting it defaults to 50. + + Errors: + - `INVALID_PARAMS` if neither or both of `user_defined_function_id` and `name` are set, or + if `filter` is not a valid CEL expression. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - Use this to find the `user_defined_function_version_id` a calculated channel or another + function pins, or to show the user how an expression evolved. Resolve the id with + `list_user_defined_functions` first when you only have a partial name. + ", + annotations( + title = "user_defined_functions/list_user_defined_function_versions", + read_only_hint = true + ) + )] + pub async fn list_user_defined_function_versions( + &self, + params: Parameters, + ) -> error::McpResult { + let Parameters(UserDefinedFunctionVersionListParams { + user_defined_function_id, + name, + filter, + order_by, + limit, + }) = params; + + let (user_defined_function_id, name) = function_identifier(user_defined_function_id, name)?; + + let versions = self + .user_defined_function_service + .list_user_defined_function_versions( + user_defined_function_id, + name, + filter.unwrap_or_default(), + order_by, + limit, + ) + .await + .map_err(from_anyhow)?; + + Ok(CallToolResult::structured( + serde_json::json!({ "user_defined_function_versions": versions }), + )) + } + + #[tool( + name = "create_user_defined_function", + description = " + Create a user defined function: a named, reusable expression that calculated channels, rules, + and other user defined functions can call. This is a WRITE. + + Output: + - `{ \"user_defined_function\": UserDefinedFunction, \"user_defined_function_id\": \"\", + \"next_step\": \"...\" }`. The returned function is the server's post-create state, + including the resolved `function_output_type` and `version` 1. + + Parameters: + - `name`: required. The name callers use to reference the function. Must be non-empty. + - `expression`: required. The function body. Reference each declared input by its + `identifier`. Mirror an existing function retrieved with `list_user_defined_functions` + rather than authoring the syntax blind. + - `function_inputs_json`: required. A JSON array string declaring the function's inputs, in + the order callers pass them. Each element is + `{ \"identifier\": \"\", \"data_type\": \"numeric\"|\"string\"|\"bool\", + \"constant\": }`. `constant` defaults to `false` and marks an input that takes a + literal value rather than a channel. Pass `[]` for a function that takes no inputs. + - `description`: optional. Human-readable summary. + - `user_notes`: optional. Notes recorded against this first version. + - `metadata`: optional. Array of `{ \"name\": \"\", \"value\": }` where + `value` is a string, number, or boolean. A `name` that does not yet exist in the + organization's metadata schema is created on the fly with the type inferred from `value`; + for an existing key the type must match. + + Errors: + - `INVALID_PARAMS` if `name` or `expression` is empty, `function_inputs_json` is not a JSON + array of input objects, an `identifier` is empty, a `data_type` is not one of `numeric`, + `string`, or `bool`, or the server rejects the expression (unresolved identifier, type + mismatch, duplicate function name). + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - This creates a live resource. Confirm the name, the expression, and the input list with + the user before calling. + - An expression may call other user defined functions. List them first so you match their + real names, input order, and output types; the server records the calls as + `function_dependencies`, and those dependencies then restrict what can be updated later. + ", + annotations( + title = "user_defined_functions/create_user_defined_function", + read_only_hint = false, + destructive_hint = false, + idempotent_hint = false, + ) + )] + pub async fn create_user_defined_function( + &self, + params: Parameters, + ) -> error::McpResult { + self.require_create()?; + + let Parameters(CreateUserDefinedFunctionParams { + name, + expression, + function_inputs_json, + description, + user_notes, + metadata, + }) = params; + + if name.trim().is_empty() { + return Err(ErrorData::invalid_params("`name` must not be empty", None)); + } + if expression.trim().is_empty() { + return Err(ErrorData::invalid_params( + "`expression` must not be empty", + None, + )); + } + + let function_inputs = parse_function_inputs(&function_inputs_json)?; + let metadata = metadata_values(metadata).unwrap_or_default(); + + let function = self + .user_defined_function_service + .create_user_defined_function( + name, + description, + expression, + function_inputs, + user_notes, + metadata, + ) + .await + .map_err(from_anyhow)?; + + let next_step = format!( + "Created user defined function `{}` with id `{}` at version {}. Tell the user the new id \ + and confirm the expression matches their intent. Reference it from a calculated channel \ + or rule by name.", + function.name, function.user_defined_function_id, function.version, + ); + + let mut result = CallToolResult::structured(serde_json::json!({ + "user_defined_function": function, + "user_defined_function_id": function.user_defined_function_id, + "next_step": next_step, + })); + result.content = vec![ContentBlock::text(next_step)]; + Ok(result) + } + + #[tool( + name = "update_user_defined_function", + description = " + Update a user defined function. This is a WRITE. Only the fields you set are written; the + rest of the function is left as it is. The update creates a NEW version and leaves the + previous version intact, so callers pinned to an older version are unaffected. + + Output: + - `{ \"user_defined_function\": UserDefinedFunction, \"next_step\": \"...\" }`. The + returned function is the newly created version, with its own + `user_defined_function_version_id` and incremented `version`. + + Parameters: + - `user_defined_function_id`: required. The function to update. + - `name`: optional. New name. The API applies a rename BY ITSELF and ignores any other + field in the same call, so this tool rejects `name` combined with another field — send + the rename as its own call. + - `description`: optional. New description. + - `expression`: optional. New function body. + - `function_inputs_json`: optional. REPLACES the declared input list. Same array shape as + `create_user_defined_function`. + - `metadata`: optional. REPLACES the function's full metadata list. Same entry shape as + `create_user_defined_function`; pass `[]` to clear. + - At least one field besides `user_defined_function_id` must be set. + - Archive state is not settable here. Use `archive_user_defined_function` / + `unarchive_user_defined_function`. + + Errors: + - `INVALID_PARAMS` if no updatable field is set, `name` is combined with another field, or + `function_inputs_json` is not a valid input array. The server also rejects updates that + its dependency rules forbid: `name` cannot change once the function has ever had + dependencies, `function_inputs` cannot change while any function or calculated channel + depends on this one, and `expression` cannot change the output type while dependents + exist. + - `RESOURCE_NOT_FOUND` if no function matches `user_defined_function_id`. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - Confirm the target function and the exact changes with the user before calling. + - `function_inputs_json` and `metadata` are REPLACE, not merge. Read the current values + with `list_user_defined_functions` (filter `user_defined_function_id == \"\"`) and + send the full intended list. + - There is no version precondition on this RPC, so a concurrent edit is not detected. Read + the function immediately before updating when a change may be racing another author. + ", + annotations( + title = "user_defined_functions/update_user_defined_function", + read_only_hint = false, + destructive_hint = true, + idempotent_hint = true, + ) + )] + pub async fn update_user_defined_function( + &self, + params: Parameters, + ) -> error::McpResult { + self.require_destructive()?; + + let Parameters(UpdateUserDefinedFunctionParams { + user_defined_function_id, + name, + description, + expression, + function_inputs_json, + metadata, + }) = params; + + if user_defined_function_id.is_empty() { + return Err(ErrorData::invalid_params( + "`user_defined_function_id` must not be empty", + None, + )); + } + + let function_inputs = function_inputs_json + .as_deref() + .map(parse_function_inputs) + .transpose()?; + + let changes = UdfUpdate { + name, + description, + expression, + function_inputs, + metadata: metadata_values(metadata), + }; + + let others_set = changes.description.is_some() + || changes.expression.is_some() + || changes.function_inputs.is_some() + || changes.metadata.is_some(); + + if changes.name.is_none() && !others_set { + return Err(ErrorData::invalid_params( + "at least one of `name`, `description`, `expression`, `function_inputs_json`, or \ + `metadata` must be set", + None, + )); + } + if changes.name.is_some() && others_set { + return Err(ErrorData::invalid_params( + "the API applies a `name` change on its own and ignores every other field in the \ + same request; send the rename as a separate call", + None, + )); + } + + let function = self + .user_defined_function_service + .update_user_defined_function(user_defined_function_id, changes) + .await + .map_err(from_anyhow)?; + + let next_step = format!( + "Updated user defined function `{}` ({}); it is now at version {}. Earlier versions are \ + untouched. Surface the new state to the user and confirm nothing was unintentionally \ + replaced — input and metadata lists are REPLACE operations.", + function.name, function.user_defined_function_id, function.version, + ); + + let mut result = CallToolResult::structured(serde_json::json!({ + "user_defined_function": function, + "next_step": next_step, + })); + result.content = vec![ContentBlock::text(next_step)]; + Ok(result) + } + + #[tool( + name = "archive_user_defined_function", + description = " + Archive a user defined function so it stops appearing as an available function. This is a + WRITE. Reversible with `unarchive_user_defined_function`. + + Output: + - `{ \"archived\": true, \"user_defined_function\": UserDefinedFunction, + \"next_step\": \"...\" }`. The returned function is the post-archive state. + + Parameters: + - `user_defined_function_id`: required. The function to archive. + + Errors: + - `INVALID_PARAMS` if `user_defined_function_id` is empty. + - `RESOURCE_NOT_FOUND` if no function matches `user_defined_function_id`. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - Archiving does not delete the function and does not rewrite anything that already calls + it. Confirm the target with the user before calling, and check for dependents first — + calculated channels and rules that call the function keep referencing it. + ", + annotations( + title = "user_defined_functions/archive_user_defined_function", + read_only_hint = false, + destructive_hint = true, + idempotent_hint = true, + ) + )] + pub async fn archive_user_defined_function( + &self, + params: Parameters, + ) -> error::McpResult { + self.require_destructive()?; + + let Parameters(ArchiveUserDefinedFunctionParams { + user_defined_function_id, + }) = params; + + let function = self.set_archived(user_defined_function_id, true).await?; + + let next_step = format!( + "Archived user defined function `{}` ({}). Tell the user it no longer appears as an \ + available function and that `unarchive_user_defined_function` restores it.", + function.name, function.user_defined_function_id, + ); + + let mut result = CallToolResult::structured(serde_json::json!({ + "archived": true, + "user_defined_function": function, + "next_step": next_step, + })); + result.content = vec![ContentBlock::text(next_step)]; + Ok(result) + } + + #[tool( + name = "unarchive_user_defined_function", + description = " + Restore a previously archived user defined function. This is a WRITE. + + Output: + - `{ \"unarchived\": true, \"user_defined_function\": UserDefinedFunction, + \"next_step\": \"...\" }`. The returned function is the post-unarchive state. + + Parameters: + - `user_defined_function_id`: required. The function to unarchive. + + Errors: + - `INVALID_PARAMS` if `user_defined_function_id` is empty. + - `RESOURCE_NOT_FOUND` if no function matches `user_defined_function_id`. + - `INTERNAL_ERROR` for upstream gRPC failures. + + Guidance: + - Confirm the target function with the user before calling. Find archived functions with + `list_user_defined_functions` and filter `is_archived == true`. + ", + annotations( + title = "user_defined_functions/unarchive_user_defined_function", + read_only_hint = false, + destructive_hint = true, + idempotent_hint = true, + ) + )] + pub async fn unarchive_user_defined_function( + &self, + params: Parameters, + ) -> error::McpResult { + self.require_destructive()?; + + let Parameters(ArchiveUserDefinedFunctionParams { + user_defined_function_id, + }) = params; + + let function = self.set_archived(user_defined_function_id, false).await?; + + let next_step = format!( + "Unarchived user defined function `{}` ({}). Tell the user it is available again.", + function.name, function.user_defined_function_id, + ); + + let mut result = CallToolResult::structured(serde_json::json!({ + "unarchived": true, + "user_defined_function": function, + "next_step": next_step, + })); + result.content = vec![ContentBlock::text(next_step)]; + Ok(result) + } +} + +impl SiftMcpServer { + /// Shared body of the archive and unarchive tools. Both flip the same + /// `is_archived` field through the update mask, so only the validation and + /// the reported wording differ. + async fn set_archived( + &self, + user_defined_function_id: String, + is_archived: bool, + ) -> Result { + if user_defined_function_id.is_empty() { + return Err(ErrorData::invalid_params( + "`user_defined_function_id` must not be empty", + None, + )); + } + + self.user_defined_function_service + .set_user_defined_function_archived(user_defined_function_id, is_archived) + .await + .map_err(from_anyhow) + } +} + +/// Resolve the `(user_defined_function_id, name)` request fields from the +/// mutually exclusive optional params. The proto silently ignores `name` when an +/// id is present, so reject the ambiguous call rather than pick for the caller. +fn function_identifier( + user_defined_function_id: Option, + name: Option, +) -> Result<(String, String), ErrorData> { + match (user_defined_function_id, name) { + (Some(id), None) => Ok((id, String::new())), + (None, Some(name)) => Ok((String::new(), name)), + (Some(_), Some(_)) => Err(ErrorData::invalid_params( + "exactly one of `user_defined_function_id` or `name` must be set, not both", + None, + )), + (None, None) => Err(ErrorData::invalid_params( + "one of `user_defined_function_id` or `name` must be set", + None, + )), + } +} + +/// Parse the documented `function_inputs_json` array into proto inputs, mapping +/// every shape error to `INVALID_PARAMS` so the agent can correct its input. +fn parse_function_inputs(function_inputs_json: &str) -> Result, ErrorData> { + let specs: Vec = + serde_json::from_str(function_inputs_json).map_err(|e| { + ErrorData::invalid_params( + format!( + "`function_inputs_json` is not a JSON array of \ + {{\"identifier\", \"data_type\", \"constant\"}} objects: {e}" + ), + None, + ) + })?; + + specs + .into_iter() + .map(|spec| { + if spec.identifier.trim().is_empty() { + return Err(ErrorData::invalid_params( + "every `function_inputs_json` entry needs a non-empty `identifier`", + None, + )); + } + Ok(FunctionInput { + identifier: spec.identifier, + data_type: parse_function_data_type(&spec.data_type)?.into(), + constant: spec.constant, + }) + }) + .collect() +} + +fn parse_function_data_type(data_type: &str) -> Result { + match data_type.to_ascii_lowercase().as_str() { + "numeric" | "function_data_type_numeric" => Ok(FunctionDataType::Numeric), + "string" | "function_data_type_string" => Ok(FunctionDataType::String), + "bool" | "boolean" | "function_data_type_bool" => Ok(FunctionDataType::Bool), + other => Err(ErrorData::invalid_params( + format!("unknown `data_type` `{other}`; expected `numeric`, `string`, or `bool`"), + None, + )), + } +} + +fn metadata_values(metadata: Option>) -> Option> { + metadata.map(|entries| entries.into_iter().map(MetadataValue::from).collect()) +} diff --git a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs new file mode 100644 index 0000000000..9a7b6b09d6 --- /dev/null +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs @@ -0,0 +1,560 @@ +use rmcp::{handler::server::wrapper::Parameters, model::ErrorCode}; +use sift_rs::{ + common::r#type::v1::{FunctionDataType, FunctionInput, UserDefinedFunction}, + user_defined_functions::v1::{ + CreateUserDefinedFunctionResponse, ListUserDefinedFunctionVersionsResponse, + ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionResponse, + user_defined_function_service_server::UserDefinedFunctionServiceServer, + }, +}; +use sift_test_util::{ + grpc::memory_sift_channel, mock::user_defined_functions::v1::MockUserDefinedFunctionServiceImpl, +}; +use tokio::task::JoinHandle; +use tonic::{Response, Status, transport::Server}; + +use crate::{ + server::SiftMcpServer, + tool::{ + common::test_support::{list_params, structured, structured_field}, + user_defined_functions::{ + ArchiveUserDefinedFunctionParams, CreateUserDefinedFunctionParams, + UpdateUserDefinedFunctionParams, UserDefinedFunctionVersionListParams, + }, + }, +}; + +async fn server_with_mock( + mock: MockUserDefinedFunctionServiceImpl, + allow_create: bool, + allow_destructive: bool, +) -> (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(UserDefinedFunctionServiceServer::new(mock)) + .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) + .await + .unwrap(); + }); + + ( + SiftMcpServer::new( + channel, + String::from("https://app.test.local"), + allow_create, + allow_destructive, + ), + handle, + ) +} + +fn numeric_input(identifier: &str) -> FunctionInput { + FunctionInput { + identifier: identifier.into(), + data_type: FunctionDataType::Numeric.into(), + constant: false, + } +} + +fn version_list_params( + user_defined_function_id: Option<&str>, + name: Option<&str>, +) -> Parameters { + Parameters(UserDefinedFunctionVersionListParams { + user_defined_function_id: user_defined_function_id.map(str::to_string), + name: name.map(str::to_string), + filter: None, + order_by: None, + limit: None, + }) +} + +fn create_params(function_inputs_json: &str) -> Parameters { + Parameters(CreateUserDefinedFunctionParams { + name: "rms".into(), + expression: "sqrt(mean($x * $x))".into(), + function_inputs_json: function_inputs_json.into(), + description: None, + user_notes: None, + metadata: None, + }) +} + +fn update_params( + name: Option<&str>, + description: Option<&str>, + function_inputs_json: Option<&str>, +) -> Parameters { + Parameters(UpdateUserDefinedFunctionParams { + user_defined_function_id: "f1".into(), + name: name.map(str::to_string), + description: description.map(str::to_string), + expression: None, + function_inputs_json: function_inputs_json.map(str::to_string), + metadata: None, + }) +} + +fn archive_params(id: &str) -> Parameters { + Parameters(ArchiveUserDefinedFunctionParams { + user_defined_function_id: id.into(), + }) +} + +#[tokio::test] +async fn list_user_defined_functions_returns_structured_rows() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .withf(|req| req.get_ref().filter == "is_archived == false") + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: vec![UserDefinedFunction { + user_defined_function_id: "f1".into(), + name: "rms".into(), + version: 3, + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (server, _h) = server_with_mock(mock, false, false).await; + + let resp = server + .list_user_defined_functions(list_params("is_archived == false", None)) + .await + .expect("list_user_defined_functions failed"); + + let functions = structured_field(resp, "user_defined_functions"); + let functions = functions.as_array().expect("array"); + assert_eq!(functions.len(), 1); + assert_eq!(functions[0]["userDefinedFunctionId"], "f1"); + assert_eq!(functions[0]["name"], "rms"); +} + +#[tokio::test] +async fn list_user_defined_functions_propagates_grpc_error() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions() + .returning(|_| Err(Status::invalid_argument("bad filter"))); + + let (server, _h) = server_with_mock(mock, false, false).await; + + let err = server + .list_user_defined_functions(list_params("nope", None)) + .await + .expect_err("expected error"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("bad filter")); +} + +#[tokio::test] +async fn list_versions_returns_structured_rows() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .withf(|req| req.get_ref().user_defined_function_id == "f1") + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: vec![UserDefinedFunction { + user_defined_function_id: "f1".into(), + user_defined_function_version_id: "v2".into(), + version: 2, + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (server, _h) = server_with_mock(mock, false, false).await; + + let resp = server + .list_user_defined_function_versions(version_list_params(Some("f1"), None)) + .await + .expect("list_user_defined_function_versions failed"); + + let versions = structured_field(resp, "user_defined_function_versions"); + let versions = versions.as_array().expect("array"); + assert_eq!(versions.len(), 1); + assert_eq!(versions[0]["userDefinedFunctionVersionId"], "v2"); +} + +#[tokio::test] +async fn list_versions_rejects_both_identifiers() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, false, false).await; + + let err = server + .list_user_defined_function_versions(version_list_params(Some("f1"), Some("rms"))) + .await + .expect_err("expected mutually exclusive params to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("exactly one")); +} + +#[tokio::test] +async fn list_versions_rejects_missing_identifier() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, false, false).await; + + let err = server + .list_user_defined_function_versions(version_list_params(None, None)) + .await + .expect_err("expected missing identifier to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn create_blocked_without_allow_create() { + // No expectations on the mock: the gate must fire before any RPC. + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, false, false).await; + + let err = server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"data_type\":\"numeric\"}]", + )) + .await + .expect_err("expected create gate to reject the call"); + + assert_eq!(err.code, ErrorCode::INVALID_REQUEST); + assert!(err.message.contains("--allow-create")); + let data = err + .data + .expect("create gate should return remediation data"); + assert_eq!( + data["remediation_command"], + "sift-cli agent update --allow-create" + ); +} + +#[tokio::test] +async fn create_rejects_malformed_function_inputs_json() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .create_user_defined_function(create_params("{not json")) + .await + .expect_err("expected malformed JSON to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("`function_inputs_json`")); +} + +#[tokio::test] +async fn create_rejects_unknown_data_type() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"data_type\":\"complex\"}]", + )) + .await + .expect_err("expected unknown data_type to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("complex")); +} + +#[tokio::test] +async fn create_returns_the_new_function_and_next_step() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + req.name == "rms" + && req.function_inputs.len() == 1 + && req.function_inputs[0].identifier == "x" + && req.function_inputs[0].data_type == i32::from(FunctionDataType::Numeric) + && !req.function_inputs[0].constant + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(CreateUserDefinedFunctionResponse { + user_defined_function: Some(UserDefinedFunction { + user_defined_function_id: "f1".into(), + user_defined_function_version_id: "v1".into(), + version: 1, + name: req.name, + function_inputs: req.function_inputs, + ..Default::default() + }), + })) + }); + + let (server, _h) = server_with_mock(mock, true, false).await; + + let resp = server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"data_type\":\"numeric\",\"constant\":false}]", + )) + .await + .expect("create_user_defined_function failed"); + + let body = structured(resp); + assert_eq!(body["user_defined_function_id"], "f1"); + assert_eq!(body["user_defined_function"]["name"], "rms"); + assert!( + body["next_step"] + .as_str() + .expect("next_step") + .contains("f1") + ); +} + +#[tokio::test] +async fn update_blocked_without_allow_destructive() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, false, false).await; + + let err = server + .update_user_defined_function(update_params(None, Some("updated"), None)) + .await + .expect_err("expected destructive gate to reject the call"); + + assert_eq!(err.code, ErrorCode::INVALID_REQUEST); + assert!(err.message.contains("--allow-destructive")); + let data = err + .data + .expect("destructive gate should return remediation data"); + assert_eq!( + data["remediation_command"], + "sift-cli agent update --allow-destructive" + ); +} + +#[tokio::test] +async fn update_rejects_an_empty_mask() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, true).await; + + let err = server + .update_user_defined_function(update_params(None, None, None)) + .await + .expect_err("expected an empty update to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("at least one")); +} + +#[tokio::test] +async fn update_rejects_name_combined_with_other_fields() { + // The API applies a name change on its own and ignores the rest, so a mixed + // request would silently drop half the caller's intent. + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, true).await; + + let err = server + .update_user_defined_function(update_params(Some("rms_v2"), Some("updated"), None)) + .await + .expect_err("expected a mixed name update to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("`name`")); +} + +#[tokio::test] +async fn update_rejects_malformed_function_inputs_json() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, true).await; + + let err = server + .update_user_defined_function(update_params(None, None, Some("[{\"identifier\":}]"))) + .await + .expect_err("expected malformed JSON to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("`function_inputs_json`")); +} + +#[tokio::test] +async fn update_returns_the_new_version() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let mask = req.get_ref().update_mask.as_ref().expect("mask present"); + mask.paths == vec!["description".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + let mut function = req.user_defined_function.expect("function present"); + function.version = 4; + function.user_defined_function_version_id = "v4".into(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: Some(function), + })) + }); + + let (server, _h) = server_with_mock(mock, true, true).await; + + let resp = server + .update_user_defined_function(update_params(None, Some("updated"), None)) + .await + .expect("update_user_defined_function failed"); + + let body = structured(resp); + assert_eq!(body["user_defined_function"]["version"], 4); + assert_eq!( + body["user_defined_function"]["userDefinedFunctionVersionId"], + "v4" + ); + assert!(body["next_step"].as_str().expect("next_step").contains("4")); +} + +#[tokio::test] +async fn archive_blocked_without_allow_destructive() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .archive_user_defined_function(archive_params("f1")) + .await + .expect_err("expected destructive gate to reject the call"); + + assert_eq!(err.code, ErrorCode::INVALID_REQUEST); + assert!(err.message.contains("--allow-destructive")); +} + +#[tokio::test] +async fn unarchive_blocked_without_allow_destructive() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .unarchive_user_defined_function(archive_params("f1")) + .await + .expect_err("expected destructive gate to reject the call"); + + assert_eq!(err.code, ErrorCode::INVALID_REQUEST); + assert!(err.message.contains("--allow-destructive")); +} + +#[tokio::test] +async fn archive_rejects_an_empty_id() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, true).await; + + let err = server + .archive_user_defined_function(archive_params("")) + .await + .expect_err("expected an empty id to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); +} + +#[tokio::test] +async fn archive_sets_the_archive_flag_and_reports_it() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + function.is_archived && mask.paths == vec!["is_archived".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (server, _h) = server_with_mock(mock, true, true).await; + + let resp = server + .archive_user_defined_function(archive_params("f1")) + .await + .expect("archive_user_defined_function failed"); + + let body = structured(resp); + assert_eq!(body["archived"], true); + assert_eq!(body["user_defined_function"]["isArchived"], true); +} + +#[tokio::test] +async fn unarchive_clears_the_archive_flag_and_reports_it() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let req = req.get_ref(); + let function = req + .user_defined_function + .as_ref() + .expect("function present"); + let mask = req.update_mask.as_ref().expect("mask present"); + !function.is_archived && mask.paths == vec!["is_archived".to_string()] + }) + .returning(|req| { + let req = req.into_inner(); + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.user_defined_function, + })) + }); + + let (server, _h) = server_with_mock(mock, true, true).await; + + let resp = server + .unarchive_user_defined_function(archive_params("f2")) + .await + .expect("unarchive_user_defined_function failed"); + + let body = structured(resp); + assert_eq!(body["unarchived"], true); +} + +#[tokio::test] +async fn create_maps_every_documented_data_type() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_create_user_defined_function() + .times(1) + .withf(|req| { + let inputs = &req.get_ref().function_inputs; + inputs + == &vec![ + numeric_input("x"), + FunctionInput { + identifier: "label".into(), + data_type: FunctionDataType::String.into(), + constant: true, + }, + FunctionInput { + identifier: "flag".into(), + data_type: FunctionDataType::Bool.into(), + constant: false, + }, + ] + }) + .returning(|_| { + Ok(Response::new(CreateUserDefinedFunctionResponse { + user_defined_function: Some(UserDefinedFunction { + user_defined_function_id: "f1".into(), + ..Default::default() + }), + })) + }); + + let (server, _h) = server_with_mock(mock, true, false).await; + + server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"data_type\":\"numeric\"},\ + {\"identifier\":\"label\",\"data_type\":\"STRING\",\"constant\":true},\ + {\"identifier\":\"flag\",\"data_type\":\"bool\",\"constant\":false}]", + )) + .await + .expect("create_user_defined_function failed"); +} diff --git a/rust/crates/sift_mcp/src/tool_events.json b/rust/crates/sift_mcp/src/tool_events.json index 0706bf055c..91334fea04 100644 --- a/rust/crates/sift_mcp/src/tool_events.json +++ b/rust/crates/sift_mcp/src/tool_events.json @@ -2,6 +2,7 @@ "append_test_measurements": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_APPEND_TEST_MEASUREMENTS", "archive_calculated_channel": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_ARCHIVE_CALCULATED_CHANNEL", "archive_rule": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_ARCHIVE_RULE", + "archive_user_defined_function": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_ARCHIVE_USER_DEFINED_FUNCTION", "check_for_updates": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_CHECK_FOR_UPDATES", "count_test_measurements": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_COUNT_TEST_MEASUREMENTS", "count_test_steps": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_COUNT_TEST_STEPS", @@ -11,6 +12,7 @@ "create_report_template": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_CREATE_REPORT_TEMPLATE", "create_rule": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_CREATE_RULE", "create_test_report": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_CREATE_TEST_REPORT", + "create_user_defined_function": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_CREATE_USER_DEFINED_FUNCTION", "explore_url": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_EXPLORE_URL", "get_data": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_GET_DATA", "list_annotations": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_ANNOTATIONS", @@ -27,6 +29,8 @@ "list_test_measurements": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_TEST_MEASUREMENTS", "list_test_reports": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_TEST_REPORTS", "list_test_steps": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_TEST_STEPS", + "list_user_defined_function_versions": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_USER_DEFINED_FUNCTION_VERSIONS", + "list_user_defined_functions": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_LIST_USER_DEFINED_FUNCTIONS", "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", @@ -34,6 +38,7 @@ "sql": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_SQL", "unarchive_calculated_channel": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UNARCHIVE_CALCULATED_CHANNEL", "unarchive_rule": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UNARCHIVE_RULE", + "unarchive_user_defined_function": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UNARCHIVE_USER_DEFINED_FUNCTION", "update_annotation": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_ANNOTATION", "update_asset": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_ASSET", "update_calculated_channel": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_CALCULATED_CHANNEL", @@ -41,5 +46,6 @@ "update_report_template": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_REPORT_TEMPLATE", "update_rule": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_RULE", "update_run": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_RUN", + "update_user_defined_function": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPDATE_USER_DEFINED_FUNCTION", "upload_dataset": "CLIENT_EVENT_USER_CALLED_MCP_TOOL_UPLOAD_DATASET" } From 0da64eb785396a12fb1f5e851cd9d6d685148165 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Tue, 18 Aug 2026 22:44:13 -0700 Subject: [PATCH 3/8] refactor: tighten user defined function tool params and descriptions --- rust/crates/sift_mcp/src/server/mod.rs | 3 +- .../src/tool/user_defined_functions/mod.rs | 25 +++--- .../src/tool/user_defined_functions/test.rs | 76 ++++++++++++++++++- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/rust/crates/sift_mcp/src/server/mod.rs b/rust/crates/sift_mcp/src/server/mod.rs index 4a7ebed0a3..16a2e45a99 100644 --- a/rust/crates/sift_mcp/src/server/mod.rs +++ b/rust/crates/sift_mcp/src/server/mod.rs @@ -44,8 +44,7 @@ use crate::service::{ docs::DocsService, ingest::IngestService, ping::PingService, report_templates::ReportTemplateService, reports::ReportService, rule_evaluation::RuleEvaluationService, rules::RuleService, runs::RunService, url::UrlService, - user_defined_functions::UserDefinedFunctionService, - users::UserService, + user_defined_functions::UserDefinedFunctionService, users::UserService, }; #[derive(Clone)] 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 05deb8d779..c11b6e864a 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 @@ -25,7 +25,7 @@ mod test; pub struct UserDefinedFunctionVersionListParams { pub(crate) user_defined_function_id: Option, pub(crate) name: Option, - pub(crate) filter: Option, + pub(crate) filter: String, pub(crate) order_by: Option, pub(crate) limit: Option, } @@ -61,7 +61,6 @@ pub struct ArchiveUserDefinedFunctionParams { #[derive(Debug, Deserialize)] struct FunctionInputSpec { identifier: String, - #[serde(alias = "dataType")] data_type: String, #[serde(default)] constant: bool, @@ -154,7 +153,7 @@ impl SiftMcpServer { - `user_defined_function_id`: optional. The id of the function whose versions to list. - `name`: optional. The name of the function whose versions to list. - Exactly one of `user_defined_function_id` or `name` must be set. - - `filter`: optional CEL expression. Omit or pass an empty string to list every version. + - `filter`: CEL expression. Pass an empty string to list every version. Filterable fields: `user_defined_function_id`, `name`, `version`, `archived_date`, `is_archived`. `name` is the only free-text field; when filtering or searching it, use `name.matches(\"(?i)rms\")`, not `==`. Use `==` only for an exact value from a prior @@ -200,7 +199,7 @@ impl SiftMcpServer { .list_user_defined_function_versions( user_defined_function_id, name, - filter.unwrap_or_default(), + filter, order_by, limit, ) @@ -231,8 +230,9 @@ impl SiftMcpServer { - `function_inputs_json`: required. A JSON array string declaring the function's inputs, in the order callers pass them. Each element is `{ \"identifier\": \"\", \"data_type\": \"numeric\"|\"string\"|\"bool\", - \"constant\": }`. `constant` defaults to `false` and marks an input that takes a - literal value rather than a channel. Pass `[]` for a function that takes no inputs. + \"constant\": }`. `data_type` is matched case-insensitively; no other spelling is + accepted. `constant` defaults to `false` and marks an input that takes a literal value + rather than a channel. Pass `[]` for a function that takes no inputs. - `description`: optional. Human-readable summary. - `user_notes`: optional. Notes recorded against this first version. - `metadata`: optional. Array of `{ \"name\": \"\", \"value\": }` where @@ -466,8 +466,11 @@ impl SiftMcpServer { Guidance: - Archiving does not delete the function and does not rewrite anything that already calls - it. Confirm the target with the user before calling, and check for dependents first — - calculated channels and rules that call the function keep referencing it. + it. Calculated channels and rules that call it keep referencing it. Confirm the target + with the user before calling. + - This toolset cannot enumerate what depends on a function. `function_dependencies` on a + `list_user_defined_functions` row is the reverse direction — the functions this one + calls. Treat the archive as reversible and let the user tell you what else is affected. ", annotations( title = "user_defined_functions/archive_user_defined_function", @@ -636,9 +639,9 @@ fn parse_function_inputs(function_inputs_json: &str) -> Result Result { match data_type.to_ascii_lowercase().as_str() { - "numeric" | "function_data_type_numeric" => Ok(FunctionDataType::Numeric), - "string" | "function_data_type_string" => Ok(FunctionDataType::String), - "bool" | "boolean" | "function_data_type_bool" => Ok(FunctionDataType::Bool), + "numeric" => Ok(FunctionDataType::Numeric), + "string" => Ok(FunctionDataType::String), + "bool" => Ok(FunctionDataType::Bool), other => Err(ErrorData::invalid_params( format!("unknown `data_type` `{other}`; expected `numeric`, `string`, or `bool`"), None, diff --git a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs index 9a7b6b09d6..b83a26f86f 100644 --- a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs @@ -66,7 +66,7 @@ fn version_list_params( Parameters(UserDefinedFunctionVersionListParams { user_defined_function_id: user_defined_function_id.map(str::to_string), name: name.map(str::to_string), - filter: None, + filter: String::new(), order_by: None, limit: None, }) @@ -156,7 +156,11 @@ async fn list_user_defined_functions_propagates_grpc_error() { async fn list_versions_returns_structured_rows() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); mock.expect_list_user_defined_function_versions() - .withf(|req| req.get_ref().user_defined_function_id == "f1") + // An empty `filter` lists every version; it is a required String, not an Option. + .withf(|req| { + let req = req.get_ref(); + req.user_defined_function_id == "f1" && req.filter.is_empty() + }) .returning(|_| { Ok(Response::new(ListUserDefinedFunctionVersionsResponse { user_defined_functions: vec![UserDefinedFunction { @@ -182,6 +186,36 @@ async fn list_versions_returns_structured_rows() { assert_eq!(versions[0]["userDefinedFunctionVersionId"], "v2"); } +#[tokio::test] +async fn list_versions_forwards_the_required_filter() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .times(1) + .withf(|req| req.get_ref().filter == "version == 2") + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: vec![UserDefinedFunction { + version: 2, + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (server, _h) = server_with_mock(mock, false, false).await; + + server + .list_user_defined_function_versions(Parameters(UserDefinedFunctionVersionListParams { + user_defined_function_id: Some("f1".into()), + name: None, + filter: "version == 2".into(), + order_by: None, + limit: None, + })) + .await + .expect("list_user_defined_function_versions failed"); +} + #[tokio::test] async fn list_versions_rejects_both_identifiers() { let mock = MockUserDefinedFunctionServiceImpl::new(); @@ -516,6 +550,44 @@ async fn unarchive_clears_the_archive_flag_and_reports_it() { assert_eq!(body["unarchived"], true); } +#[tokio::test] +async fn create_rejects_the_proto_enum_spelling_of_data_type() { + // The description documents `numeric`, `string`, and `bool` only. Accepting + // the raw proto enum names as well would be undocumented behavior. + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"data_type\":\"FUNCTION_DATA_TYPE_NUMERIC\"}]", + )) + .await + .expect_err("expected the proto enum spelling to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!( + err.message + .contains("expected `numeric`, `string`, or `bool`") + ); +} + +#[tokio::test] +async fn create_rejects_a_camel_case_data_type_key() { + // Only the documented `data_type` key is accepted. + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + + let err = server + .create_user_defined_function(create_params( + "[{\"identifier\":\"x\",\"dataType\":\"numeric\"}]", + )) + .await + .expect_err("expected a camelCase key to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("`function_inputs_json`")); +} + #[tokio::test] async fn create_maps_every_documented_data_type() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); From a6704bf148e75732e990524a29daf700b8c043c8 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 00:45:07 -0700 Subject: [PATCH 4/8] feat: add fields projection and item count to user defined function list tools --- .../src/service/user_defined_functions/mod.rs | 32 +++++-- .../service/user_defined_functions/test.rs | 21 +++-- .../src/tool/user_defined_functions/mod.rs | 59 +++++++++++-- .../src/tool/user_defined_functions/test.rs | 85 +++++++++++++++++-- 4 files changed, 170 insertions(+), 27 deletions(-) diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs index db59a3c270..af1d6b0b39 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs @@ -48,11 +48,12 @@ impl UserDefinedFunctionService { filter: String, order_by: Option, limit: Option, - ) -> Result> { + ) -> Result> { let (page_size, record_limit) = common::paging(limit); let mut page_token = String::new(); let mut results = Vec::new(); + let mut has_more = false; let order_by = order_by.unwrap_or_default(); @@ -92,7 +93,13 @@ impl UserDefinedFunctionService { } results.extend(user_defined_functions); - if results.len() >= record_limit || next_page_token.is_empty() { + if results.len() >= record_limit { + // The cap, not the end of the data: report that more exist so the + // caller does not read this page's size as the match total. + has_more = results.len() > record_limit || !next_page_token.is_empty(); + break; + } + if next_page_token.is_empty() { break; } page_token = next_page_token; @@ -100,7 +107,10 @@ impl UserDefinedFunctionService { results.truncate(record_limit); - Ok(results) + Ok(common::Page { + items: results, + has_more, + }) } /// Lists the version history of one user defined function. The caller @@ -113,11 +123,12 @@ impl UserDefinedFunctionService { filter: String, order_by: Option, limit: Option, - ) -> Result> { + ) -> Result> { let (page_size, record_limit) = common::paging(limit); let mut page_token = String::new(); let mut results = Vec::new(); + let mut has_more = false; let order_by = order_by.unwrap_or_default(); @@ -165,7 +176,13 @@ impl UserDefinedFunctionService { } results.extend(user_defined_functions); - if results.len() >= record_limit || next_page_token.is_empty() { + if results.len() >= record_limit { + // The cap, not the end of the data: report that more exist so the + // caller does not read this page's size as the match total. + has_more = results.len() > record_limit || !next_page_token.is_empty(); + break; + } + if next_page_token.is_empty() { break; } page_token = next_page_token; @@ -173,7 +190,10 @@ impl UserDefinedFunctionService { results.truncate(record_limit); - Ok(results) + Ok(common::Page { + items: results, + has_more, + }) } /// Creates a user defined function at version 1 and returns it. diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs index 0e3e07b41e..e70f0ba113 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs @@ -95,7 +95,8 @@ async fn list_user_defined_functions_returns_single_page() { None, ) .await - .expect("list_user_defined_functions failed"); + .expect("list_user_defined_functions failed") + .items; assert_eq!(functions.len(), 2); assert_eq!(functions[0].user_defined_function_id, "f1"); @@ -125,7 +126,8 @@ async fn list_user_defined_functions_paginates_until_token_empty() { let functions = service .list_user_defined_functions(String::new(), None, None) .await - .expect("list_user_defined_functions failed"); + .expect("list_user_defined_functions failed") + .items; let ids: Vec<&str> = functions .iter() @@ -156,7 +158,8 @@ async fn list_user_defined_functions_truncates_to_limit_across_pages() { let functions = service .list_user_defined_functions(String::new(), None, Some(3)) .await - .expect("list_user_defined_functions failed"); + .expect("list_user_defined_functions failed") + .items; let ids: Vec<&str> = functions .iter() @@ -203,7 +206,8 @@ async fn list_user_defined_functions_breaks_on_empty_page() { let functions = service .list_user_defined_functions(String::new(), None, None) .await - .expect("list_user_defined_functions failed"); + .expect("list_user_defined_functions failed") + .items; assert!(functions.is_empty()); } @@ -264,7 +268,8 @@ async fn list_versions_sends_id_filter_and_order_by() { None, ) .await - .expect("list_user_defined_function_versions failed"); + .expect("list_user_defined_function_versions failed") + .items; assert_eq!(versions.len(), 1); assert_eq!(versions[0].version, 2); @@ -297,7 +302,8 @@ async fn list_versions_sends_name_when_id_is_empty() { None, ) .await - .expect("list_user_defined_function_versions failed"); + .expect("list_user_defined_function_versions failed") + .items; } #[tokio::test] @@ -347,7 +353,8 @@ async fn list_versions_paginates_and_truncates_to_limit() { Some(2), ) .await - .expect("list_user_defined_function_versions failed"); + .expect("list_user_defined_function_versions failed") + .items; let numbers: Vec = versions.iter().map(|v| v.version).collect(); assert_eq!(numbers, vec![1, 2]); 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 c11b6e864a..2f4b2bbeaa 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}, + tool::common::{ListParams, MetadataEntry, list_body, to_values}, }; #[cfg(test)] @@ -28,6 +28,7 @@ pub struct UserDefinedFunctionVersionListParams { pub(crate) filter: String, pub(crate) order_by: Option, pub(crate) limit: Option, + pub(crate) fields: Option>, } #[derive(Debug, Deserialize, JsonSchema)] @@ -84,6 +85,12 @@ impl SiftMcpServer { state. - Fields at their proto3 default are OMITTED from the JSON: a missing `is_archived` or `version` key means `false` / `0`, not \"unknown\". + - `count`: how many items THIS response carries — read it instead of + counting the array yourself. It is the size of the page you got back, not + how many items match `filter`. + - `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. Parameters: - `filter`: CEL expression. Pass an empty string to list everything. Filterable fields: @@ -97,6 +104,14 @@ impl SiftMcpServer { (newest first). Example: `\"name,modified_date desc\"`. - `limit`: max items to return. Start at 50 and only raise it if the result is capped and you still need more. Values are clamped to `1..=200`; omitting it defaults to 50. + - `fields`: optional array of field names to keep on each item, e.g. + `[\"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 + 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. Errors: - `INVALID_PARAMS` if `filter` is not a valid CEL expression or `order_by` references an @@ -123,17 +138,23 @@ impl SiftMcpServer { filter, order_by, limit, + fields, }) = params; - let functions = self + let page = self .user_defined_function_service .list_user_defined_functions(filter, order_by, limit) .await .map_err(from_anyhow)?; - Ok(CallToolResult::structured( - serde_json::json!({ "user_defined_functions": functions }), - )) + let functions = to_values(&page.items)?; + + Ok(CallToolResult::structured(list_body( + "user_defined_functions", + functions, + fields, + page.has_more, + ))) } #[tool( @@ -148,6 +169,12 @@ impl SiftMcpServer { `user_defined_function_version_id`, `version`, `expression`, `function_inputs`, `change_message` (server-generated summary of the change), `user_notes`, and `modified_by_user_id`. + - `count`: how many items THIS response carries — read it instead of + counting the array yourself. It is the size of the page you got back, not + how many items match `filter`. + - `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. Parameters: - `user_defined_function_id`: optional. The id of the function whose versions to list. @@ -164,6 +191,14 @@ impl SiftMcpServer { by `name` ascending — pass `\"version desc\"` for newest-first. - `limit`: max items to return. Start at 50 and only raise it if the result is capped and you still need more. Values are clamped to `1..=200`; omitting it defaults to 50. + - `fields`: optional array of field names to keep on each item, e.g. + `[\"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 + 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. Errors: - `INVALID_PARAMS` if neither or both of `user_defined_function_id` and `name` are set, or @@ -190,11 +225,12 @@ impl SiftMcpServer { filter, order_by, limit, + fields, }) = params; let (user_defined_function_id, name) = function_identifier(user_defined_function_id, name)?; - let versions = self + let page = self .user_defined_function_service .list_user_defined_function_versions( user_defined_function_id, @@ -206,9 +242,14 @@ impl SiftMcpServer { .await .map_err(from_anyhow)?; - Ok(CallToolResult::structured( - serde_json::json!({ "user_defined_function_versions": versions }), - )) + let versions = to_values(&page.items)?; + + Ok(CallToolResult::structured(list_body( + "user_defined_function_versions", + versions, + fields, + page.has_more, + ))) } #[tool( diff --git a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs index b83a26f86f..92bc85068f 100644 --- a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs @@ -16,7 +16,7 @@ use tonic::{Response, Status, transport::Server}; use crate::{ server::SiftMcpServer, tool::{ - common::test_support::{list_params, structured, structured_field}, + common::test_support::{list_params, list_params_with_fields, structured}, user_defined_functions::{ ArchiveUserDefinedFunctionParams, CreateUserDefinedFunctionParams, UpdateUserDefinedFunctionParams, UserDefinedFunctionVersionListParams, @@ -69,6 +69,7 @@ fn version_list_params( filter: String::new(), order_by: None, limit: None, + fields: None, }) } @@ -128,11 +129,12 @@ async fn list_user_defined_functions_returns_structured_rows() { .await .expect("list_user_defined_functions failed"); - let functions = structured_field(resp, "user_defined_functions"); - let functions = functions.as_array().expect("array"); + let body = structured(resp); + let functions = body["user_defined_functions"].as_array().expect("array"); assert_eq!(functions.len(), 1); assert_eq!(functions[0]["userDefinedFunctionId"], "f1"); assert_eq!(functions[0]["name"], "rms"); + assert_eq!(body["count"], 1); } #[tokio::test] @@ -152,6 +154,36 @@ async fn list_user_defined_functions_propagates_grpc_error() { assert!(err.message.contains("bad filter")); } +#[tokio::test] +async fn list_user_defined_functions_projects_requested_fields() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_functions().returning(|_| { + Ok(Response::new(ListUserDefinedFunctionsResponse { + user_defined_functions: vec![UserDefinedFunction { + user_defined_function_id: "f1".into(), + name: "rms".into(), + description: "root mean square".into(), + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (server, _h) = server_with_mock(mock, false, false).await; + + let resp = server + .list_user_defined_functions(list_params_with_fields("", &["name"])) + .await + .expect("list_user_defined_functions failed"); + + let body = structured(resp); + assert_eq!( + body["user_defined_functions"], + serde_json::json!([{ "name": "rms" }]) + ); + assert_eq!(body["count"], 1); +} + #[tokio::test] async fn list_versions_returns_structured_rows() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); @@ -180,10 +212,13 @@ async fn list_versions_returns_structured_rows() { .await .expect("list_user_defined_function_versions failed"); - let versions = structured_field(resp, "user_defined_function_versions"); - let versions = versions.as_array().expect("array"); + let body = structured(resp); + let versions = body["user_defined_function_versions"] + .as_array() + .expect("array"); assert_eq!(versions.len(), 1); assert_eq!(versions[0]["userDefinedFunctionVersionId"], "v2"); + assert_eq!(body["count"], 1); } #[tokio::test] @@ -211,11 +246,51 @@ async fn list_versions_forwards_the_required_filter() { filter: "version == 2".into(), order_by: None, limit: None, + fields: None, })) .await .expect("list_user_defined_function_versions failed"); } +#[tokio::test] +async fn list_versions_projects_requested_fields() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + mock.expect_list_user_defined_function_versions() + .returning(|_| { + Ok(Response::new(ListUserDefinedFunctionVersionsResponse { + user_defined_functions: vec![UserDefinedFunction { + user_defined_function_id: "f1".into(), + user_defined_function_version_id: "v2".into(), + version: 2, + name: "rms".into(), + ..Default::default() + }], + next_page_token: String::new(), + })) + }); + + let (server, _h) = server_with_mock(mock, false, false).await; + + let resp = server + .list_user_defined_function_versions(Parameters(UserDefinedFunctionVersionListParams { + user_defined_function_id: Some("f1".into()), + name: None, + filter: String::new(), + order_by: None, + limit: None, + fields: Some(vec!["version".into()]), + })) + .await + .expect("list_user_defined_function_versions failed"); + + let body = structured(resp); + assert_eq!( + body["user_defined_function_versions"], + serde_json::json!([{ "version": 2 }]) + ); + assert_eq!(body["count"], 1); +} + #[tokio::test] async fn list_versions_rejects_both_identifiers() { let mock = MockUserDefinedFunctionServiceImpl::new(); From a4c6cd9af682c7505f62e562cbf0b0eef5ab4f89 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 01:12:42 -0700 Subject: [PATCH 5/8] test: drop dangling items expression in UDF versions test --- .../crates/sift_mcp/src/service/user_defined_functions/test.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs index e70f0ba113..976d225605 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs @@ -302,8 +302,7 @@ async fn list_versions_sends_name_when_id_is_empty() { None, ) .await - .expect("list_user_defined_function_versions failed") - .items; + .expect("list_user_defined_function_versions failed"); } #[tokio::test] From 45ae46be9dfa74ba96b26849656c996345f1b06f Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 10:13:31 -0700 Subject: [PATCH 6/8] docs: state the user defined function name constraints in the tool descriptions --- rust/crates/sift_mcp/src/tool/user_defined_functions/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 2f4b2bbeaa..aaa6080fb1 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 @@ -264,7 +264,8 @@ impl SiftMcpServer { including the resolved `function_output_type` and `version` 1. Parameters: - - `name`: required. The name callers use to reference the function. Must be non-empty. + - `name`: required. The name callers use to reference the function. Must start and end + with a letter character and may contain only alphanumeric characters or `_`. - `expression`: required. The function body. Reference each declared input by its `identifier`. Mirror an existing function retrieved with `list_user_defined_functions` rather than authoring the syntax blind. @@ -373,7 +374,8 @@ impl SiftMcpServer { Parameters: - `user_defined_function_id`: required. The function to update. - - `name`: optional. New name. The API applies a rename BY ITSELF and ignores any other + - `name`: optional. New name. Must start and end with a letter character and may contain + only alphanumeric characters or `_`. The API applies a rename BY ITSELF and ignores any other field in the same call, so this tool rejects `name` combined with another field — send the rename as its own call. - `description`: optional. New description. From 1c714d8b201f4902badff77203e7a7a3a02781ae Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 21:39:22 -0700 Subject: [PATCH 7/8] fix: preserve user notes on user defined function updates and validate names like the backend --- .../src/service/user_defined_functions/mod.rs | 44 ++++++++- .../service/user_defined_functions/test.rs | 37 ++++++- .../src/tool/user_defined_functions/mod.rs | 38 ++++++-- .../src/tool/user_defined_functions/test.rs | 96 ++++++++++++++++++- 4 files changed, 200 insertions(+), 15 deletions(-) diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs index af1d6b0b39..728a53a337 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/mod.rs @@ -7,9 +7,10 @@ use sift_rs::{ common::r#type::v1::{FunctionInput, UserDefinedFunction}, metadata::v1::MetadataValue, user_defined_functions::v1::{ - CreateUserDefinedFunctionRequest, ListUserDefinedFunctionVersionsRequest, - ListUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionsRequest, - ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionRequest, + CreateUserDefinedFunctionRequest, GetUserDefinedFunctionRequest, + ListUserDefinedFunctionVersionsRequest, ListUserDefinedFunctionVersionsResponse, + ListUserDefinedFunctionsRequest, ListUserDefinedFunctionsResponse, + UpdateUserDefinedFunctionRequest, user_defined_function_service_client::UserDefinedFunctionServiceClient, }, }; @@ -253,8 +254,13 @@ impl UserDefinedFunctionService { user_defined_function_id: String, changes: UdfUpdate, ) -> Result { + let user_notes = self + .get_user_defined_function(user_defined_function_id.clone()) + .await? + .user_notes; let mut function = UserDefinedFunction { user_defined_function_id, + user_notes, ..Default::default() }; let mut paths = Vec::new(); @@ -298,9 +304,14 @@ impl UserDefinedFunctionService { user_defined_function_id: String, is_archived: bool, ) -> Result { + let user_notes = self + .get_user_defined_function(user_defined_function_id.clone()) + .await? + .user_notes; let function = UserDefinedFunction { user_defined_function_id, is_archived, + user_notes, ..Default::default() }; @@ -308,6 +319,33 @@ impl UserDefinedFunctionService { .await } + async fn get_user_defined_function( + &self, + user_defined_function_id: String, + ) -> Result { + let channel = self.channel.clone(); + let resp = with_retry(&self.policy, move || { + let channel = channel.clone(); + let user_defined_function_id = user_defined_function_id.clone(); + async move { + let mut client = UserDefinedFunctionServiceClient::new(channel); + client + .get_user_defined_function(GetUserDefinedFunctionRequest { + user_defined_function_id, + name: String::new(), + }) + .await + .map(|resp| resp.into_inner()) + } + }) + .await + .context("failed to fetch user defined function")?; + + resp.user_defined_function.ok_or_else(|| { + anyhow!("get_user_defined_function response missing user defined function") + }) + } + async fn send_update( &self, function: UserDefinedFunction, diff --git a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs index 976d225605..9d26d16fb8 100644 --- a/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/service/user_defined_functions/test.rs @@ -4,8 +4,9 @@ use sift_rs::{ MetadataKey, MetadataKeyType, MetadataValue, metadata_value::Value as MetadataValueInner, }, user_defined_functions::v1::{ - CreateUserDefinedFunctionResponse, ListUserDefinedFunctionVersionsResponse, - ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionResponse, + CreateUserDefinedFunctionResponse, GetUserDefinedFunctionResponse, + ListUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionsResponse, + UpdateUserDefinedFunctionResponse, user_defined_function_service_server::UserDefinedFunctionServiceServer, }, }; @@ -47,6 +48,27 @@ fn udf(id: &str, name: &str) -> UserDefinedFunction { } } +fn expect_current_udf(mock: &mut MockUserDefinedFunctionServiceImpl, id: &str, user_notes: &str) { + let expected_id = id.to_string(); + let response_id = id.to_string(); + let user_notes = user_notes.to_string(); + mock.expect_get_user_defined_function() + .times(1) + .withf(move |req| { + let req = req.get_ref(); + req.user_defined_function_id == expected_id && req.name.is_empty() + }) + .returning(move |_| { + Ok(Response::new(GetUserDefinedFunctionResponse { + user_defined_function: Some(UserDefinedFunction { + user_defined_function_id: response_id.clone(), + user_notes: user_notes.clone(), + ..Default::default() + }), + })) + }); +} + async fn service_with_mock( mock: MockUserDefinedFunctionServiceImpl, ) -> (UserDefinedFunctionService, JoinHandle<()>) { @@ -521,6 +543,7 @@ async fn create_propagates_grpc_error() { #[tokio::test] async fn update_masks_only_the_provided_fields() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", "existing notes"); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -532,6 +555,7 @@ async fn update_masks_only_the_provided_fields() { let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f1" && function.description == "updated" + && function.user_notes == "existing notes" && function.expression.is_empty() && function.function_inputs.is_empty() && function.metadata.is_empty() @@ -563,6 +587,7 @@ async fn update_masks_only_the_provided_fields() { #[tokio::test] async fn update_masks_every_provided_field_in_declaration_order() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", ""); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -617,6 +642,7 @@ async fn update_sends_no_version_precondition() { // carries no stale version identifiers that could be mistaken for one, and // that the caller sees the new version from the response. let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", ""); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -661,6 +687,7 @@ async fn update_with_no_fields_sends_an_empty_mask() { // The tool handler rejects this case; the service contract is to send // exactly what it was given. let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", ""); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -690,6 +717,7 @@ async fn update_with_no_fields_sends_an_empty_mask() { #[tokio::test] async fn update_errors_when_response_missing_function() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", ""); mock.expect_update_user_defined_function().returning(|_| { Ok(Response::new(UpdateUserDefinedFunctionResponse { user_defined_function: None, @@ -718,6 +746,7 @@ async fn update_errors_when_response_missing_function() { #[tokio::test] async fn update_propagates_grpc_error() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", ""); mock.expect_update_user_defined_function() .returning(|_| Err(Status::failed_precondition("function has dependents"))); @@ -743,6 +772,7 @@ async fn update_propagates_grpc_error() { #[tokio::test] async fn archive_sets_is_archived_true_through_the_mask() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1", "archived notes"); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -754,6 +784,7 @@ async fn archive_sets_is_archived_true_through_the_mask() { let mask = req.update_mask.as_ref().expect("mask present"); function.user_defined_function_id == "f1" && function.is_archived + && function.user_notes == "archived notes" && function.archived_date.is_none() && mask.paths == vec!["is_archived".to_string()] }) @@ -777,6 +808,7 @@ async fn archive_sets_is_archived_true_through_the_mask() { #[tokio::test] async fn unarchive_sets_is_archived_false_through_the_mask() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f2", ""); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -810,6 +842,7 @@ async fn unarchive_sets_is_archived_false_through_the_mask() { #[tokio::test] async fn set_archived_propagates_grpc_error() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "missing", ""); mock.expect_update_user_defined_function() .returning(|_| Err(Status::not_found("function missing"))); 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 aaa6080fb1..789f453c20 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 @@ -187,8 +187,8 @@ impl SiftMcpServer { result. `contains`/`startsWith`/`endsWith` are case-SENSITIVE: `contains(\"RMS\")` silently misses `rms_window`. - `order_by`: optional comma-separated `FIELD_NAME[ desc]` list. Orderable fields: - `created_date`, `modified_date`, `name`, `version`. When empty, items come back ordered - by `name` ascending — pass `\"version desc\"` for newest-first. + `created_date`, `modified_date`, `version`. Default sort is `version desc` + (newest version first). Example: `\"version desc,modified_date\"`. - `limit`: max items to return. Start at 50 and only raise it if the result is capped and you still need more. Values are clamped to `1..=200`; omitting it defaults to 50. - `fields`: optional array of field names to keep on each item, e.g. @@ -264,8 +264,8 @@ impl SiftMcpServer { including the resolved `function_output_type` and `version` 1. Parameters: - - `name`: required. The name callers use to reference the function. Must start and end - with a letter character and may contain only alphanumeric characters or `_`. + - `name`: required. The name callers use to reference the function. It must start with a + letter, then contain only alphanumeric characters or `_`, and be at most 253 characters. - `expression`: required. The function body. Reference each declared input by its `identifier`. Mirror an existing function retrieved with `list_user_defined_functions` rather than authoring the syntax blind. @@ -321,6 +321,7 @@ impl SiftMcpServer { if name.trim().is_empty() { return Err(ErrorData::invalid_params("`name` must not be empty", None)); } + validate_function_name(&name)?; if expression.trim().is_empty() { return Err(ErrorData::invalid_params( "`expression` must not be empty", @@ -374,10 +375,10 @@ impl SiftMcpServer { Parameters: - `user_defined_function_id`: required. The function to update. - - `name`: optional. New name. Must start and end with a letter character and may contain - only alphanumeric characters or `_`. The API applies a rename BY ITSELF and ignores any other - field in the same call, so this tool rejects `name` combined with another field — send - the rename as its own call. + - `name`: optional. New name. It must start with a letter, then contain only alphanumeric + characters or `_`, and be at most 253 characters. The API applies a rename BY ITSELF and + ignores any other field in the same call, so this tool rejects `name` combined with another + field — send the rename as its own call. - `description`: optional. New description. - `expression`: optional. New function body. - `function_inputs_json`: optional. REPLACES the declared input list. Same array shape as @@ -434,6 +435,9 @@ impl SiftMcpServer { None, )); } + if let Some(name) = &name { + validate_function_name(name)?; + } let function_inputs = function_inputs_json .as_deref() @@ -648,6 +652,24 @@ fn function_identifier( } } +fn validate_function_name(name: &str) -> Result<(), ErrorData> { + let valid = name.len() <= 253 + && matches!(name.as_bytes().first(), Some(b'A'..=b'Z' | b'a'..=b'z')) + && name + .bytes() + .skip(1) + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_'); + + if valid { + Ok(()) + } else { + Err(ErrorData::invalid_params( + "`name` must start with a letter, contain only alphanumeric characters or `_`, and be at most 253 characters", + None, + )) + } +} + /// Parse the documented `function_inputs_json` array into proto inputs, mapping /// every shape error to `INVALID_PARAMS` so the agent can correct its input. fn parse_function_inputs(function_inputs_json: &str) -> Result, ErrorData> { diff --git a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs index 92bc85068f..75233353d8 100644 --- a/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs +++ b/rust/crates/sift_mcp/src/tool/user_defined_functions/test.rs @@ -2,8 +2,9 @@ use rmcp::{handler::server::wrapper::Parameters, model::ErrorCode}; use sift_rs::{ common::r#type::v1::{FunctionDataType, FunctionInput, UserDefinedFunction}, user_defined_functions::v1::{ - CreateUserDefinedFunctionResponse, ListUserDefinedFunctionVersionsResponse, - ListUserDefinedFunctionsResponse, UpdateUserDefinedFunctionResponse, + CreateUserDefinedFunctionResponse, GetUserDefinedFunctionResponse, + ListUserDefinedFunctionVersionsResponse, ListUserDefinedFunctionsResponse, + UpdateUserDefinedFunctionResponse, user_defined_function_service_server::UserDefinedFunctionServiceServer, }, }; @@ -105,6 +106,20 @@ fn archive_params(id: &str) -> Parameters { }) } +fn expect_current_udf(mock: &mut MockUserDefinedFunctionServiceImpl, id: &str) { + let id = id.to_string(); + mock.expect_get_user_defined_function() + .times(1) + .returning(move |_| { + Ok(Response::new(GetUserDefinedFunctionResponse { + user_defined_function: Some(UserDefinedFunction { + user_defined_function_id: id.clone(), + ..Default::default() + }), + })) + }); +} + #[tokio::test] async fn list_user_defined_functions_returns_structured_rows() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); @@ -372,6 +387,52 @@ async fn create_rejects_unknown_data_type() { assert!(err.message.contains("complex")); } +#[tokio::test] +async fn create_rejects_a_name_that_starts_with_a_digit() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + let mut params = create_params("[]"); + params.0.name = "1udf".into(); + + let err = server + .create_user_defined_function(params) + .await + .expect_err("expected invalid name to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("start with a letter")); +} + +#[tokio::test] +async fn update_rejects_a_name_with_a_hyphen() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, true).await; + + let err = server + .update_user_defined_function(update_params(Some("udf-name"), None, None)) + .await + .expect_err("expected invalid name to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("alphanumeric characters")); +} + +#[tokio::test] +async fn create_rejects_a_name_longer_than_253_characters() { + let mock = MockUserDefinedFunctionServiceImpl::new(); + let (server, _h) = server_with_mock(mock, true, false).await; + let mut params = create_params("[]"); + params.0.name = format!("a{}", "b".repeat(253)); + + let err = server + .create_user_defined_function(params) + .await + .expect_err("expected invalid name to be rejected"); + + assert_eq!(err.code, ErrorCode::INVALID_PARAMS); + assert!(err.message.contains("253 characters")); +} + #[tokio::test] async fn create_returns_the_new_function_and_next_step() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); @@ -470,6 +531,34 @@ async fn update_rejects_name_combined_with_other_fields() { assert!(err.message.contains("`name`")); } +#[tokio::test] +async fn update_accepts_a_name_with_a_trailing_digit() { + let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1"); + mock.expect_update_user_defined_function() + .times(1) + .withf(|req| { + let function = req + .get_ref() + .user_defined_function + .as_ref() + .expect("function present"); + function.name == "udf_1" + }) + .returning(|req| { + Ok(Response::new(UpdateUserDefinedFunctionResponse { + user_defined_function: req.into_inner().user_defined_function, + })) + }); + + let (server, _h) = server_with_mock(mock, true, true).await; + + server + .update_user_defined_function(update_params(Some("udf_1"), None, None)) + .await + .expect("name with trailing digit should be accepted"); +} + #[tokio::test] async fn update_rejects_malformed_function_inputs_json() { let mock = MockUserDefinedFunctionServiceImpl::new(); @@ -487,6 +576,7 @@ async fn update_rejects_malformed_function_inputs_json() { #[tokio::test] async fn update_returns_the_new_version() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1"); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -563,6 +653,7 @@ async fn archive_rejects_an_empty_id() { #[tokio::test] async fn archive_sets_the_archive_flag_and_reports_it() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f1"); mock.expect_update_user_defined_function() .times(1) .withf(|req| { @@ -596,6 +687,7 @@ async fn archive_sets_the_archive_flag_and_reports_it() { #[tokio::test] async fn unarchive_clears_the_archive_flag_and_reports_it() { let mut mock = MockUserDefinedFunctionServiceImpl::new(); + expect_current_udf(&mut mock, "f2"); mock.expect_update_user_defined_function() .times(1) .withf(|req| { From 8573279333ce486f1cae5194da7c41f3fe927fbf 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:27 -0700 Subject: [PATCH 8/8] rust(feat): bulk annotation updates (#752) 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 + .../sift_mcp/src/service/annotations/mod.rs | 267 +++++++++- .../sift_mcp/src/service/annotations/test.rs | 185 ++++++- .../sift_mcp/src/tool/annotations/mod.rs | 179 +++++-- .../sift_mcp/src/tool/annotations/test.rs | 480 +++++++++++++++++- 8 files changed, 1159 insertions(+), 74 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index d8d0acbcea..090d08c0fb 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 933525bd96..f1b096a04a 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 de9eab083b..47f56b3cb7 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". ---