From b3f89d26ece443e0ba632085c4515cc619c11e57 Mon Sep 17 00:00:00 2001 From: Konstantinos Stefanidis Vozikis Date: Sat, 11 Jul 2026 18:06:02 +0200 Subject: [PATCH] feat: add beta label to CLI output --- Cargo.lock | 1 + crates/config/Cargo.toml | 3 + crates/config/src/lib.rs | 4 +- crates/config/src/session.rs | 104 ++++++++++++++++++++++++++++++- crates/tower-cmd/src/beta.rs | 103 ++++++++++++++++++++++++++++++ crates/tower-cmd/src/catalogs.rs | 70 +++++++++++++++++++-- crates/tower-cmd/src/lib.rs | 16 +++++ crates/tower-cmd/src/output.rs | 5 ++ 8 files changed, 298 insertions(+), 8 deletions(-) create mode 100644 crates/tower-cmd/src/beta.rs diff --git a/Cargo.lock b/Cargo.lock index 013fb4b7..0e39ade0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -501,6 +501,7 @@ dependencies = [ "serde", "serde_json", "snafu", + "tempfile", "testutils", "tokio", "tower-api", diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 3cf75e69..60b85e58 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -21,3 +21,6 @@ url = { workspace = true } tower-api = { workspace = true } tower-package = { workspace = true } tower-telemetry = { workspace = true } + +[dev-dependencies] +tempfile = { workspace = true } diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 2e4a48a2..07b36f3d 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -10,7 +10,9 @@ pub use error::Error; pub use session::{default_tower_url, Session, Team, Token, User}; pub use tower_package::{Parameter, Towerfile}; -pub use session::{get_last_version_check_timestamp, set_last_version_check_timestamp}; +pub use session::{ + claim_notice, get_last_version_check_timestamp, set_last_version_check_timestamp, +}; #[derive(Clone, Serialize, Deserialize)] pub struct Config { diff --git a/crates/config/src/session.rs b/crates/config/src/session.rs index 44bea0e4..5537161b 100644 --- a/crates/config/src/session.rs +++ b/crates/config/src/session.rs @@ -1,9 +1,10 @@ use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use chrono::{DateTime, TimeZone, Utc}; use serde::{Deserialize, Serialize}; -use std::fs; +use std::fs::{self, OpenOptions}; use std::future::Future; -use std::path::PathBuf; +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; use url::Url; use crate::error::Error; @@ -87,6 +88,43 @@ fn find_or_create_config_dir() -> Result { Ok(config_dir) } +/// Atomically claims a persistent, user-level CLI notice. +/// +/// Returns `true` only for the first process to claim this notice ID. Notice +/// IDs are internal constants and may contain ASCII letters, digits, `-`, and +/// `_` only. +pub fn claim_notice(id: &str) -> std::io::Result { + let config_dir = find_or_create_config_dir() + .map_err(|err| std::io::Error::other(format!("finding Tower config directory: {err}")))?; + claim_notice_at(&config_dir, id) +} + +fn claim_notice_at(base_dir: &Path, id: &str) -> std::io::Result { + if id.is_empty() + || !id + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_')) + { + return Err(std::io::Error::new( + ErrorKind::InvalidInput, + "invalid notice ID", + )); + } + + let notices_dir = base_dir.join("notices"); + fs::create_dir_all(¬ices_dir)?; + + match OpenOptions::new() + .write(true) + .create_new(true) + .open(notices_dir.join(id)) + { + Ok(_) => Ok(true), + Err(err) if err.kind() == ErrorKind::AlreadyExists => Ok(false), + Err(err) => Err(err), + } +} + pub fn get_last_version_check_timestamp() -> DateTime { let default: DateTime = Utc.timestamp_opt(0, 0).unwrap(); @@ -346,3 +384,65 @@ where { tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(future)) } + +#[cfg(test)] +mod tests { + use std::{fs, sync::Arc, thread}; + + use tempfile::TempDir; + + use super::claim_notice_at; + + #[test] + fn notice_can_only_be_claimed_once() { + let temp_dir = TempDir::new().expect("temporary directory should be created"); + + assert!(claim_notice_at(temp_dir.path(), "storage-beta-v1").unwrap()); + assert!(!claim_notice_at(temp_dir.path(), "storage-beta-v1").unwrap()); + } + + #[test] + fn notice_generations_are_independent() { + let temp_dir = TempDir::new().expect("temporary directory should be created"); + + assert!(claim_notice_at(temp_dir.path(), "storage-beta-v1").unwrap()); + assert!(claim_notice_at(temp_dir.path(), "storage-beta-v2").unwrap()); + } + + #[test] + fn concurrent_notice_claims_have_one_winner() { + let temp_dir = TempDir::new().expect("temporary directory should be created"); + let base_dir = Arc::new(temp_dir.path().to_path_buf()); + let handles = (0..8) + .map(|_| { + let base_dir = Arc::clone(&base_dir); + thread::spawn(move || claim_notice_at(&base_dir, "storage-beta-v1").unwrap()) + }) + .collect::>(); + + let winners = handles + .into_iter() + .map(|handle| handle.join().expect("claim thread should finish")) + .filter(|claimed| *claimed) + .count(); + + assert_eq!(winners, 1); + } + + #[test] + fn notice_claim_rejects_invalid_ids() { + let temp_dir = TempDir::new().expect("temporary directory should be created"); + + assert!(claim_notice_at(temp_dir.path(), "../outside").is_err()); + assert!(claim_notice_at(temp_dir.path(), "").is_err()); + } + + #[test] + fn notice_claim_reports_unavailable_base_directory() { + let temp_dir = TempDir::new().expect("temporary directory should be created"); + let base_dir = temp_dir.path().join("config-file"); + fs::write(&base_dir, "not a directory").expect("fixture should be written"); + + assert!(claim_notice_at(&base_dir, "storage-beta-v1").is_err()); + } +} diff --git a/crates/tower-cmd/src/beta.rs b/crates/tower-cmd/src/beta.rs new file mode 100644 index 00000000..fb6c0c29 --- /dev/null +++ b/crates/tower-cmd/src/beta.rs @@ -0,0 +1,103 @@ +use std::io::{self, IsTerminal}; + +use tower_telemetry::debug; + +use crate::output::{self, OutputMode}; + +pub(crate) struct BetaFeature { + id: &'static str, + message: &'static str, + docs_url: Option<&'static str>, +} + +impl BetaFeature { + pub fn short_about(&self, description: &str) -> String { + format!("{description} [beta]") + } + + pub fn notice(&self) -> String { + match self.docs_url { + Some(url) => format!("{} Learn more: {url}", self.message), + None => self.message.to_string(), + } + } +} + +pub(crate) const STORAGE_BETA_MESSAGE: &str = "Tower Storage is in beta. Core functionality is stable, but some featues and interfaces might change before general availability."; + +pub(crate) const STORAGE: BetaFeature = BetaFeature { + id: "storage-beta-v1", + message: STORAGE_BETA_MESSAGE, + docs_url: None, +}; + +pub(crate) fn notify_once(feature: &BetaFeature) { + let output_mode = output::get_output_mode(); + let stdout_is_terminal = io::stdout().is_terminal(); + let stderr_is_terminal = io::stderr().is_terminal(); + + if !should_notify(output_mode, stdout_is_terminal, stderr_is_terminal) { + return; + } + + match config::claim_notice(feature.id) { + Ok(true) => output::notice_to_stderr("Beta:", &feature.notice()), + Ok(false) => {} + Err(err) => debug!("Failed to persist CLI notice {}: {}", feature.id, err), + } +} + +fn should_notify( + output_mode: OutputMode, + stdout_is_terminal: bool, + stderr_is_terminal: bool, +) -> bool { + output_mode.is_normal() && stdout_is_terminal && stderr_is_terminal +} + +#[cfg(test)] +mod tests { + use super::{should_notify, BetaFeature, STORAGE, STORAGE_BETA_MESSAGE}; + use crate::output::OutputMode; + + #[test] + fn short_about_has_one_beta_suffix() { + let about = STORAGE.short_about("Use Tower Storage"); + + assert_eq!(about, "Use Tower Storage [beta]"); + assert_eq!(about.matches("[beta]").count(), 1); + } + + #[test] + fn notice_omits_docs_sentence_without_a_url() { + let notice = STORAGE.notice(); + + assert_eq!(notice, STORAGE_BETA_MESSAGE); + assert!(!notice.contains("Learn more:")); + } + + #[test] + fn notice_includes_docs_url_when_configured() { + let feature = BetaFeature { + id: "example-beta-v1", + message: "Example is in beta. Its interface may change.", + docs_url: Some("https://example.com/beta"), + }; + + assert_eq!( + feature.notice(), + "Example is in beta. Its interface may change. Learn more: https://example.com/beta" + ); + } + + #[test] + fn notice_requires_normal_output_and_two_terminals() { + assert!(should_notify(OutputMode::Normal, true, true)); + assert!(!should_notify(OutputMode::Normal, false, true)); + assert!(!should_notify(OutputMode::Normal, true, false)); + assert!(!should_notify(OutputMode::Normal, false, false)); + assert!(!should_notify(OutputMode::Json, true, true)); + assert!(!should_notify(OutputMode::McpStdio, true, true)); + assert!(!should_notify(OutputMode::McpStreaming, true, true)); + } +} diff --git a/crates/tower-cmd/src/catalogs.rs b/crates/tower-cmd/src/catalogs.rs index e52a3f11..4c0a3801 100644 --- a/crates/tower-cmd/src/catalogs.rs +++ b/crates/tower-cmd/src/catalogs.rs @@ -5,13 +5,17 @@ use tower_api::models::{ vend_catalog_credentials_body, CatalogCredentials, DescribeCatalogResponse, }; -use crate::{api, output, util::cmd}; +use crate::{api, beta, output, util::cmd}; const STORAGE_CATALOG_TYPE: &str = "tower-catalog"; pub fn catalogs_cmd() -> Command { Command::new("catalogs") - .about("Interact with the catalogs in your Tower account") + .about(format!( + "Interact with the catalogs in your Tower account (includes {})", + beta::STORAGE.short_about("Storage") + )) + .after_help(beta::STORAGE.notice()) .arg_required_else_help(true) .subcommand( Command::new("list") @@ -41,7 +45,7 @@ pub fn catalogs_cmd() -> Command { .arg( Arg::new("storage") .long("storage") - .help("List Tower-managed storage catalogs") + .help(beta::STORAGE.short_about("List Tower-managed storage catalogs")) .conflicts_with("type") .action(ArgAction::SetTrue), ) @@ -107,7 +111,10 @@ pub fn catalogs_cmd() -> Command { .help("Print the vended OAuth token in normal output") .action(ArgAction::SetTrue), ) - .about("Vend short-lived catalog credentials for external tools"), + .about( + beta::STORAGE + .short_about("Vend short-lived catalog credentials for external tools"), + ), ) } @@ -120,6 +127,10 @@ pub async fn do_list(config: Config, args: &ArgMatches) { args.get_one::("type").map(String::as_str) }; + if is_storage_catalog_type(catalog_type) { + beta::notify_once(&beta::STORAGE); + } + let catalogs = output::with_spinner( "Listing catalogs", api::list_catalogs(&config, &env, all, catalog_type), @@ -144,6 +155,8 @@ pub async fn do_list(config: Config, args: &ArgMatches) { } pub async fn do_credentials(config: Config, args: &ArgMatches) { + beta::notify_once(&beta::STORAGE); + let name = args .get_one::("catalog_name") .expect("catalog_name is required"); @@ -184,6 +197,9 @@ pub async fn do_show(config: Config, args: &ArgMatches) { match api::describe_catalog(&config, name, &env).await { Ok(response) => { + if is_storage_catalog_type(Some(&response.catalog.r#type)) { + beta::notify_once(&beta::STORAGE); + } let human = catalog_details_text(&response); output::text(&human, &response); } @@ -191,6 +207,10 @@ pub async fn do_show(config: Config, args: &ArgMatches) { } } +fn is_storage_catalog_type(catalog_type: Option<&str>) -> bool { + catalog_type == Some(STORAGE_CATALOG_TYPE) +} + fn parse_mode(mode: &str) -> vend_catalog_credentials_body::Mode { match mode { "read-write" => vend_catalog_credentials_body::Mode::ReadWrite, @@ -420,7 +440,9 @@ fn snippets( #[cfg(test)] mod tests { - use super::{catalogs_cmd, parse_mode, snippets, token_export_command}; + use super::{ + catalogs_cmd, is_storage_catalog_type, parse_mode, snippets, token_export_command, + }; use tower_api::models::{vend_catalog_credentials_body, CatalogCredentials}; #[test] @@ -495,6 +517,44 @@ mod tests { assert!(result.is_err()); } + #[test] + fn storage_catalog_type_detection_is_scoped() { + assert!(is_storage_catalog_type(Some("tower-catalog"))); + assert!(!is_storage_catalog_type(Some("snowflake-open-catalog"))); + assert!(!is_storage_catalog_type(None)); + } + + #[test] + fn catalog_help_marks_storage_beta_in_short_and_long_help() { + let short_help = catalogs_cmd().render_help().to_string(); + let long_help = catalogs_cmd().render_long_help().to_string(); + + for help in [short_help, long_help] { + assert!(help.contains("includes Storage [beta]")); + assert!(help.contains("Tower Storage is in beta.")); + } + } + + #[test] + fn storage_specific_command_and_flag_are_marked_beta() { + let mut command = catalogs_cmd(); + let credentials_help = command + .find_subcommand_mut("credentials") + .expect("credentials command should exist") + .render_help() + .to_string(); + assert!(credentials_help + .contains("Vend short-lived catalog credentials for external tools [beta]")); + + let mut command = catalogs_cmd(); + let list_help = command + .find_subcommand_mut("list") + .expect("list command should exist") + .render_help() + .to_string(); + assert!(list_help.contains("List Tower-managed storage catalogs [beta]")); + } + #[test] fn show_requires_catalog_name() { let result = catalogs_cmd().try_get_matches_from(["catalogs", "show"]); diff --git a/crates/tower-cmd/src/lib.rs b/crates/tower-cmd/src/lib.rs index 9eb84280..b45bc38e 100644 --- a/crates/tower-cmd/src/lib.rs +++ b/crates/tower-cmd/src/lib.rs @@ -3,6 +3,7 @@ use config::{Config, Session}; pub mod api; mod apps; +mod beta; mod catalogs; mod deploy; mod environments; @@ -304,3 +305,18 @@ fn root_cmd() -> Command { .subcommand(mcp::mcp_cmd()) .subcommand(skill::skill_cmd()) } + +#[cfg(test)] +mod tests { + use super::root_cmd; + + #[test] + fn root_help_scopes_beta_label_to_storage() { + let help = root_cmd().render_help().to_string(); + + assert!(help.contains( + "Interact with the catalogs in your Tower account (includes Storage [beta])" + )); + assert!(!help.contains("catalogs [beta]")); + } +} diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index 862e8839..12743ec1 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -68,6 +68,11 @@ fn write_to_stderr(msg: &str) { stderr.flush().ok(); } +pub(crate) fn notice_to_stderr(label: &str, msg: &str) { + let line = format!("{} {}\n", label.bold().yellow(), msg); + write_to_stderr(&line); +} + pub fn set_output_mode(mode: OutputMode) { OUTPUT_MODE.set(mode).ok(); if mode.is_mcp() {