From 7d58f19adb5f067e0c29b3082887cd7ff8f79a41 Mon Sep 17 00:00:00 2001 From: Ben Lovell Date: Thu, 30 Jul 2026 16:17:23 +0200 Subject: [PATCH] refactor: show beta notices whenever stderr is a terminal --- crates/tower-cmd/src/beta.rs | 45 +++++--------------------------- crates/tower-cmd/src/catalogs.rs | 16 ++++++------ crates/tower-cmd/src/output.rs | 17 +++++++++++- 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/crates/tower-cmd/src/beta.rs b/crates/tower-cmd/src/beta.rs index 09e746c4..83b9ab9d 100644 --- a/crates/tower-cmd/src/beta.rs +++ b/crates/tower-cmd/src/beta.rs @@ -1,8 +1,4 @@ -use std::io::{self, IsTerminal}; - -use tower_telemetry::debug; - -use crate::output::{self, Out}; +use crate::output; pub(crate) struct BetaFeature { id: &'static str, @@ -21,6 +17,10 @@ impl BetaFeature { None => self.message.to_string(), } } + + pub fn notify_once(&self) { + output::notice_once(self.id, "Beta:", &self.notice()); + } } 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."; @@ -31,30 +31,9 @@ pub(crate) const STORAGE: BetaFeature = BetaFeature { docs_url: None, }; -pub(crate) fn notify_once(out: &Out, feature: &BetaFeature) { - let stderr_is_terminal = io::stderr().is_terminal(); - - if !should_notify(out.interactive(), out.foreground(), 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), - } -} - -/// The notice only goes out for a foreground CLI driving an interactive terminal: -/// human output on a stdout TTY (never JSON or MCP capture), with stderr also a -/// TTY so the notice itself is seen. -fn should_notify(interactive: bool, foreground: bool, stderr_is_terminal: bool) -> bool { - interactive && foreground && stderr_is_terminal -} - #[cfg(test)] mod tests { - use super::{should_notify, BetaFeature, STORAGE, STORAGE_BETA_MESSAGE}; + use super::{BetaFeature, STORAGE, STORAGE_BETA_MESSAGE}; #[test] fn short_about_has_one_beta_suffix() { @@ -85,16 +64,4 @@ mod tests { "Example is in beta. Its interface may change. Learn more: https://example.com/beta" ); } - - #[test] - fn notice_requires_interactive_foreground_and_stderr_terminal() { - assert!(should_notify(true, true, true)); - // stdout not an interactive terminal (redirected, JSON, or MCP capture) - assert!(!should_notify(false, true, true)); - // not a foreground CLI (MCP or discarded output) - assert!(!should_notify(true, false, true)); - // stderr not a terminal - assert!(!should_notify(true, true, false)); - assert!(!should_notify(false, false, false)); - } } diff --git a/crates/tower-cmd/src/catalogs.rs b/crates/tower-cmd/src/catalogs.rs index 1bc6ea19..f829069f 100644 --- a/crates/tower-cmd/src/catalogs.rs +++ b/crates/tower-cmd/src/catalogs.rs @@ -186,7 +186,7 @@ pub async fn do_list(out: &output::Out, config: Config, args: &ArgMatches) { }; if is_storage_catalog_type(catalog_type) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); } let catalogs = out @@ -214,7 +214,7 @@ pub async fn do_list(out: &output::Out, config: Config, args: &ArgMatches) { } pub async fn do_credentials(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let name = args .get_one::("catalog_name") @@ -263,7 +263,7 @@ pub async fn do_show(out: &output::Out, config: Config, args: &ArgMatches) { let is_storage = is_storage_catalog_type(Some(&response.catalog.r#type)); if is_storage { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); } let tables = if is_storage { @@ -464,7 +464,7 @@ fn redact_token(message: &str, token: &str) -> String { } pub async fn do_query(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let name = args .get_one::("catalog_name") @@ -1550,7 +1550,7 @@ fn knowledge_cmd() -> Command { } pub async fn do_knowledge_list(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let catalog = args .get_one::("catalog_name") @@ -1587,7 +1587,7 @@ pub async fn do_knowledge_list(out: &output::Out, config: Config, args: &ArgMatc } pub async fn do_knowledge_show(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let catalog = args .get_one::("catalog_name") @@ -1609,7 +1609,7 @@ pub async fn do_knowledge_show(out: &output::Out, config: Config, args: &ArgMatc } pub async fn do_knowledge_set(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let catalog = args .get_one::("catalog_name") @@ -1657,7 +1657,7 @@ pub async fn do_knowledge_set(out: &output::Out, config: Config, args: &ArgMatch } pub async fn do_knowledge_delete(out: &output::Out, config: Config, args: &ArgMatches) { - beta::notify_once(out, &beta::STORAGE); + beta::STORAGE.notify_once(); let catalog = args .get_one::("catalog_name") diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index cb395739..2758e8f8 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -616,8 +616,23 @@ pub fn background_error(msg: &str) { write_to_stderr(&format!("{} {}\n", "Oh no!".red(), msg)); } +/// Writes a labelled notice to stderr once per user, ever. The once-per-user +/// claim is only spent when stderr is a terminal, so a script, MCP capture, or +/// CI run can't use it up on a notice nobody saw. +pub(crate) fn notice_once(id: &str, label: &str, msg: &str) { + if !io::stderr().is_terminal() { + return; + } + + match config::claim_notice(id) { + Ok(true) => notice_to_stderr(label, msg), + Ok(false) => {} + Err(err) => debug!("Failed to persist CLI notice {}: {}", id, err), + } +} + /// Writes a labelled notice to stderr, keeping stdout clean for command output. -pub(crate) fn notice_to_stderr(label: &str, msg: &str) { +fn notice_to_stderr(label: &str, msg: &str) { let line = format!("{} {}\n", label.bold().yellow(), msg); write_to_stderr(&line); }