From 24eb7193e8f4f9bd04251cf0d1ea2702db41b590 Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Thu, 26 Feb 2026 12:07:17 +0100 Subject: [PATCH 1/6] feat(params): add secret parameter support (#210) * feat(params): add secret parameter support * Rename secret parameter field to hidden --- crates/config/src/towerfile.rs | 24 ++++++++++++++++++++ crates/tower-api/src/models/parameter.rs | 4 ++++ crates/tower-api/src/models/run_parameter.rs | 9 +++++++- crates/tower-cmd/src/api.rs | 12 ++++++++-- crates/tower-cmd/src/mcp.rs | 17 +++++++++++++- crates/tower/src/lib.rs | 11 ++++----- 6 files changed, 67 insertions(+), 10 deletions(-) diff --git a/crates/config/src/towerfile.rs b/crates/config/src/towerfile.rs index aa385e1a..703c999d 100644 --- a/crates/config/src/towerfile.rs +++ b/crates/config/src/towerfile.rs @@ -12,6 +12,9 @@ pub struct Parameter { #[serde(default)] pub default: String, + + #[serde(default)] + pub hidden: bool, } #[derive(Deserialize, Serialize, Debug)] @@ -122,6 +125,7 @@ impl Towerfile { name, description, default, + hidden: false, }); } } @@ -272,6 +276,26 @@ mod test { assert_eq!(towerfile.parameters.len(), 2); assert_eq!(towerfile.parameters[0].name, "my_first_param"); assert_eq!(towerfile.parameters[1].name, "my_second_param"); + assert!(!towerfile.parameters[0].hidden); + } + + #[test] + fn test_parses_secret_parameters() { + let toml = r#" + [app] + name = "my-app" + script = "./script.py" + source = ["*.py"] + + [[parameters]] + name = "MY_PARAMETER" + hidden = true + "#; + + let towerfile = crate::Towerfile::from_toml(toml).unwrap(); + assert_eq!(towerfile.parameters.len(), 1); + assert_eq!(towerfile.parameters[0].name, "MY_PARAMETER"); + assert!(towerfile.parameters[0].hidden); } #[test] diff --git a/crates/tower-api/src/models/parameter.rs b/crates/tower-api/src/models/parameter.rs index ce85288d..f53c7b45 100644 --- a/crates/tower-api/src/models/parameter.rs +++ b/crates/tower-api/src/models/parameter.rs @@ -23,6 +23,9 @@ pub struct Parameter { #[serde_as(as = "DefaultOnNull")] #[serde(rename = "name")] pub name: String, + #[serde(default)] + #[serde(rename = "hidden")] + pub hidden: bool, } impl Parameter { @@ -31,6 +34,7 @@ impl Parameter { default, description, name, + hidden: false, } } } diff --git a/crates/tower-api/src/models/run_parameter.rs b/crates/tower-api/src/models/run_parameter.rs index 7e9164d4..f45cc23d 100644 --- a/crates/tower-api/src/models/run_parameter.rs +++ b/crates/tower-api/src/models/run_parameter.rs @@ -20,10 +20,17 @@ pub struct RunParameter { #[serde_as(as = "DefaultOnNull")] #[serde(rename = "value")] pub value: String, + #[serde(default)] + #[serde(rename = "hidden")] + pub hidden: bool, } impl RunParameter { pub fn new(name: String, value: String) -> RunParameter { - RunParameter { name, value } + RunParameter { + name, + value, + hidden: false, + } } } diff --git a/crates/tower-cmd/src/api.rs b/crates/tower-cmd/src/api.rs index f3ddc421..e4024606 100644 --- a/crates/tower-cmd/src/api.rs +++ b/crates/tower-cmd/src/api.rs @@ -840,7 +840,11 @@ pub async fn create_schedule( let run_parameters = parameters.map(|params| { params .into_iter() - .map(|(key, value)| RunParameter { name: key, value }) + .map(|(key, value)| RunParameter { + name: key, + value, + hidden: false, + }) .collect() }); @@ -874,7 +878,11 @@ pub async fn update_schedule( let run_parameters = parameters.map(|params| { params .into_iter() - .map(|(key, value)| RunParameter { name: key, value }) + .map(|(key, value)| RunParameter { + name: key, + value, + hidden: false, + }) .collect() }); diff --git a/crates/tower-cmd/src/mcp.rs b/crates/tower-cmd/src/mcp.rs index 95d9440d..efe7e2a0 100644 --- a/crates/tower-cmd/src/mcp.rs +++ b/crates/tower-cmd/src/mcp.rs @@ -889,7 +889,22 @@ IMPORTANT REMINDERS: async fn tower_schedules_list(&self) -> Result { match api::list_schedules(&self.config, None, None).await { Ok(response) => { - Self::json_success(serde_json::json!({"schedules": response.schedules})) + let schedules = response + .schedules + .into_iter() + .map(|mut schedule| { + if let Some(parameters) = schedule.parameters.as_mut() { + for parameter in parameters { + if parameter.hidden { + parameter.value = "[hidden]".to_string(); + } + } + } + schedule + }) + .collect::>(); + + Self::json_success(serde_json::json!({"schedules": schedules})) } Err(e) => Self::error_result("Failed to list schedules", e), } diff --git a/crates/tower/src/lib.rs b/crates/tower/src/lib.rs index 751f4ba6..1549e234 100644 --- a/crates/tower/src/lib.rs +++ b/crates/tower/src/lib.rs @@ -23,8 +23,8 @@ mod bindings { let spec = PackageSpec::from_towerfile(&towerfile); - let runtime = tokio::runtime::Runtime::new() - .map_err(|e| PyRuntimeError::new_err(e.to_string()))?; + let runtime = + tokio::runtime::Runtime::new().map_err(|e| PyRuntimeError::new_err(e.to_string()))?; let output = PathBuf::from(output); @@ -40,8 +40,7 @@ mod bindings { .as_ref() .ok_or_else(|| PyRuntimeError::new_err("package build produced no output file"))?; - std::fs::copy(src, &output) - .map_err(|e| PyRuntimeError::new_err(e.to_string()))?; + std::fs::copy(src, &output).map_err(|e| PyRuntimeError::new_err(e.to_string()))?; Ok(()) }) @@ -53,8 +52,8 @@ mod bindings { /// args: Command line arguments (typically sys.argv). #[pyfunction] fn _run_cli(args: Vec) -> PyResult<()> { - let runtime = tokio::runtime::Runtime::new() - .map_err(|e| PyRuntimeError::new_err(e.to_string()))?; + let runtime = + tokio::runtime::Runtime::new().map_err(|e| PyRuntimeError::new_err(e.to_string()))?; // App::new_from_args() must run inside block_on because // Session::from_config_dir() requires an active tokio reactor. From 58456c5456b315064e9bc45dd9be36a6705169b4 Mon Sep 17 00:00:00 2001 From: Burak Dede Date: Thu, 26 Feb 2026 12:08:34 +0100 Subject: [PATCH 2/6] fix(schedules): restore direct update semantics and strengthen parsing tests (#205) --- crates/tower-cmd/src/api.rs | 5 +- crates/tower-cmd/src/schedules.rs | 207 +++++++++++++++++++++++++++--- 2 files changed, 192 insertions(+), 20 deletions(-) diff --git a/crates/tower-cmd/src/api.rs b/crates/tower-cmd/src/api.rs index e4024606..9a3ffaf9 100644 --- a/crates/tower-cmd/src/api.rs +++ b/crates/tower-cmd/src/api.rs @@ -889,7 +889,10 @@ pub async fn update_schedule( let params = tower_api::apis::default_api::UpdateScheduleParams { id_or_name: schedule_id.to_string(), update_schedule_params: tower_api::models::UpdateScheduleParams { - cron: cron.map(|s| s.clone()), + schema: None, + cron: cron.cloned(), + environment: None, + app_version: None, parameters: run_parameters, ..Default::default() }, diff --git a/crates/tower-cmd/src/schedules.rs b/crates/tower-cmd/src/schedules.rs index aec97162..56a06d70 100644 --- a/crates/tower-cmd/src/schedules.rs +++ b/crates/tower-cmd/src/schedules.rs @@ -71,13 +71,26 @@ pub fn schedules_cmd() -> Command { ) .subcommand( Command::new("delete") - .allow_external_subcommands(true) + .arg( + Arg::new("schedule_id") + .required(true) + .value_parser(value_parser!(String)) + .help("The schedule ID to delete") + .action(clap::ArgAction::Set), + ) .override_usage("tower schedules delete [OPTIONS] ") .after_help("Example: tower schedules delete 123") .about("Delete a schedule"), ) .subcommand( Command::new("update") + .arg( + Arg::new("schedule_id") + .required(true) + .value_parser(value_parser!(String)) + .help("The schedule ID to update") + .action(clap::ArgAction::Set), + ) .arg( Arg::new("cron") .short('c') @@ -93,7 +106,6 @@ pub fn schedules_cmd() -> Command { .help("Parameters (key=value) to pass to the app") .action(clap::ArgAction::Append), ) - .allow_external_subcommands(true) .override_usage("tower schedules update [OPTIONS] ") .after_help("Example: tower schedules update 123 --cron \"*/15 * * * *\"") .about("Update an existing schedule"), @@ -164,13 +176,13 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } pub async fn do_update(config: Config, args: &ArgMatches) { - let schedule_id = extract_schedule_id("update", args.subcommand()); + let schedule_id = args.get_one::("schedule_id").unwrap(); let cron = args.get_one::("cron"); let parameters = parse_parameters(args); output::with_spinner( "Updating schedule", - api::update_schedule(&config, &schedule_id, cron, parameters), + api::update_schedule(&config, schedule_id, cron, parameters), ) .await; @@ -178,29 +190,17 @@ pub async fn do_update(config: Config, args: &ArgMatches) { } pub async fn do_delete(config: Config, args: &ArgMatches) { - let schedule_id = extract_schedule_id("delete", args.subcommand()); + let schedule_id = args.get_one::("schedule_id").unwrap(); output::with_spinner( "Deleting schedule", - api::delete_schedule(&config, &schedule_id), + api::delete_schedule(&config, schedule_id), ) .await; output::success(&format!("Schedule {} deleted", schedule_id)); } -fn extract_schedule_id(subcmd: &str, cmd: Option<(&str, &ArgMatches)>) -> String { - if let Some((id, _)) = cmd { - return id.to_string(); - } - - let line = format!( - "Schedule ID is required. Example: tower schedules {} ", - subcmd - ); - output::die(&line); -} - /// Parses `--parameter` arguments into a HashMap of key-value pairs. /// Handles format like "--parameter key=value" fn parse_parameters(args: &ArgMatches) -> Option> { @@ -228,8 +228,177 @@ fn parse_parameters(args: &ArgMatches) -> Option> { } } - Some(param_map) + if param_map.is_empty() { + None + } else { + Some(param_map) + } } else { None } } + +#[cfg(test)] +mod tests { + use super::{parse_parameters, schedules_cmd}; + + #[test] + fn update_accepts_positional_schedule_id_and_flags() { + let matches = schedules_cmd() + .try_get_matches_from([ + "schedules", + "update", + "sch_123", + "--cron", + "*/10 * * * *", + "--parameter", + "env=prod", + "-p", + "team=platform", + ]) + .expect("update args should parse"); + + let ("update", update_args) = matches.subcommand().expect("expected update subcommand") + else { + panic!("expected update subcommand"); + }; + + assert_eq!( + update_args + .get_one::("schedule_id") + .map(String::as_str), + Some("sch_123") + ); + assert_eq!( + update_args.get_one::("cron").map(String::as_str), + Some("*/10 * * * *") + ); + + let params: Vec<&str> = update_args + .get_many::("parameters") + .expect("expected parameters") + .map(String::as_str) + .collect(); + assert_eq!(params, vec!["env=prod", "team=platform"]); + } + + #[test] + fn update_accepts_equals_sign_flag_forms() { + let matches = schedules_cmd() + .try_get_matches_from([ + "schedules", + "update", + "sch_456", + "--cron=*/5 * * * *", + "--parameter=region=us-east-1", + ]) + .expect("equals-form args should parse"); + + let ("update", update_args) = matches.subcommand().expect("expected update subcommand") + else { + panic!("expected update subcommand"); + }; + + assert_eq!( + update_args + .get_one::("schedule_id") + .map(String::as_str), + Some("sch_456") + ); + assert_eq!( + update_args.get_one::("cron").map(String::as_str), + Some("*/5 * * * *") + ); + assert_eq!( + update_args + .get_many::("parameters") + .expect("expected parameter") + .next() + .map(String::as_str), + Some("region=us-east-1") + ); + } + + #[test] + fn update_requires_schedule_id() { + let result = + schedules_cmd().try_get_matches_from(["schedules", "update", "--cron", "*/15 * * * *"]); + assert!(result.is_err()); + } + + #[test] + fn delete_requires_schedule_id() { + let result = schedules_cmd().try_get_matches_from(["schedules", "delete"]); + assert!(result.is_err()); + } + + #[test] + fn parse_parameters_valid_pairs() { + let matches = schedules_cmd() + .try_get_matches_from([ + "schedules", + "update", + "sch_789", + "--parameter", + "env=prod", + "-p", + "team=platform", + ]) + .expect("update args should parse"); + + let ("update", update_args) = matches.subcommand().expect("expected update subcommand") + else { + panic!("expected update subcommand"); + }; + + let params = parse_parameters(update_args).expect("expected parsed parameters"); + assert_eq!(params.get("env"), Some(&"prod".to_string())); + assert_eq!(params.get("team"), Some(&"platform".to_string())); + } + + #[test] + fn parse_parameters_invalid_entries_return_none() { + let matches = schedules_cmd() + .try_get_matches_from([ + "schedules", + "update", + "sch_789", + "--parameter", + "invalid", + "-p", + "=missing-key", + ]) + .expect("update args should parse"); + + let ("update", update_args) = matches.subcommand().expect("expected update subcommand") + else { + panic!("expected update subcommand"); + }; + + assert_eq!(parse_parameters(update_args), None); + } + + #[test] + fn parse_parameters_mixed_valid_and_invalid_keeps_valid() { + let matches = schedules_cmd() + .try_get_matches_from([ + "schedules", + "update", + "sch_789", + "--parameter", + "env=prod", + "-p", + "invalid", + ]) + .expect("update args should parse"); + + let ("update", update_args) = matches.subcommand().expect("expected update subcommand") + else { + panic!("expected update subcommand"); + }; + + let params = parse_parameters(update_args).expect("expected parsed parameters"); + assert_eq!(params.get("env"), Some(&"prod".to_string())); + assert_eq!(params.len(), 1); + } +} From b9e28716a43a66975eeb386e689dbe369361a7a6 Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Thu, 26 Feb 2026 13:41:00 +0100 Subject: [PATCH 3/6] Restore Ctrl+C behavior when running CLI via Python entrypoint (#211) --- src/tower/cli.py | 6 ++++++ tests/tower/test_cli.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/tower/test_cli.py diff --git a/src/tower/cli.py b/src/tower/cli.py index d846ac4a..30272891 100644 --- a/src/tower/cli.py +++ b/src/tower/cli.py @@ -1,7 +1,13 @@ +import signal import sys from ._native import _run_cli def main(): + # When invoked via Python, SIGINT defaults to raising KeyboardInterrupt + # in the interpreter instead of terminating the process immediately. + # Restore default SIGINT behavior so Ctrl+C reliably cancels interactive + # CLI flows (matching behavior of the standalone Rust binary). + signal.signal(signal.SIGINT, signal.SIG_DFL) _run_cli(sys.argv) diff --git a/tests/tower/test_cli.py b/tests/tower/test_cli.py new file mode 100644 index 00000000..948d4d97 --- /dev/null +++ b/tests/tower/test_cli.py @@ -0,0 +1,24 @@ +import signal + +import tower.cli + + +def test_main_restores_default_sigint_and_calls_native(monkeypatch): + captured = {} + + def fake_signal(sig, handler): + captured["sig"] = sig + captured["handler"] = handler + + def fake_run_cli(argv): + captured["argv"] = argv + + monkeypatch.setattr(tower.cli.signal, "signal", fake_signal) + monkeypatch.setattr(tower.cli, "_run_cli", fake_run_cli) + + tower.cli.main() + + assert captured["sig"] == signal.SIGINT + assert captured["handler"] == signal.SIG_DFL + assert isinstance(captured["argv"], list) + assert len(captured["argv"]) > 0 From af8ab6597f442645b9d255867bd7c10795b2779f Mon Sep 17 00:00:00 2001 From: Burak Dede Date: Thu, 26 Feb 2026 13:41:12 +0100 Subject: [PATCH 4/6] fix: avoid panic when command output is piped (#206) --- crates/tower-cmd/src/output.rs | 38 +++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/crates/tower-cmd/src/output.rs b/crates/tower-cmd/src/output.rs index 272a31da..b4b6eb46 100644 --- a/crates/tower-cmd/src/output.rs +++ b/crates/tower-cmd/src/output.rs @@ -46,6 +46,28 @@ impl OutputMode { static OUTPUT_MODE: OnceLock = OnceLock::new(); static CURRENT_SENDER: Mutex>> = Mutex::new(None); +fn write_to_stdout(msg: &str) { + let mut stdout = io::stdout(); + if let Err(err) = stdout.write_all(msg.as_bytes()) { + if err.kind() == io::ErrorKind::BrokenPipe { + std::process::exit(0); + } + panic!("failed writing to stdout: {err}"); + } + stdout.flush().ok(); +} + +fn write_to_stderr(msg: &str) { + let mut stderr = io::stderr(); + if let Err(err) = stderr.write_all(msg.as_bytes()) { + if err.kind() == io::ErrorKind::BrokenPipe { + std::process::exit(0); + } + panic!("failed writing to stderr: {err}"); + } + stderr.flush().ok(); +} + pub fn set_output_mode(mode: OutputMode) { OUTPUT_MODE.set(mode).ok(); if mode.is_mcp() { @@ -210,8 +232,7 @@ pub fn write(msg: &str) { let clean_msg = msg.trim_end().to_string(); send_to_current_sender(clean_msg); } else { - io::stdout().write_all(msg.as_bytes()).unwrap(); - io::stdout().flush().ok(); + write_to_stdout(msg); } } @@ -433,7 +454,12 @@ pub fn table(headers: Vec, data: Vec>, json_da .separator(separator) .title(headers.iter().map(|h| h.yellow().to_string())); - print_stdout(table).unwrap(); + if let Err(err) = print_stdout(table) { + if err.kind() == io::ErrorKind::BrokenPipe { + std::process::exit(0); + } + panic!("failed writing table to stdout: {err}"); + } } } @@ -513,8 +539,7 @@ pub fn write_update_available_message(latest: &str, current: &str) { ); // Always write version check messages to stderr to avoid polluting stdout - use std::io::{self, Write}; - io::stderr().write_all(line.as_bytes()).unwrap(); + write_to_stderr(&line); } pub fn write_dev_version_message(current: &str, latest: &str) { @@ -527,8 +552,7 @@ pub fn write_dev_version_message(current: &str, latest: &str) { .dimmed() ); - use std::io::{self, Write}; - io::stderr().write_all(line.as_bytes()).unwrap(); + write_to_stderr(&line); } /// newline just outputs a newline. This is useful when you have a very specific formatting you From 198783e21d867dcb7cc5878dea44557d91596adc Mon Sep 17 00:00:00 2001 From: Brad Heller Date: Thu, 26 Feb 2026 13:44:55 +0100 Subject: [PATCH 5/6] Bump version to v0.3.50 --- Cargo.lock | 22 +++++++++++----------- Cargo.toml | 2 +- pyproject.toml | 2 +- uv.lock | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd8bd32e..0fea61cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -491,7 +491,7 @@ dependencies = [ [[package]] name = "config" -version = "0.3.49" +version = "0.3.50" dependencies = [ "base64", "chrono", @@ -598,7 +598,7 @@ checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" [[package]] name = "crypto" -version = "0.3.49" +version = "0.3.50" dependencies = [ "aes-gcm", "base64", @@ -3316,7 +3316,7 @@ dependencies = [ [[package]] name = "testutils" -version = "0.3.49" +version = "0.3.50" dependencies = [ "pem", "rsa", @@ -3586,7 +3586,7 @@ checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" [[package]] name = "tower" -version = "0.3.49" +version = "0.3.50" dependencies = [ "config", "pyo3", @@ -3614,7 +3614,7 @@ dependencies = [ [[package]] name = "tower-api" -version = "0.3.49" +version = "0.3.50" dependencies = [ "reqwest", "serde", @@ -3626,7 +3626,7 @@ dependencies = [ [[package]] name = "tower-cmd" -version = "0.3.49" +version = "0.3.50" dependencies = [ "axum", "bytes", @@ -3696,7 +3696,7 @@ checksum = "121c2a6cda46980bb0fcd1647ffaf6cd3fc79a013de288782836f6df9c48780e" [[package]] name = "tower-package" -version = "0.3.49" +version = "0.3.50" dependencies = [ "async-compression", "config", @@ -3714,7 +3714,7 @@ dependencies = [ [[package]] name = "tower-runtime" -version = "0.3.49" +version = "0.3.50" dependencies = [ "async-trait", "chrono", @@ -3737,7 +3737,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3" [[package]] name = "tower-telemetry" -version = "0.3.49" +version = "0.3.50" dependencies = [ "tracing", "tracing-appender", @@ -3746,7 +3746,7 @@ dependencies = [ [[package]] name = "tower-uv" -version = "0.3.49" +version = "0.3.50" dependencies = [ "async-compression", "async_zip", @@ -3764,7 +3764,7 @@ dependencies = [ [[package]] name = "tower-version" -version = "0.3.49" +version = "0.3.50" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 78784617..011288e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.3.49" +version = "0.3.50" description = "Tower is the best way to host Python data apps in production" rust-version = "1.81" authors = ["Brad Heller "] diff --git a/pyproject.toml b/pyproject.toml index 6d669d5b..b0a8f99d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "tower" -version = "0.3.49" +version = "0.3.50" description = "Tower CLI and runtime environment for Tower." authors = [{ name = "Tower Computing Inc.", email = "brad@tower.dev" }] readme = "README.md" diff --git a/uv.lock b/uv.lock index 04cbf795..59cb1d32 100644 --- a/uv.lock +++ b/uv.lock @@ -2488,7 +2488,7 @@ wheels = [ [[package]] name = "tower" -version = "0.3.49" +version = "0.3.50" source = { editable = "." } dependencies = [ { name = "attrs" }, From 3c58d471fafdd70042f6e5ff308514912de58708 Mon Sep 17 00:00:00 2001 From: Ben Lovell Date: Fri, 27 Feb 2026 12:58:55 +0100 Subject: [PATCH 6/6] feat: return to first class clap handling (#213) Specifically to improve error messages for run params, and to give guidance for what to do when something goes wrong. Additionally, moving back to clap means we get stuff like still working for free (this was confusing claude, and also users). --- crates/tower-cmd/src/apps.rs | 112 +++++++++-------- crates/tower-cmd/src/lib.rs | 2 +- crates/tower-cmd/src/mcp.rs | 4 +- crates/tower-cmd/src/run.rs | 116 ++++++++++++++---- crates/tower-cmd/src/schedules.rs | 28 ++--- crates/tower-cmd/src/secrets.rs | 46 +++---- crates/tower-cmd/src/teams.rs | 27 ++-- tests/integration/features/cli_runs.feature | 4 +- tests/integration/features/steps/cli_steps.py | 15 +++ 9 files changed, 227 insertions(+), 127 deletions(-) diff --git a/crates/tower-cmd/src/apps.rs b/crates/tower-cmd/src/apps.rs index 31652eea..319ded8a 100644 --- a/crates/tower-cmd/src/apps.rs +++ b/crates/tower-cmd/src/apps.rs @@ -14,13 +14,29 @@ pub fn apps_cmd() -> Command { .subcommand(Command::new("list").about("List all of your apps")) .subcommand( Command::new("show") - .allow_external_subcommands(true) - .override_usage("tower apps show [OPTIONS] ") - .after_help("Example: tower apps show hello-world") + .arg( + Arg::new("app_name") + .value_parser(value_parser!(String)) + .index(1) + .required(true) + .help("Name of the app"), + ) .about("Show the details about an app in Tower"), ) .subcommand( Command::new("logs") + .arg( + Arg::new("app_name") + .value_parser(value_parser!(String)) + .index(1) + .required(true) + .help("app_name#run_number"), + ) + .arg( + Arg::new("run_number") + .value_parser(value_parser!(i64)) + .index(2), + ) .arg( Arg::new("follow") .short('f') @@ -28,9 +44,6 @@ pub fn apps_cmd() -> Command { .help("Follow logs in real time") .action(clap::ArgAction::SetTrue), ) - .allow_external_subcommands(true) - .override_usage("tower apps logs [OPTIONS] #") - .after_help("Example: tower apps logs hello-world#11") .about("Get the logs from a previous Tower app run"), ) .subcommand( @@ -54,15 +67,29 @@ pub fn apps_cmd() -> Command { ) .subcommand( Command::new("delete") - .allow_external_subcommands(true) - .override_usage("tower apps delete [OPTIONS] ") - .after_help("Example: tower apps delete hello-world") + .arg( + Arg::new("app_name") + .value_parser(value_parser!(String)) + .index(1) + .required(true) + .help("Name of the app"), + ) .about("Delete an app in Tower"), ) } pub async fn do_logs(config: Config, cmd: &ArgMatches) { - let (name, seq) = extract_app_name_and_run("logs", cmd.subcommand()); + let app_name_raw = cmd.get_one::("app_name").expect("app_name is required"); + let (name, seq) = if let Some((name, num_str)) = app_name_raw.split_once('#') { + let num = num_str.parse::().unwrap_or_else(|_| output::die("Run number must be a number")); + (name.to_string(), num) + } else { + let num = match cmd.get_one::("run_number").copied() { + Some(n) => n, + None => latest_run_number(&config, app_name_raw).await, + }; + (app_name_raw.clone(), num) + }; let follow = cmd.get_one::("follow").copied().unwrap_or(false); if follow { @@ -78,7 +105,7 @@ pub async fn do_logs(config: Config, cmd: &ArgMatches) { } pub async fn do_show(config: Config, cmd: &ArgMatches) { - let name = extract_app_name("show", cmd.subcommand()); + let name = cmd.get_one::("app_name").expect("app_name is required"); match api::describe_app(&config, &name).await { Ok(app_response) => { @@ -191,46 +218,20 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } pub async fn do_delete(config: Config, cmd: &ArgMatches) { - let name = extract_app_name("delete", cmd.subcommand()); + let name = cmd.get_one::("app_name").expect("app_name is required"); - output::with_spinner("Deleting app", api::delete_app(&config, &name)).await; + output::with_spinner("Deleting app", api::delete_app(&config, name)).await; } -/// Extract app name and run number from command -fn extract_app_name_and_run(subcmd: &str, cmd: Option<(&str, &ArgMatches)>) -> (String, i64) { - if let Some((name, _)) = cmd { - if let Some((name, num)) = name.split_once('#') { - return ( - name.to_string(), - num.parse::().unwrap_or_else(|_| { - output::die("Run number must be an actual number"); - }), - ); - } - - let line = format!( - "Run number is required. Example: tower apps {} #", - subcmd - ); - output::die(&line); - } - let line = format!( - "App name is required. Example: tower apps {} #", - subcmd - ); - output::die(&line) -} - -fn extract_app_name(subcmd: &str, cmd: Option<(&str, &ArgMatches)>) -> String { - if let Some((name, _)) = cmd { - return name.to_string(); +async fn latest_run_number(config: &Config, name: &str) -> i64 { + match api::describe_app(config, name).await { + Ok(resp) => resp.runs + .iter() + .map(|r| r.number) + .max() + .unwrap_or_else(|| output::die(&format!("No runs found for app '{}'", name))), + Err(err) => output::tower_error_and_die(err, "Fetching app details failed"), } - - let line = format!( - "App name is required. Example: tower apps {} ", - subcmd - ); - output::die(&line); } const FOLLOW_BACKOFF_INITIAL: Duration = Duration::from_millis(500); @@ -539,9 +540,24 @@ mod tests { assert_eq!(cmd, "logs"); assert_eq!(sub_matches.get_one::("follow"), Some(&true)); assert_eq!( - sub_matches.subcommand().map(|(name, _)| name), + sub_matches.get_one::("app_name").map(|s| s.as_str()), Some("hello-world#11") ); + assert_eq!(sub_matches.get_one::("run_number"), None); + } + + #[test] + fn test_separate_run_number_parsing() { + let matches = apps_cmd() + .try_get_matches_from(["apps", "logs", "hello-world", "11"]) + .unwrap(); + let (_, sub_matches) = matches.subcommand().unwrap(); + + assert_eq!( + sub_matches.get_one::("app_name").map(|s| s.as_str()), + Some("hello-world") + ); + assert_eq!(sub_matches.get_one::("run_number"), Some(&11)); } #[test] diff --git a/crates/tower-cmd/src/lib.rs b/crates/tower-cmd/src/lib.rs index 864fc34f..a1086e28 100644 --- a/crates/tower-cmd/src/lib.rs +++ b/crates/tower-cmd/src/lib.rs @@ -172,7 +172,7 @@ impl App { } Some(("deploy", args)) => deploy::do_deploy(sessionized_config, args).await, Some(("package", args)) => package::do_package(sessionized_config, args).await, - Some(("run", args)) => run::do_run(sessionized_config, args, args.subcommand()).await, + Some(("run", args)) => run::do_run(sessionized_config, args).await, Some(("teams", sub_matches)) => { let teams_command = sub_matches.subcommand(); diff --git a/crates/tower-cmd/src/mcp.rs b/crates/tower-cmd/src/mcp.rs index efe7e2a0..ad6043ae 100644 --- a/crates/tower-cmd/src/mcp.rs +++ b/crates/tower-cmd/src/mcp.rs @@ -651,7 +651,7 @@ impl TowerService { } #[tool( - description = "Run on Tower cloud. Prerequisites: Towerfile, tower_apps_create, tower_deploy." + description = "Run an app on Tower cloud. Pass parameters as a JSON object, e.g. {\"key\": \"value\"}. Prerequisites: Towerfile, tower_apps_create, tower_deploy." )] async fn tower_run_remote( &self, @@ -983,6 +983,8 @@ Rules: - Use tower_file_update/add_parameter to modify Towerfiles (never edit TOML directly) - DO NOT add hatchling/setuptools to pyproject.toml - Tower handles deployment - Tower apps need: pyproject.toml (deps only), Python code, Towerfile +- Pass run parameters as a JSON object {\"key\": \"value\"}, not as CLI flags +- There is no --param or --params flag Use tower_workflow_help for the complete workflow.".to_string()), } diff --git a/crates/tower-cmd/src/run.rs b/crates/tower-cmd/src/run.rs index bec58f5a..b0a79ea7 100644 --- a/crates/tower-cmd/src/run.rs +++ b/crates/tower-cmd/src/run.rs @@ -27,7 +27,19 @@ use tower_runtime::subprocess::SubprocessBackend; pub fn run_cmd() -> Command { Command::new("run") - .allow_external_subcommands(true) + .after_help( + "Examples:\n \ + tower run Run app from ./Towerfile (remote)\n \ + tower run --local Run app from ./Towerfile (local)\n \ + tower run my-app Run a deployed app by name\n \ + tower run -p key=value Pass a parameter to the run\n \ + tower run my-app -p key=value Run a named app with parameters", + ) + .arg( + Arg::new("app_name") + .help("Name of a deployed app to run (uses ./Towerfile if omitted)") + .index(1), + ) .arg( Arg::new("dir") .long("dir") @@ -65,11 +77,26 @@ pub fn run_cmd() -> Command { .about("Run your code in Tower or locally") } -pub async fn do_run(config: Config, args: &ArgMatches, cmd: Option<(&str, &ArgMatches)>) { - if let Err(e) = do_run_inner(config, args, cmd).await { +pub async fn do_run(config: Config, args: &ArgMatches) { + if let Err(e) = do_run_inner(config, args).await { match e { - Error::ApiRunError { source } => { - output::tower_error_and_die(source, "Scheduling run failed"); + Error::ApiRunError { ref source } => { + let is_not_found = matches!( + source, + tower_api::apis::Error::ResponseError(resp) if resp.status == reqwest::StatusCode::NOT_FOUND + ); + if is_not_found { + output::error("App not found. It may not exist or hasn't been deployed yet."); + output::write("\nTo fix this:\n"); + output::write(" 1. Check your app exists: tower apps list\n"); + output::write(" 2. Deploy your app: tower deploy\n"); + output::write(" 3. Then run it: tower run\n"); + std::process::exit(1); + } + if let Error::ApiRunError { source } = e { + output::tower_error_and_die(source, "Scheduling run failed"); + } + unreachable!(); } _ => { output::die(&e.to_string()); @@ -83,9 +110,8 @@ pub async fn do_run(config: Config, args: &ArgMatches, cmd: Option<(&str, &ArgMa pub async fn do_run_inner( config: Config, args: &ArgMatches, - cmd: Option<(&str, &ArgMatches)>, ) -> Result<(), Error> { - let res = get_run_parameters(args, cmd); + let res = get_run_parameters(args); // We always expect there to be an environment due to the fact that there is a // default value. @@ -467,18 +493,15 @@ fn handle_run_completion(res: Result) -> Result< } } -/// get_run_parameters takes care of all the hairy bits around digging about in the `clap` -/// internals to figure out what the user is requesting. In the end, it determines if we are meant -/// to do a local run or a remote run, and it determines the path to the relevant Towerfile that -/// should be loaded. +/// Extracts the local/remote flag, Towerfile directory, parameters, and optional app name +/// from the parsed CLI args. fn get_run_parameters( args: &ArgMatches, - cmd: Option<(&str, &ArgMatches)>, ) -> Result<(bool, PathBuf, HashMap, Option), config::Error> { let local = *args.get_one::("local").unwrap(); let path = resolve_path(args); let params = parse_parameters(args); - let app_name = get_app_name(cmd); + let app_name = args.get_one::("app_name").cloned(); Ok((local, path, params, app_name)) } @@ -528,14 +551,6 @@ fn resolve_path(args: &ArgMatches) -> PathBuf { } } -/// get_app_name is a helper function that will extract the app name from the `clap` arguments if -fn get_app_name(cmd: Option<(&str, &ArgMatches)>) -> Option { - match cmd { - Some((name, _)) if !name.is_empty() => Some(name.to_string()), - _ => None, - } -} - /// get_secrets_inner manages the process of getting secrets from the Tower server in a way that can be /// used by the local runtime during local app execution. Returns API errors for spinner handling. async fn get_secrets( @@ -799,3 +814,62 @@ fn monitor_run_completion(config: &Config, run: &Run) -> oneshot::Receiver rx } + +#[cfg(test)] +mod tests { + use super::run_cmd; + + fn parse(args: &[&str]) -> Result { + let mut full = vec!["run"]; + full.extend_from_slice(args); + run_cmd().try_get_matches_from(full) + } + + #[test] + fn app_name_parsed_as_positional_arg() { + let m = parse(&["my-app"]).unwrap(); + assert_eq!(m.get_one::("app_name").map(|s| s.as_str()), Some("my-app")); + } + + #[test] + fn no_app_name_is_fine() { + let m = parse(&[]).unwrap(); + assert_eq!(m.get_one::("app_name"), None); + } + + #[test] + fn unknown_flags_are_rejected() { + let err = parse(&["--param", "x=y"]).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("--param"), "should mention the bad flag: {msg}"); + assert!(msg.contains("--parameter"), "should suggest the correct flag: {msg}"); + } + + #[test] + fn help_flag_is_not_swallowed_as_app_name() { + let err = parse(&["--help"]).unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp); + } + + #[test] + fn help_after_app_name_still_shows_help() { + let err = parse(&["my-app", "--help"]).unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp); + } + + #[test] + fn parameters_after_app_name() { + let m = parse(&["my-app", "-p", "key=val"]).unwrap(); + assert_eq!(m.get_one::("app_name").map(|s| s.as_str()), Some("my-app")); + let params: Vec<&String> = m.get_many::("parameters").unwrap().collect(); + assert_eq!(params, vec!["key=val"]); + } + + #[test] + fn parameters_before_app_name() { + let m = parse(&["-p", "key=val", "my-app"]).unwrap(); + assert_eq!(m.get_one::("app_name").map(|s| s.as_str()), Some("my-app")); + let params: Vec<&String> = m.get_many::("parameters").unwrap().collect(); + assert_eq!(params, vec!["key=val"]); + } +} diff --git a/crates/tower-cmd/src/schedules.rs b/crates/tower-cmd/src/schedules.rs index 56a06d70..bfdd4d13 100644 --- a/crates/tower-cmd/src/schedules.rs +++ b/crates/tower-cmd/src/schedules.rs @@ -73,23 +73,22 @@ pub fn schedules_cmd() -> Command { Command::new("delete") .arg( Arg::new("schedule_id") - .required(true) .value_parser(value_parser!(String)) - .help("The schedule ID to delete") - .action(clap::ArgAction::Set), + .index(1) + .required(true) + .help("The schedule ID to delete"), ) - .override_usage("tower schedules delete [OPTIONS] ") .after_help("Example: tower schedules delete 123") .about("Delete a schedule"), ) .subcommand( Command::new("update") .arg( - Arg::new("schedule_id") - .required(true) + Arg::new("id_or_name") .value_parser(value_parser!(String)) - .help("The schedule ID to update") - .action(clap::ArgAction::Set), + .index(1) + .required(true) + .help("ID or name of the schedule to update"), ) .arg( Arg::new("cron") @@ -106,7 +105,6 @@ pub fn schedules_cmd() -> Command { .help("Parameters (key=value) to pass to the app") .action(clap::ArgAction::Append), ) - .override_usage("tower schedules update [OPTIONS] ") .after_help("Example: tower schedules update 123 --cron \"*/15 * * * *\"") .about("Update an existing schedule"), ) @@ -176,21 +174,21 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } pub async fn do_update(config: Config, args: &ArgMatches) { - let schedule_id = args.get_one::("schedule_id").unwrap(); + let id_or_name = args.get_one::("id_or_name").expect("id_or_name is required"); let cron = args.get_one::("cron"); let parameters = parse_parameters(args); output::with_spinner( "Updating schedule", - api::update_schedule(&config, schedule_id, cron, parameters), + api::update_schedule(&config, id_or_name, cron, parameters), ) .await; - output::success(&format!("Schedule {} updated", schedule_id)); + output::success(&format!("Schedule {} updated", id_or_name)); } pub async fn do_delete(config: Config, args: &ArgMatches) { - let schedule_id = args.get_one::("schedule_id").unwrap(); + let schedule_id = args.get_one::("schedule_id").expect("schedule_id is required"); output::with_spinner( "Deleting schedule", @@ -265,7 +263,7 @@ mod tests { assert_eq!( update_args - .get_one::("schedule_id") + .get_one::("id_or_name") .map(String::as_str), Some("sch_123") ); @@ -301,7 +299,7 @@ mod tests { assert_eq!( update_args - .get_one::("schedule_id") + .get_one::("id_or_name") .map(String::as_str), Some("sch_456") ); diff --git a/crates/tower-cmd/src/secrets.rs b/crates/tower-cmd/src/secrets.rs index ecd0b94a..71198856 100644 --- a/crates/tower-cmd/src/secrets.rs +++ b/crates/tower-cmd/src/secrets.rs @@ -73,9 +73,22 @@ pub fn secrets_cmd() -> Command { ) .subcommand( Command::new("delete") - .allow_external_subcommands(true) - .override_usage("tower secrets delete [OPTIONS] ") - .after_help("Example: tower secrets delete MY_API_KEY") + .arg( + Arg::new("secret_name") + .value_parser(value_parser!(String)) + .index(1) + .required(true) + .help("secret name, or environment/secret_name"), + ) + .arg( + Arg::new("environment") + .short('e') + .long("environment") + .default_value("default") + .value_parser(value_parser!(String)) + .help("environment to delete the secret from") + .action(clap::ArgAction::Set), + ) .about("Delete a secret in Tower"), ) } @@ -174,7 +187,13 @@ pub async fn do_create(config: Config, args: &ArgMatches) { } pub async fn do_delete(config: Config, args: &ArgMatches) { - let (environment, name) = extract_secret_environment_and_name("delete", args.subcommand()); + let secret_name_arg = args.get_one::("secret_name").expect("secret_name is required"); + let (environment, name) = if let Some((env, name)) = secret_name_arg.split_once('/') { + (env.to_string(), name.to_string()) + } else { + let env = args.get_one::("environment").expect("environment has default"); + (env.clone(), secret_name_arg.clone()) + }; debug!("deleting secret, environment={} name={}", environment, name); output::with_spinner( @@ -224,22 +243,3 @@ async fn encrypt_and_create_secret( .map_err(SecretCreationError::CreateFailed) } -fn extract_secret_environment_and_name( - subcmd: &str, - cmd: Option<(&str, &ArgMatches)>, -) -> (String, String) { - if let Some((slug, _)) = cmd { - if let Some((env, name)) = slug.split_once('/') { - return (env.to_string(), name.to_string()); - } - - let line = format!( - "Secret name is required. Example: tower secrets {} /", - subcmd - ); - output::die(&line); - } - - let line = format!("Secret name and environment is required. Example: tower secrets {} /", subcmd); - output::die(&line); -} diff --git a/crates/tower-cmd/src/teams.rs b/crates/tower-cmd/src/teams.rs index d631e532..1a5a9174 100644 --- a/crates/tower-cmd/src/teams.rs +++ b/crates/tower-cmd/src/teams.rs @@ -1,4 +1,4 @@ -use clap::{ArgMatches, Command}; +use clap::{value_parser, Arg, ArgMatches, Command}; use colored::*; use config::Config; @@ -11,7 +11,13 @@ pub fn teams_cmd() -> Command { .subcommand(Command::new("list").about("List teams you belong to")) .subcommand( Command::new("switch") - .allow_external_subcommands(true) + .arg( + Arg::new("team_name") + .value_parser(value_parser!(String)) + .index(1) + .required(true) + .help("Name of the team to switch to"), + ) .about("Switch context to a different team"), ) } @@ -82,18 +88,18 @@ pub async fn do_list(config: Config) { } pub async fn do_switch(config: Config, args: &ArgMatches) { - let name = extract_team_name("switch", args.subcommand()); + let name = args.get_one::("team_name").expect("team_name is required"); // Refresh the session first to ensure we have the latest teams data let session = refresh_session(&config).await; // Check if the provided team name exists in the refreshed session - let team = session.teams.iter().find(|team| team.name == name); + let team = session.teams.iter().find(|team| team.name == *name); match team { Some(team) => { // Team found, set it as active - match config.set_active_team_by_name(&name) { + match config.set_active_team_by_name(name) { Ok(_) => { output::success(&format!("Switched to team: {}", team.name)); } @@ -114,14 +120,3 @@ pub async fn do_switch(config: Config, args: &ArgMatches) { } } -fn extract_team_name(subcmd: &str, cmd: Option<(&str, &ArgMatches)>) -> String { - if let Some((name, _)) = cmd { - return name.to_string(); - } - - let line = format!( - "Team name is required. Example: tower teams {} ", - subcmd - ); - output::die(&line); -} diff --git a/tests/integration/features/cli_runs.feature b/tests/integration/features/cli_runs.feature index 4df64d6e..27f272b9 100644 --- a/tests/integration/features/cli_runs.feature +++ b/tests/integration/features/cli_runs.feature @@ -3,10 +3,10 @@ Feature: CLI Run Commands I want to run applications locally and remotely with proper feedback So that I can develop and deploy my applications effectively - Scenario: CLI remote run should show red error message when API fails + Scenario: CLI remote run should show helpful error when app not found Given I have a simple hello world application When I run "tower run" via CLI - Then the final crash status should show red "Error:" + Then the output should show "App not found" with guidance Scenario: CLI local run should have green timestamps and detect crashes Given I have a simple hello world application that exits with code 1 diff --git a/tests/integration/features/steps/cli_steps.py b/tests/integration/features/steps/cli_steps.py index 1d8151f5..e0b71da6 100644 --- a/tests/integration/features/steps/cli_steps.py +++ b/tests/integration/features/steps/cli_steps.py @@ -143,6 +143,21 @@ def step_final_crash_status_should_show_error(context): assert "Error:" in output, f"Expected 'Error:' in crash message, got: {output}" +@step('the output should show "App not found" with guidance') +def step_output_should_show_app_not_found_with_guidance(context): + """Verify 404 error shows helpful guidance instead of a bare error""" + output = context.cli_output + assert ( + red_color_code in output + ), f"Expected red color codes in output, got: {output}" + assert ( + "App not found" in output + ), f"Expected 'App not found' in output, got: {output}" + assert ( + "tower deploy" in output + ), f"Expected 'tower deploy' guidance in output, got: {output}" + + @step('the final status should show "Your local run crashed!" in red') def step_final_status_should_show_crashed_in_red(context): """Verify local run shows 'Your local run crashed!' in red"""