From b5d8c0546141e6577d2a23d9d79b586ed8003412 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 12:46:25 -0700 Subject: [PATCH 1/5] rust(feat): gate MCP tools with account feature flags at startup --- rust/crates/sift_cli/CHANGELOG.md | 5 + rust/crates/sift_cli/Cargo.toml | 3 - .../sift_cli/assets/skills/sift/SKILL.md | 6 +- rust/crates/sift_cli/src/cli/mod.rs | 3 +- rust/crates/sift_cli/src/cmd/mcp.rs | 10 + rust/crates/sift_mcp/CLAUDE.md | 31 ++- rust/crates/sift_mcp/Cargo.toml | 3 - rust/crates/sift_mcp/src/client_event.rs | 39 ++-- rust/crates/sift_mcp/src/feature_flags.rs | 135 +++++++++++++ rust/crates/sift_mcp/src/lib.rs | 9 + rust/crates/sift_mcp/src/server/mod.rs | 18 +- rust/crates/sift_mcp/src/server/test.rs | 179 +++++++++++++++++- rust/crates/sift_mcp/src/service/mod.rs | 1 - rust/crates/sift_mcp/src/service/url/mod.rs | 1 - rust/crates/sift_mcp/src/tool/mod.rs | 1 - 15 files changed, 389 insertions(+), 55 deletions(-) create mode 100644 rust/crates/sift_mcp/src/feature_flags.rs diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index d8d0acbce..fa6dceabf 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +- Test-report tools are now always built and enabled at runtime per account via + feature flags evaluated when MCP starts. Flags are fetched in a brief startup + network call regardless of `--disable-nonessential-traffic`. The `test-reports` + Cargo feature is gone. + ## [v0.4.4] - August 24, 2026 ### What's New diff --git a/rust/crates/sift_cli/Cargo.toml b/rust/crates/sift_cli/Cargo.toml index 933525bd9..bdf7960ff 100644 --- a/rust/crates/sift_cli/Cargo.toml +++ b/rust/crates/sift_cli/Cargo.toml @@ -1,6 +1,3 @@ -[features] -test-reports = ["sift_mcp/test-reports"] - [package] name = "sift_cli" version = "0.4.4" diff --git a/rust/crates/sift_cli/assets/skills/sift/SKILL.md b/rust/crates/sift_cli/assets/skills/sift/SKILL.md index 052ef6ae3..622b815ba 100644 --- a/rust/crates/sift_cli/assets/skills/sift/SKILL.md +++ b/rust/crates/sift_cli/assets/skills/sift/SKILL.md @@ -56,8 +56,10 @@ exists. - **Test results:** `list_test_reports`, `list_test_steps`, `list_test_measurements`, `count_test_steps`, `count_test_measurements`. These, along with the `create_test_report` and `append_test_measurements` - writes below, are gated behind the `test-reports` Cargo feature (default on). - A server built with `--no-default-features` will not expose them. + writes below, are enabled per account by feature flags resolved when the MCP + server starts, so they may be absent from the tool list. Enabling them requires + the account's `test-reports` flag and an MCP restart. Tell the account owner to + contact Sift to enable the flag, then restart the MCP client. - **Data:** `get_data` writes channel data to a Parquet file. `sql` queries Parquet files. `upload_dataset` streams a Parquet dataset into Sift. - **Links:** `explore_url`. diff --git a/rust/crates/sift_cli/src/cli/mod.rs b/rust/crates/sift_cli/src/cli/mod.rs index 794fb5ffc..f0667642f 100644 --- a/rust/crates/sift_cli/src/cli/mod.rs +++ b/rust/crates/sift_cli/src/cli/mod.rs @@ -94,7 +94,8 @@ pub struct McpArgs { #[arg(long)] pub disable_update_check: bool, - /// Disable non-essential network traffic. Release checks are controlled + /// Disable non-essential network traffic. Feature-flag resolution at startup + /// is essential traffic and remains enabled. Release checks are controlled /// separately by `--disable-update-check`. #[arg(long)] pub disable_nonessential_traffic: bool, diff --git a/rust/crates/sift_cli/src/cmd/mcp.rs b/rust/crates/sift_cli/src/cmd/mcp.rs index 4b3d3cdf1..cdbc688aa 100644 --- a/rust/crates/sift_cli/src/cmd/mcp.rs +++ b/rust/crates/sift_cli/src/cmd/mcp.rs @@ -48,6 +48,15 @@ pub async fn run(ctx: Context, args: McpArgs, app_uri: String) -> Result Result Option<&'static str> { } #[cfg(test)] -pub(crate) async fn start_event_server() -> (String, tokio::task::JoinHandle>) { +pub(crate) async fn start_http_server( + response: Vec, +) -> (String, tokio::task::JoinHandle>) { use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, net::TcpListener, }; - fn request_length(request: &[u8]) -> Option { - let header_end = request - .windows(4) - .position(|window| window == b"\r\n\r\n")?; - let headers = std::str::from_utf8(&request[..header_end]).ok()?; + fn request_complete(request: &[u8]) -> bool { + let header_end = request.windows(4).position(|window| window == b"\r\n\r\n"); + let Some(header_end) = header_end else { + return false; + }; + let Ok(headers) = std::str::from_utf8(&request[..header_end]) else { + return false; + }; let content_length = headers.lines().find_map(|line| { let (name, value) = line.split_once(':')?; name.eq_ignore_ascii_case("content-length") .then(|| value.trim().parse::().ok()) .flatten() - })?; - Some(header_end + 4 + content_length) + }); + content_length.is_none_or(|length| request.len() >= header_end + 4 + length) } let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -129,22 +134,26 @@ pub(crate) async fn start_event_server() -> (String, tokio::task::JoinHandle= length) { + if request_complete(&request) { break; } } - stream - .write_all( - b"HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: 2\r\nconnection: close\r\n\r\n{}", - ) - .await - .unwrap(); + stream.write_all(&response).await.unwrap(); request }); (format!("http://{address}"), server) } +#[cfg(test)] +pub(crate) async fn start_event_server() -> (String, tokio::task::JoinHandle>) { + start_http_server( + b"HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: 2\r\nconnection: close\r\n\r\n{}" + .to_vec(), + ) + .await +} + #[cfg(test)] mod tests { use super::{ClientEventConfig, ClientEventReporter, start_event_server}; diff --git a/rust/crates/sift_mcp/src/feature_flags.rs b/rust/crates/sift_mcp/src/feature_flags.rs new file mode 100644 index 000000000..b8b6e78f2 --- /dev/null +++ b/rust/crates/sift_mcp/src/feature_flags.rs @@ -0,0 +1,135 @@ +use std::{collections::HashMap, time::Duration}; + +use anyhow::{Context, Result}; +use serde::Deserialize; + +const FEATURE_FLAGS_PATH: &str = "/api/v1/feature-flags/variants"; +const REQUEST_TIMEOUT: Duration = Duration::from_secs(5); + +pub(crate) static TOOL_FEATURE_FLAGS: &[(&str, &str)] = &[ + ("list_test_reports", "test-reports"), + ("list_test_steps", "test-reports"), + ("list_test_measurements", "test-reports"), + ("count_test_steps", "test-reports"), + ("count_test_measurements", "test-reports"), + ("create_test_report", "test-reports"), + ("append_test_measurements", "test-reports"), +]; + +#[derive(Clone, Debug, Default, Deserialize)] +pub struct FeatureFlags { + #[serde(default)] + variants: HashMap, +} + +#[derive(Clone, Debug, Deserialize)] +struct FeatureFlagVariant { + value: String, +} + +impl FeatureFlags { + pub fn enabled(&self, flag: &str) -> bool { + self.variants + .get(flag) + .is_some_and(|variant| !variant.value.is_empty() && variant.value != "off") + } + + pub async fn fetch(rest_uri: &str, api_key: &str) -> Result { + let endpoint = format!("{}{FEATURE_FLAGS_PATH}", rest_uri.trim_end_matches('/')); + reqwest::Client::new() + .get(endpoint) + .timeout(REQUEST_TIMEOUT) + .bearer_auth(api_key) + .send() + .await + .context("feature flag request failed")? + .error_for_status() + .context("feature flag request returned an error status")? + .json() + .await + .context("failed to parse feature flag response") + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::{FeatureFlagVariant, FeatureFlags}; + use crate::client_event::start_http_server; + + fn response(status: &str, body: &str) -> Vec { + format!( + "HTTP/1.1 {status}\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes() + } + + fn flags(value: Option<&str>) -> FeatureFlags { + let variants = value.map_or_else(HashMap::new, |value| { + HashMap::from([( + "test-flag".to_string(), + FeatureFlagVariant { + value: value.to_string(), + }, + )]) + }); + FeatureFlags { variants } + } + + #[test] + fn enabled_requires_a_non_off_variant() { + assert!(!flags(None).enabled("test-flag")); + assert!(!flags(Some("off")).enabled("test-flag")); + assert!(!flags(Some("")).enabled("test-flag")); + assert!(flags(Some("on")).enabled("test-flag")); + assert!(flags(Some("experimental")).enabled("test-flag")); + } + + #[test] + fn deserializes_feature_flag_response() { + let flags: FeatureFlags = serde_json::from_str( + r#"{"variants":{"some-flag":{"value":"on"},"other":{"value":"off"}}}"#, + ) + .unwrap(); + + assert!(flags.enabled("some-flag")); + assert!(!flags.enabled("other")); + } + + #[test] + fn empty_response_disables_all_flags() { + let flags: FeatureFlags = serde_json::from_str("{}").unwrap(); + + assert!(!flags.enabled("test-flag")); + } + + #[tokio::test] + async fn fetches_feature_flags_with_the_expected_request() { + let (rest_uri, server) = start_http_server(response( + "200 OK", + r#"{"variants":{"test-reports":{"value":"on"}}}"#, + )) + .await; + + let flags = FeatureFlags::fetch(&format!("{rest_uri}/"), "test-key") + .await + .unwrap(); + assert!(flags.enabled("test-reports")); + + let request = String::from_utf8(server.await.unwrap()).unwrap(); + let (headers, _) = request.split_once("\r\n\r\n").unwrap(); + assert!(headers.starts_with("GET /api/v1/feature-flags/variants HTTP/1.1")); + assert!( + headers + .lines() + .any(|line| line.eq_ignore_ascii_case("authorization: Bearer test-key")) + ); + + let (rest_uri, server) = + start_http_server(response("500 Internal Server Error", "{}")).await; + assert!(FeatureFlags::fetch(&rest_uri, "test-key").await.is_err()); + server.await.unwrap(); + } +} diff --git a/rust/crates/sift_mcp/src/lib.rs b/rust/crates/sift_mcp/src/lib.rs index dd78445f0..f9792979f 100644 --- a/rust/crates/sift_mcp/src/lib.rs +++ b/rust/crates/sift_mcp/src/lib.rs @@ -8,6 +8,9 @@ use tokio::sync::watch; mod client_event; pub use client_event::ClientEventConfig; +mod feature_flags; +pub use feature_flags::FeatureFlags; + mod server; use server::SiftMcpServer; @@ -109,6 +112,7 @@ pub async fn run_with_update_check( cli_version, update_check, None, + FeatureFlags::default(), ) .await } @@ -116,6 +120,7 @@ pub async fn run_with_update_check( /// Runs the server, reporting anonymous tool-call events only when a client /// event config is supplied. `None` leaves the server silent, which is what /// `sift-cli mcp --disable-nonessential-traffic` passes. +#[allow(clippy::too_many_arguments)] pub async fn run_with_client_events( credentials: Credentials, use_tls: bool, @@ -125,6 +130,7 @@ pub async fn run_with_client_events( cli_version: String, update_check: Option, client_event_config: Option, + feature_flags: FeatureFlags, ) -> Result<()> { let client_event_reporter = client_event::ClientEventReporter::from_config(client_event_config, &cli_version); @@ -138,6 +144,7 @@ pub async fn run_with_client_events( cli_version, update_check, client_event_reporter, + feature_flags, }, ) .await @@ -150,6 +157,7 @@ struct RunConfig { cli_version: String, update_check: Option, client_event_reporter: client_event::ClientEventReporter, + feature_flags: FeatureFlags, } async fn run_server(credentials: Credentials, use_tls: bool, config: RunConfig) -> Result<()> { @@ -167,6 +175,7 @@ async fn run_server(credentials: Credentials, use_tls: bool, config: RunConfig) config.cli_version, config.update_check, config.client_event_reporter, + config.feature_flags, ) .serve(stdio()) .await diff --git a/rust/crates/sift_mcp/src/server/mod.rs b/rust/crates/sift_mcp/src/server/mod.rs index a1c0a859c..7431b9714 100644 --- a/rust/crates/sift_mcp/src/server/mod.rs +++ b/rust/crates/sift_mcp/src/server/mod.rs @@ -16,7 +16,10 @@ use tokio::sync::watch; #[cfg(test)] use crate::UpdateCheck; -use crate::{UpdateCheckReceiver, client_event::ClientEventReporter, policy::RetryPolicy}; +use crate::{ + FeatureFlags, UpdateCheckReceiver, client_event::ClientEventReporter, + feature_flags::TOOL_FEATURE_FLAGS, policy::RetryPolicy, +}; #[cfg(test)] mod test; @@ -36,7 +39,6 @@ pub(crate) const BASE_INSTRUCTIONS: &str = concat!( "rules: fields at their default value (false, 0, empty string/list) are ", "omitted, so a missing boolean key means false, not unknown." ); -#[cfg(feature = "test-reports")] use crate::service::test_reports::TestReportService; use crate::service::{ annotations::AnnotationService, assets::AssetService, channels::ChannelService, @@ -61,7 +63,6 @@ pub struct SiftMcpServer { pub report_service: ReportService, pub report_template_service: ReportTemplateService, pub rule_service: RuleService, - #[cfg(feature = "test-reports")] pub test_report_service: TestReportService, pub docs_service: DocsService, pub user_service: UserService, @@ -162,9 +163,11 @@ impl SiftMcpServer { version, Some(update_check), ClientEventReporter::default(), + FeatureFlags::default(), ) } + #[allow(clippy::too_many_arguments)] pub(crate) fn new_with_client_events( channel: SiftChannel, app_uri: String, @@ -173,6 +176,7 @@ impl SiftMcpServer { cli_version: String, update_check: Option, client_event_reporter: ClientEventReporter, + feature_flags: FeatureFlags, ) -> Self { // Add more routers here as new tool groups are introduced, e.g. // tool_router.merge(Self::ingestion_router()) @@ -186,13 +190,17 @@ impl SiftMcpServer { tool_router.merge(Self::ping_router()); tool_router.merge(Self::rules_router()); tool_router.merge(Self::annotations_router()); - #[cfg(feature = "test-reports")] tool_router.merge(Self::test_reports_router()); tool_router.merge(Self::docs_router()); tool_router.merge(Self::users_router()); if update_check.is_some() { tool_router.merge(Self::update_router()); } + for &(tool_name, flag) in TOOL_FEATURE_FLAGS { + if !feature_flags.enabled(flag) { + tool_router.remove_route(tool_name); + } + } let prompt_router = Self::prompt_router(); @@ -210,7 +218,6 @@ impl SiftMcpServer { let report_template_service = ReportTemplateService::new(channel.clone(), retry_policy.clone()); let rule_service = RuleService::new(channel.clone(), retry_policy.clone()); - #[cfg(feature = "test-reports")] let test_report_service = TestReportService::new(channel.clone(), retry_policy.clone()); let docs_service = DocsService::new(channel.clone(), retry_policy.clone()); let user_service = UserService::new(channel.clone(), retry_policy); @@ -227,7 +234,6 @@ impl SiftMcpServer { report_service, report_template_service, rule_service, - #[cfg(feature = "test-reports")] test_report_service, docs_service, user_service, diff --git a/rust/crates/sift_mcp/src/server/test.rs b/rust/crates/sift_mcp/src/server/test.rs index c34c5b567..881df4b69 100644 --- a/rust/crates/sift_mcp/src/server/test.rs +++ b/rust/crates/sift_mcp/src/server/test.rs @@ -3,7 +3,13 @@ use std::time::Duration; use rmcp::{ServerHandler, ServiceExt, model::ProtocolVersion}; use serde_json::Value; use sift_rs::assets::v1::{ListAssetsResponse, asset_service_server::AssetServiceServer}; -use sift_test_util::{grpc::memory_sift_channel, mock::assets::v1::MockAssetServiceImpl}; +use sift_rs::test_reports::v1::{ + ListTestReportsResponse, test_report_service_server::TestReportServiceServer, +}; +use sift_test_util::{ + grpc::memory_sift_channel, + mock::{assets::v1::MockAssetServiceImpl, test_reports::v1::MockTestReportServiceImpl}, +}; use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt, BufReader, DuplexStream}, sync::watch, @@ -12,8 +18,9 @@ use tokio::{ use tonic::{Response, transport::Server}; use crate::{ - ClientEventConfig, UpdateCheck, + ClientEventConfig, FeatureFlags, UpdateCheck, client_event::{ClientEventReporter, event_for_tool, start_event_server}, + feature_flags::TOOL_FEATURE_FLAGS, }; use super::SiftMcpServer; @@ -34,6 +41,14 @@ const EXPECTED_BASE_INSTRUCTIONS: &str = concat!( "omitted, so a missing boolean key means false, not unknown." ); +fn all_feature_flags() -> FeatureFlags { + let variants = TOOL_FEATURE_FLAGS + .iter() + .map(|&(_, flag)| (flag.to_string(), serde_json::json!({ "value": "on" }))) + .collect::>(); + serde_json::from_value(serde_json::json!({ "variants": variants })).unwrap() +} + fn update_available() -> UpdateCheck { UpdateCheck::UpdateAvailable { current_version: "0.3.0".to_string(), @@ -65,17 +80,39 @@ async fn server_with_client_events( update_check: Option>, asset_tool_calls: usize, client_event_reporter: ClientEventReporter, +) -> (SiftMcpServer, JoinHandle<()>) { + server_with_feature_flags( + update_check, + asset_tool_calls, + client_event_reporter, + FeatureFlags::default(), + ) + .await +} + +async fn server_with_feature_flags( + update_check: Option>, + asset_tool_calls: usize, + client_event_reporter: ClientEventReporter, + feature_flags: FeatureFlags, ) -> (SiftMcpServer, JoinHandle<()>) { let mut mock = MockAssetServiceImpl::new(); mock.expect_list_assets() .times(asset_tool_calls) .returning(|_| Ok(Response::new(ListAssetsResponse::default()))); + let mut test_report_mock = MockTestReportServiceImpl::new(); + test_report_mock + .expect_list_test_reports() + .times(0..) + .returning(|_| Ok(Response::new(ListTestReportsResponse::default()))); + let (client, server) = tokio::io::duplex(1024); let channel = memory_sift_channel(client).await; let handle = tokio::spawn(async move { Server::builder() .add_service(AssetServiceServer::new(mock)) + .add_service(TestReportServiceServer::new(test_report_mock)) .serve_with_incoming(tokio_stream::once(Ok::<_, std::io::Error>(server))) .await .unwrap(); @@ -90,11 +127,141 @@ async fn server_with_client_events( CLI_VERSION.to_string(), update_check, client_event_reporter, + feature_flags, ), handle, ) } +#[tokio::test] +async fn feature_flag_gates_test_report_tools() { + let (disabled, disabled_handle) = server_with_feature_flags( + None, + 0, + ClientEventReporter::default(), + FeatureFlags::default(), + ) + .await; + assert!(disabled.tool_router.list_all().iter().all(|tool| { + !TOOL_FEATURE_FLAGS + .iter() + .any(|&(name, _)| tool.name == name) + })); + assert!( + disabled + .tool_router + .list_all() + .iter() + .any(|tool| tool.name == "list_assets") + ); + disabled_handle.abort(); + + let enabled_flags = all_feature_flags(); + let (enabled, enabled_handle) = + server_with_feature_flags(None, 0, ClientEventReporter::default(), enabled_flags).await; + let routed_tools = enabled.tool_router.list_all(); + let missing: Vec<_> = TOOL_FEATURE_FLAGS + .iter() + .filter_map(|&(name, _)| { + (!routed_tools.iter().any(|tool| tool.name == name)).then_some(name) + }) + .collect(); + assert!( + missing.is_empty(), + "feature-flagged tools not routed: {missing:?}" + ); + enabled_handle.abort(); +} + +async fn initialized_client_with_feature_flags( + feature_flags: FeatureFlags, +) -> ( + BufReader>, + tokio::io::WriteHalf, + JoinHandle<()>, +) { + let (server, grpc_handle) = + server_with_feature_flags(None, 0, ClientEventReporter::default(), feature_flags).await; + let (reader, mut writer, mcp_handle) = connect_server(server, grpc_handle).await; + + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-11-25", + "capabilities": {}, + "clientInfo": { "name": "test-client", "version": "0.0.1" } + } + }); + writer + .write_all(format!("{request}\n").as_bytes()) + .await + .unwrap(); + + (reader, writer, mcp_handle) +} + +#[tokio::test] +async fn feature_flagged_tool_calls_require_the_flag() { + let (mut reader, mut writer, server) = + initialized_client_with_feature_flags(FeatureFlags::default()).await; + let _initialize = read_json(&mut reader).await; + writer + .write_all(b"{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n") + .await + .unwrap(); + writer + .write_all(b"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/list\",\"params\":{}}\n") + .await + .unwrap(); + let tools = read_json(&mut reader).await; + let tools = tools["result"]["tools"].as_array().unwrap(); + assert!(tools.iter().all(|tool| { + !TOOL_FEATURE_FLAGS + .iter() + .any(|&(name, _)| tool["name"] == name) + })); + assert!(tools.iter().any(|tool| tool["name"] == "list_assets")); + writer + .write_all( + b"{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":\"tools/call\",\"params\":{\"name\":\"list_test_reports\",\"arguments\":{\"filter\":\"\"}}}\n", + ) + .await + .unwrap(); + let response = read_json(&mut reader).await; + assert!(response["error"].is_object()); + assert!( + response["error"]["message"] + .as_str() + .unwrap() + .to_ascii_lowercase() + .contains("not found"), + "unexpected response: {response}" + ); + finish(reader, writer, server).await; + + let (mut reader, mut writer, server) = + initialized_client_with_feature_flags(all_feature_flags()).await; + let _initialize = read_json(&mut reader).await; + writer + .write_all(b"{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n") + .await + .unwrap(); + writer + .write_all( + b"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"list_test_reports\",\"arguments\":{\"filter\":\"\"}}}\n", + ) + .await + .unwrap(); + let response = read_json(&mut reader).await; + assert_eq!( + response["result"]["structuredContent"], + serde_json::json!({ "test_reports": [], "count": 0, "has_more": false }) + ); + finish(reader, writer, server).await; +} + async fn connected_client_with_events( update_check: Option>, asset_tool_calls: usize, @@ -284,7 +451,13 @@ async fn every_registered_tool_has_a_client_event() { current_version: "0.4.0".to_string(), latest_version: "0.4.0".to_string(), }; - let (server, grpc_handle) = server_with_update_check(Some(receiver(current)), 0).await; + let (server, grpc_handle) = server_with_feature_flags( + Some(receiver(current)), + 0, + ClientEventReporter::default(), + all_feature_flags(), + ) + .await; let missing: Vec<_> = server .tool_router .list_all() diff --git a/rust/crates/sift_mcp/src/service/mod.rs b/rust/crates/sift_mcp/src/service/mod.rs index 2ab197f71..a83451035 100644 --- a/rust/crates/sift_mcp/src/service/mod.rs +++ b/rust/crates/sift_mcp/src/service/mod.rs @@ -9,7 +9,6 @@ pub mod report_templates; pub mod reports; pub mod rules; pub mod runs; -#[cfg(feature = "test-reports")] pub mod test_reports; pub mod url; pub mod users; diff --git a/rust/crates/sift_mcp/src/service/url/mod.rs b/rust/crates/sift_mcp/src/service/url/mod.rs index dd029df5a..595e35311 100644 --- a/rust/crates/sift_mcp/src/service/url/mod.rs +++ b/rust/crates/sift_mcp/src/service/url/mod.rs @@ -127,7 +127,6 @@ impl UrlService { Ok(format!("{host}/reports/{}", encode_value(report_id))) } - #[cfg(feature = "test-reports")] pub fn build_test_report_url(&self, test_report_id: &str) -> Result { let host = self.app_host()?; Ok(format!( diff --git a/rust/crates/sift_mcp/src/tool/mod.rs b/rust/crates/sift_mcp/src/tool/mod.rs index c5f5d0c9e..a46730849 100644 --- a/rust/crates/sift_mcp/src/tool/mod.rs +++ b/rust/crates/sift_mcp/src/tool/mod.rs @@ -10,7 +10,6 @@ pub mod report_templates; pub mod reports; pub mod rules; pub mod runs; -#[cfg(feature = "test-reports")] pub mod test_reports; pub mod update; pub mod users; From 8356f2fa3ef2fc707138305f6482ac3b93f73869 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 13:15:11 -0700 Subject: [PATCH 2/5] rust(chore): keep feature-flag names out of prose docs --- rust/crates/sift_cli/AGENTS.md | 2 ++ rust/crates/sift_cli/CHANGELOG.md | 5 ----- rust/crates/sift_cli/assets/skills/sift/SKILL.md | 4 ++-- rust/crates/sift_mcp/CLAUDE.md | 6 +++--- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/rust/crates/sift_cli/AGENTS.md b/rust/crates/sift_cli/AGENTS.md index d8179349c..0da7d7036 100644 --- a/rust/crates/sift_cli/AGENTS.md +++ b/rust/crates/sift_cli/AGENTS.md @@ -54,6 +54,8 @@ Keep the skill accurate to the CLI and `sift_mcp` tool surfaces. In particular: - Teach agents to use `agent doctor`, `install`, and `update` instead of editing one client. They must obtain explicit user approval before running `agent update --allow-destructive` and tell the user to reload the client. +- Do not cite flag names or enumerate flag-gated tools in prose docs; the registry + is the source of truth and prose copies go stale. Write in direct voice and keep it concise. The skill is loaded under context pressure, so every line should change what the agent does. diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index fa6dceabf..d8d0acbce 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,11 +7,6 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New -- Test-report tools are now always built and enabled at runtime per account via - feature flags evaluated when MCP starts. Flags are fetched in a brief startup - network call regardless of `--disable-nonessential-traffic`. The `test-reports` - Cargo feature is gone. - ## [v0.4.4] - August 24, 2026 ### What's New diff --git a/rust/crates/sift_cli/assets/skills/sift/SKILL.md b/rust/crates/sift_cli/assets/skills/sift/SKILL.md index 622b815ba..a624fd536 100644 --- a/rust/crates/sift_cli/assets/skills/sift/SKILL.md +++ b/rust/crates/sift_cli/assets/skills/sift/SKILL.md @@ -58,8 +58,8 @@ exists. These, along with the `create_test_report` and `append_test_measurements` writes below, are enabled per account by feature flags resolved when the MCP server starts, so they may be absent from the tool list. Enabling them requires - the account's `test-reports` flag and an MCP restart. Tell the account owner to - contact Sift to enable the flag, then restart the MCP client. + an account setting and an MCP restart. Tell the account owner to contact Sift + to enable them, then restart the MCP client. - **Data:** `get_data` writes channel data to a Parquet file. `sql` queries Parquet files. `upload_dataset` streams a Parquet dataset into Sift. - **Links:** `explore_url`. diff --git a/rust/crates/sift_mcp/CLAUDE.md b/rust/crates/sift_mcp/CLAUDE.md index bf5ba2b88..58627209a 100644 --- a/rust/crates/sift_mcp/CLAUDE.md +++ b/rust/crates/sift_mcp/CLAUDE.md @@ -644,12 +644,12 @@ flag is disabled. Most tools are unflagged and always available. Flag changes ap restart. - To put a tool behind a flag, add its `(tool name, flag name)` pair to `TOOL_FEATURE_FLAGS` in - `feature_flags.rs`. The `test-reports` flag currently gates the seven test-report tools: - `list_test_reports`, `list_test_steps`, `list_test_measurements`, `count_test_steps`, - `count_test_measurements`, `create_test_report`, and `append_test_measurements`. + `feature_flags.rs`. - If fetching flags fails, the server starts normally with all flag-gated tools disabled. - Gated tools still need entries in `tool_events.json` and tests. The registry-drift and event-invariant tests in `server/test.rs` cover gated tools through the all-flags-enabled path. +- Do not cite flag names or enumerate flag-gated tools in prose docs, including `CLAUDE.md`, + `SKILL.md`, `CHANGELOG`, or PR text. The registry is the source of truth; prose copies go stale. --- From 0c008bdb751e53e76ee9a5cacad2a457c63ea4a7 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Wed, 26 Aug 2026 15:17:29 -0700 Subject: [PATCH 3/5] rust(chore): treat a variant without a value as disabled --- rust/crates/sift_mcp/src/feature_flags.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/crates/sift_mcp/src/feature_flags.rs b/rust/crates/sift_mcp/src/feature_flags.rs index b8b6e78f2..cfe14d1ea 100644 --- a/rust/crates/sift_mcp/src/feature_flags.rs +++ b/rust/crates/sift_mcp/src/feature_flags.rs @@ -24,6 +24,7 @@ pub struct FeatureFlags { #[derive(Clone, Debug, Deserialize)] struct FeatureFlagVariant { + #[serde(default)] value: String, } @@ -90,12 +91,13 @@ mod tests { #[test] fn deserializes_feature_flag_response() { let flags: FeatureFlags = serde_json::from_str( - r#"{"variants":{"some-flag":{"value":"on"},"other":{"value":"off"}}}"#, + r#"{"variants":{"some-flag":{"value":"on"},"other":{"value":"off"},"bare":{}}}"#, ) .unwrap(); assert!(flags.enabled("some-flag")); assert!(!flags.enabled("other")); + assert!(!flags.enabled("bare")); } #[test] From 7d72e2a894438649f7c6d5374faa5be37257eb7e Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Thu, 27 Aug 2026 17:49:06 -0700 Subject: [PATCH 4/5] rust(docs): add changelog entry for feature-flag tool gating --- rust/crates/sift_cli/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index 090d08c0f..7dc3986cd 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,6 +7,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New +- Some MCP tools are now enabled per account by feature flags resolved at server + startup. A tool absent from the tool list needs its account flag enabled and an + MCP restart. + ## [v0.5.0] - August 26, 2026 ### What's New From 471b275b19e55939c0748f3f20fce621345db9d6 Mon Sep 17 00:00:00 2001 From: Evan Frawley-Tsang Date: Thu, 27 Aug 2026 17:49:40 -0700 Subject: [PATCH 5/5] rust(docs): move feature-flag changelog entry into 0.5.0 --- rust/crates/sift_cli/CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index 7dc3986cd..26ca31c5e 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -7,14 +7,14 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### What's New -- Some MCP tools are now enabled per account by feature flags resolved at server - startup. A tool absent from the tool list needs its account flag enabled and an - MCP restart. - ## [v0.5.0] - August 26, 2026 ### What's New +- Some MCP tools are now enabled per account by feature flags resolved at server + startup. A tool absent from the tool list needs its account flag enabled and an + MCP restart. + - Added MCP tools for managing calculated channels: `list_calculated_channels`, `list_calculated_channel_versions`, `create_calculated_channel`, `update_calculated_channel`, `archive_calculated_channel`, and