diff --git a/Cargo.lock b/Cargo.lock index feb86471..12d5a13c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -841,7 +841,7 @@ dependencies = [ [[package]] name = "config" -version = "0.3.75" +version = "0.3.76" dependencies = [ "base64", "chrono", @@ -1033,7 +1033,7 @@ checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" [[package]] name = "crypto" -version = "0.3.75" +version = "0.3.76" dependencies = [ "aes-gcm", "base64", @@ -4456,7 +4456,7 @@ dependencies = [ [[package]] name = "testutils" -version = "0.3.75" +version = "0.3.76" dependencies = [ "pem", "rsa", @@ -4765,7 +4765,7 @@ checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" [[package]] name = "tower" -version = "0.3.75" +version = "0.3.76" dependencies = [ "config", "pyo3", @@ -4793,7 +4793,7 @@ dependencies = [ [[package]] name = "tower-api" -version = "0.3.75" +version = "0.3.76" dependencies = [ "reqwest", "serde", @@ -4805,7 +4805,7 @@ dependencies = [ [[package]] name = "tower-cmd" -version = "0.3.75" +version = "0.3.76" dependencies = [ "axum", "bytes", @@ -4854,7 +4854,7 @@ dependencies = [ [[package]] name = "tower-duckdb" -version = "0.3.75" +version = "0.3.76" dependencies = [ "chrono", "duckdb", @@ -4889,7 +4889,7 @@ checksum = "121c2a6cda46980bb0fcd1647ffaf6cd3fc79a013de288782836f6df9c48780e" [[package]] name = "tower-package" -version = "0.3.75" +version = "0.3.76" dependencies = [ "async-compression", "flate2", @@ -4914,7 +4914,7 @@ dependencies = [ [[package]] name = "tower-runtime" -version = "0.3.75" +version = "0.3.76" dependencies = [ "async-trait", "chrono", @@ -4938,7 +4938,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3" [[package]] name = "tower-telemetry" -version = "0.3.75" +version = "0.3.76" dependencies = [ "tracing", "tracing-appender", @@ -4947,7 +4947,7 @@ dependencies = [ [[package]] name = "tower-uv" -version = "0.3.75" +version = "0.3.76" dependencies = [ "async-compression", "async_zip", @@ -4967,7 +4967,7 @@ dependencies = [ [[package]] name = "tower-version" -version = "0.3.75" +version = "0.3.76" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 07df3a0c..74e98363 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.3.75" +version = "0.3.76" description = "Tower is the best way to host Python data apps in production" # Matches rust-toolchain.toml. The two had drifted: the toolchain has been 1.88 # for a while, and the dependency tree (testcontainers and its transitive deps, diff --git a/crates/tower-cmd/src/catalogs.rs b/crates/tower-cmd/src/catalogs.rs index c53b30c9..d3427aa3 100644 --- a/crates/tower-cmd/src/catalogs.rs +++ b/crates/tower-cmd/src/catalogs.rs @@ -591,18 +591,19 @@ async fn execute_catalog_query( // legitimate analytical scan over a large catalog can take minutes and a // person is driving; the agent path uses `Limits::agent()`, which adds one. let harden = !write; - let result = tokio::task::spawn_blocking(move || -> Result { - let session = Session::open()?; - session.run_setup(&setup)?; - if harden { - // `Hardening::agent()` adds an engine memory ceiling on top of the - // lockdown. The result `limits` bound what comes back; only the engine - // can bound what a query spends producing it. - session.harden(&Hardening::agent())?; - } - session.query(&sql, [], &limits) - }) - .await; + let result = + tokio::task::spawn_blocking(move || -> Result { + let session = Session::open()?; + session.run_setup(&setup)?; + if harden { + // `Hardening::agent()` adds an engine memory ceiling on top of the + // lockdown. The result `limits` bound what comes back; only the engine + // can bound what a query spends producing it. + session.harden(&Hardening::agent())?; + } + session.query(&sql, [], &limits) + }) + .await; match result { Ok(Ok(query_result)) => { @@ -687,13 +688,14 @@ pub(crate) async fn query_catalog_for_agent( &response.credentials, vend_catalog_credentials_body::Mode::Read, ); - let result = tokio::task::spawn_blocking(move || -> Result { - let session = Session::open()?; - session.run_setup(&setup)?; - session.harden(&Hardening::agent())?; - session.query(&sql, [], &Limits::agent()) - }) - .await; + let result = + tokio::task::spawn_blocking(move || -> Result { + let session = Session::open()?; + session.run_setup(&setup)?; + session.harden(&Hardening::agent())?; + session.query(&sql, [], &Limits::agent()) + }) + .await; match result { Ok(Ok(query_result)) => Ok(query_result), @@ -844,7 +846,10 @@ async fn fetch_catalog_columns_via_rest( rows.sort_by(|a, b| { let key = |row: &[serde_json::Value]| { ( - row.first().and_then(|v| v.as_str()).unwrap_or("").to_owned(), + row.first() + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_owned(), row.get(1).and_then(|v| v.as_str()).unwrap_or("").to_owned(), ) }; @@ -1088,7 +1093,10 @@ fn iceberg_type_to_display(ty: &serde_json::Value) -> String { format!("LIST({})", element) } Some("map") => { - let key = obj.get("key").map(iceberg_type_to_display).unwrap_or_default(); + let key = obj + .get("key") + .map(iceberg_type_to_display) + .unwrap_or_default(); let value = obj .get("value") .map(iceberg_type_to_display) @@ -1592,9 +1600,7 @@ pub async fn do_knowledge_show(out: &output::Out, config: Config, args: &ArgMatc let catalog = args .get_one::("catalog_name") .expect("catalog_name is required"); - let name = args - .get_one::("name") - .expect("name is required"); + let name = args.get_one::("name").expect("name is required"); let env = cmd::get_string_flag(args, "environment"); let response = out @@ -1614,9 +1620,7 @@ pub async fn do_knowledge_set(out: &output::Out, config: Config, args: &ArgMatch let catalog = args .get_one::("catalog_name") .expect("catalog_name is required"); - let name = args - .get_one::("name") - .expect("name is required"); + let name = args.get_one::("name").expect("name is required"); let env = cmd::get_string_flag(args, "environment"); let statement = cmd::get_string_flag(args, "statement"); let scope = cmd::get_string_flag(args, "scope"); @@ -1662,9 +1666,7 @@ pub async fn do_knowledge_delete(out: &output::Out, config: Config, args: &ArgMa let catalog = args .get_one::("catalog_name") .expect("catalog_name is required"); - let name = args - .get_one::("name") - .expect("name is required"); + let name = args.get_one::("name").expect("name is required"); let env = cmd::get_string_flag(args, "environment"); out.with_spinner( @@ -2129,7 +2131,10 @@ mod tests { ]) .expect("query --max-rows should parse"); let (_, query_args) = matches.subcommand().expect("expected query subcommand"); - assert_eq!(query_args.get_one::("max_rows").copied(), Some(50_000)); + assert_eq!( + query_args.get_one::("max_rows").copied(), + Some(50_000) + ); assert!( catalogs_cmd() @@ -2298,15 +2303,14 @@ mod tests { .expect("knowledge delete should parse"); let (_, knowledge_args) = matches.subcommand().expect("expected knowledge subcommand"); - let (_, delete_args) = knowledge_args.subcommand().expect("expected delete subcommand"); + let (_, delete_args) = knowledge_args + .subcommand() + .expect("expected delete subcommand"); assert_eq!( delete_args.get_one::("catalog_name").unwrap(), "my-catalog" ); - assert_eq!( - delete_args.get_one::("name").unwrap(), - "my-entry" - ); + assert_eq!(delete_args.get_one::("name").unwrap(), "my-entry"); } #[test] @@ -2365,7 +2369,9 @@ mod tests { .expect("knowledge list should parse"); let (_, knowledge_args) = matches.subcommand().expect("expected knowledge subcommand"); - let (_, list_args) = knowledge_args.subcommand().expect("expected list subcommand"); + let (_, list_args) = knowledge_args + .subcommand() + .expect("expected list subcommand"); assert_eq!( list_args.get_one::("catalog_name").unwrap(), @@ -2453,7 +2459,9 @@ mod tests { .expect("knowledge set with all flags should parse"); let (_, knowledge_args) = matches.subcommand().expect("expected knowledge subcommand"); - let (_, set_args) = knowledge_args.subcommand().expect("expected set subcommand"); + let (_, set_args) = knowledge_args + .subcommand() + .expect("expected set subcommand"); assert_eq!(set_args.get_one::("scope").unwrap(), "column"); assert_eq!( @@ -2486,7 +2494,9 @@ mod tests { .expect("knowledge set should parse"); let (_, knowledge_args) = matches.subcommand().expect("expected knowledge subcommand"); - let (_, set_args) = knowledge_args.subcommand().expect("expected set subcommand"); + let (_, set_args) = knowledge_args + .subcommand() + .expect("expected set subcommand"); assert_eq!(set_args.get_one::("scope").unwrap(), "catalog"); assert_eq!( @@ -2500,8 +2510,13 @@ mod tests { #[test] fn knowledge_set_requires_statement() { - let result = - catalogs_cmd().try_get_matches_from(["catalogs", "knowledge", "set", "my-catalog", "f"]); + let result = catalogs_cmd().try_get_matches_from([ + "catalogs", + "knowledge", + "set", + "my-catalog", + "f", + ]); assert!(result.is_err()); } @@ -2515,7 +2530,9 @@ mod tests { .try_get_matches_from(["catalogs", "knowledge", "show", "my-catalog", "my-entry"]) .expect("knowledge show should parse"); let (_, knowledge_args) = matches.subcommand().expect("expected knowledge subcommand"); - let (_, show_args) = knowledge_args.subcommand().expect("expected show subcommand"); + let (_, show_args) = knowledge_args + .subcommand() + .expect("expected show subcommand"); assert_eq!(show_args.get_one::("name").unwrap(), "my-entry"); assert_eq!( show_args.get_one::("environment").unwrap(), @@ -2531,5 +2548,4 @@ mod tests { assert_eq!(truncated.chars().count(), 80); assert!(truncated.ends_with('…')); } - } diff --git a/crates/tower-cmd/src/lib.rs b/crates/tower-cmd/src/lib.rs index 0bb92756..d1af559d 100644 --- a/crates/tower-cmd/src/lib.rs +++ b/crates/tower-cmd/src/lib.rs @@ -154,24 +154,26 @@ impl App { Some(("query", args)) => { catalogs::do_query(&out, sessionized_config, args).await } - Some(("knowledge", knowledge_matches)) => match knowledge_matches.subcommand() { - Some(("list", args)) => { - catalogs::do_knowledge_list(&out, sessionized_config, args).await + Some(("knowledge", knowledge_matches)) => { + match knowledge_matches.subcommand() { + Some(("list", args)) => { + catalogs::do_knowledge_list(&out, sessionized_config, args).await + } + Some(("show", args)) => { + catalogs::do_knowledge_show(&out, sessionized_config, args).await + } + Some(("set", args)) => { + catalogs::do_knowledge_set(&out, sessionized_config, args).await + } + Some(("delete", args)) => { + catalogs::do_knowledge_delete(&out, sessionized_config, args).await + } + _ => { + catalogs::catalogs_cmd().print_help().unwrap(); + std::process::exit(2); + } } - Some(("show", args)) => { - catalogs::do_knowledge_show(&out, sessionized_config, args).await - } - Some(("set", args)) => { - catalogs::do_knowledge_set(&out, sessionized_config, args).await - } - Some(("delete", args)) => { - catalogs::do_knowledge_delete(&out, sessionized_config, args).await - } - _ => { - catalogs::catalogs_cmd().print_help().unwrap(); - std::process::exit(2); - } - }, + } _ => { catalogs::catalogs_cmd().print_help().unwrap(); std::process::exit(2); diff --git a/crates/tower-cmd/src/mcp.rs b/crates/tower-cmd/src/mcp.rs index 7ccbce56..c7994819 100644 --- a/crates/tower-cmd/src/mcp.rs +++ b/crates/tower-cmd/src/mcp.rs @@ -80,6 +80,9 @@ struct UpdateTowerfileRequest { script: Option, description: Option, source: Option>, + /// Compute size for each run: one of basic.xsmall, basic.small, + /// basic.medium, basic.large. Omit to let Tower apply its default. + run_size: Option, } #[derive(Debug, Deserialize, JsonSchema)] @@ -1020,7 +1023,7 @@ impl TowerService { } #[tool( - description = "Update Towerfile config (app name, script, description, source). Use this instead of editing TOML. Optional: working_directory." + description = "Update Towerfile config (app name, script, description, source, run_size). Use this instead of editing TOML. Optional: working_directory." )] async fn tower_file_update( &self, @@ -1039,6 +1042,9 @@ impl TowerService { if let Some(source) = request.source { tf.app.source = source; } + if let Some(run_size) = request.run_size { + tf.app.run_size = Some(run_size); + } Ok("Towerfile updated".into()) }) } diff --git a/crates/tower-cmd/src/skill.rs b/crates/tower-cmd/src/skill.rs index c575293c..2f28e505 100644 --- a/crates/tower-cmd/src/skill.rs +++ b/crates/tower-cmd/src/skill.rs @@ -190,6 +190,8 @@ tower_file_generate → tower_file_update → tower_file_add/edit/remove_paramet Always use `tower_file_update` or `tower_file_add/edit/remove_parameter` to modify. Never edit the TOML directly. +To give an app more compute, set `run_size` via `tower_file_update`: one of `basic.xsmall`, `basic.small`, `basic.medium` (the default), `basic.large`. It is versioned with the code, and self-hosted runners ignore it. + ### 2. Local development (preferred) ``` diff --git a/crates/tower-duckdb/src/lib.rs b/crates/tower-duckdb/src/lib.rs index 64754fc7..a96242dc 100644 --- a/crates/tower-duckdb/src/lib.rs +++ b/crates/tower-duckdb/src/lib.rs @@ -1188,12 +1188,15 @@ mod tests { false } - /// MinIO pinned to an immutable release tag rather than `latest`. + /// Coollabs builds official MinIO sources; upstream's prebuilt images are unavailable. + /// https://github.com/coollabsio/minio /// - /// A floating tag lets a new upstream image change this test's behaviour, or - /// break it into a skip, without anyone choosing that. A security regression - /// test should only change when someone means it. Bump deliberately and re-run. - const MINIO_IMAGE_TAG: &str = "RELEASE.2025-09-07T16-13-09Z"; + /// Pin the multi-platform image digest so a rebuilt tag cannot change this security test. + /// Bump deliberately and re-run the object-store test on Linux. + const MINIO_IMAGE_TAG: &str = concat!( + "RELEASE.2025-10-15T17-29-55Z@", + "sha256:69b55a1c1c5dc285ce04db96689f5b2102317fc77a50680a1874ca6efd1c87f9" + ); /// Whether a container runtime that can run this image is reachable. /// @@ -1241,7 +1244,7 @@ mod tests { } let bucket = "warehouse"; - let image = GenericImage::new("quay.io/minio/minio", MINIO_IMAGE_TAG) + let image = GenericImage::new("ghcr.io/coollabsio/minio", MINIO_IMAGE_TAG) .with_wait_for(WaitFor::seconds(1)) .with_exposed_port(9000.tcp()) .with_entrypoint("sh") diff --git a/crates/tower-package/src/core.rs b/crates/tower-package/src/core.rs index 69383c4f..f65233c0 100644 --- a/crates/tower-package/src/core.rs +++ b/crates/tower-package/src/core.rs @@ -74,6 +74,9 @@ pub struct Manifest { pub schedule: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub run_size: Option, + #[serde(default)] pub import_paths: Vec, @@ -108,7 +111,7 @@ pub struct PackageInputs { // module_files have archive_name already rooted under "modules/". pub module_files: Vec, - // towerfile_bytes is the sole source of invoke, parameters, and import_paths. + // towerfile_bytes is the sole source of invoke, parameters, run_size, and import_paths. pub towerfile_bytes: Vec, } @@ -154,6 +157,7 @@ pub fn build_package(inputs: PackageInputs) -> Result { invoke: towerfile.app.script, parameters: towerfile.parameters, schedule: None, + run_size: towerfile.app.run_size, import_paths, app_dir_name: "app".to_string(), modules_dir_name: "modules".to_string(), diff --git a/crates/tower-package/src/lib.rs b/crates/tower-package/src/lib.rs index 3f8520ed..35586799 100644 --- a/crates/tower-package/src/lib.rs +++ b/crates/tower-package/src/lib.rs @@ -5,7 +5,7 @@ pub use core::{ build_package, compute_sha256_bytes, compute_sha256_package, normalize_path, BuiltPackage, Entry, Manifest, PackageInputs, CURRENT_PACKAGE_VERSION, }; -pub use towerfile::{App, Parameter, Towerfile}; +pub use towerfile::{App, Parameter, Towerfile, RUN_SIZES}; #[cfg(feature = "native")] mod error; diff --git a/crates/tower-package/src/native.rs b/crates/tower-package/src/native.rs index 516c8c70..e79c271e 100644 --- a/crates/tower-package/src/native.rs +++ b/crates/tower-package/src/native.rs @@ -20,10 +20,9 @@ use crate::core::{ use crate::error::Error; use crate::towerfile::Towerfile; -// PackageSpec describes how to build a package. Everything core needs (invoke, parameters, -// import_paths as manifest entries) is derived from the Towerfile on disk, so this struct only -// carries what the file resolver needs: where the Towerfile lives, what's considered the project -// root, which globs match app files, and which import paths to walk. +// PackageSpec describes how to build a package. Manifest fields are derived from the Towerfile +// on disk, so this struct only carries what the file resolver needs: where the Towerfile lives, +// what's considered the project root, which globs match app files, and which import paths to walk. #[derive(Debug)] pub struct PackageSpec { pub towerfile_path: PathBuf, @@ -85,6 +84,7 @@ impl Package { invoke: "".to_string(), parameters: vec![], schedule: None, + run_size: None, import_paths: vec![], app_dir_name: "app".to_string(), modules_dir_name: "modules".to_string(), @@ -573,6 +573,7 @@ mod test { invoke: invoke.to_string(), parameters: vec![], schedule: None, + run_size: None, import_paths: vec![], app_dir_name: "app".to_string(), modules_dir_name: "modules".to_string(), diff --git a/crates/tower-package/src/towerfile.rs b/crates/tower-package/src/towerfile.rs index c97250b4..a89e598c 100644 --- a/crates/tower-package/src/towerfile.rs +++ b/crates/tower-package/src/towerfile.rs @@ -2,6 +2,12 @@ use crate::core::Error; use serde::{Deserialize, Serialize}; use std::path::PathBuf; +/// Compute sizes accepted by `[app] run_size`. Managed runners apply the size; +/// self-hosted runners ignore it and use their own configured resources. When +/// the Towerfile omits it, the server applies its own default, so this list +/// deliberately has no default of its own. +pub const RUN_SIZES: [&str; 4] = ["basic.xsmall", "basic.small", "basic.medium", "basic.large"]; + #[derive(Clone, Deserialize, Serialize, Debug)] pub struct Parameter { #[serde(default)] @@ -39,6 +45,12 @@ pub struct App { #[serde(default)] pub import_paths: Vec, + + /// Compute size every run of this app gets, e.g. "basic.medium". `None` + /// means the Towerfile didn't set one (and the key is omitted when + /// serializing), leaving the server to apply its default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub run_size: Option, } #[derive(Deserialize, Serialize, Debug)] @@ -66,6 +78,7 @@ impl Towerfile { schedule: String::from("0 0 * * *"), description: None, import_paths: vec![], + run_size: None, }, } } @@ -74,20 +87,27 @@ impl Towerfile { /// the base_dir field always needs to be set after parsing. pub fn from_toml(toml: &str) -> Result { let towerfile: Towerfile = toml::from_str(toml)?; + towerfile.validate()?; + Ok(towerfile) + } - if towerfile.app.name.is_empty() { + /// validate checks the semantic rules that TOML parsing alone can't enforce. + /// It runs both when a Towerfile is read and before one is written back, so a + /// mutation can't leave an invalid file on disk. + pub fn validate(&self) -> Result<(), Error> { + if self.app.name.is_empty() { return Err(Error::MissingRequiredAppField { field: "name".to_string(), }); } - if towerfile.app.script.is_empty() { + if self.app.script.is_empty() { return Err(Error::MissingRequiredAppField { field: "script".to_string(), }); } - for import_path in &towerfile.app.import_paths { + for import_path in &self.app.import_paths { let as_str = import_path.to_string_lossy(); if as_str.is_empty() { return Err(Error::InvalidTowerfile { @@ -106,7 +126,21 @@ impl Towerfile { } } - Ok(towerfile) + if let Some(run_size) = &self.app.run_size { + // The server rejects unknown sizes at deploy time. Catching it here + // means the user finds out before the upload rather than after. + if !RUN_SIZES.contains(&run_size.as_str()) { + return Err(Error::InvalidTowerfile { + message: format!( + "run_size {:?} is not a valid size (expected one of: {})", + run_size, + RUN_SIZES.join(", ") + ), + }); + } + } + + Ok(()) } /// set_parameter upserts a parameter by lookup name. If a parameter with the given name @@ -161,6 +195,8 @@ impl Towerfile { pub fn save(&self, path: Option<&std::path::Path>) -> Result<(), crate::error::Error> { use crate::error::Error as OuterError; + self.validate()?; + let target_path = path.unwrap_or_else(|| std::path::Path::new("Towerfile")); let serialized = toml::to_string_pretty(self).map_err(|err| OuterError::InvalidTowerfile { @@ -436,6 +472,129 @@ mod test { assert!(serialized.contains(r#"description = "My app""#)); } + #[test] + fn test_run_size_absent_when_not_set() { + let toml = r#" + [app] + name = "test" + script = "./script.py" + "#; + + let towerfile = crate::Towerfile::from_toml(toml).unwrap(); + assert_eq!(towerfile.app.run_size, None); + + // An unset run_size must not be written into the user's Towerfile, so + // the server keeps applying its own default. + let serialized = toml::to_string_pretty(&towerfile).unwrap(); + assert!(!serialized.contains("run_size")); + } + + #[test] + fn test_run_size_accepts_every_known_size() { + for size in crate::RUN_SIZES { + let toml = format!( + r#" + [app] + name = "test" + script = "./script.py" + run_size = "{}" + "#, + size + ); + + let towerfile = crate::Towerfile::from_toml(&toml).unwrap(); + assert_eq!(towerfile.app.run_size.as_deref(), Some(size)); + } + } + + #[test] + fn test_run_size_rejects_unknown_size() { + let toml = r#" + [app] + name = "test" + script = "./script.py" + run_size = "basic.enormous" + "#; + + let err = crate::Towerfile::from_toml(toml).unwrap_err(); + let message = err.to_string(); + assert!(message.contains("basic.enormous"), "was: {}", message); + assert!(message.contains("basic.medium"), "was: {}", message); + } + + #[test] + fn test_run_size_is_case_and_prefix_sensitive() { + // The size names are matched exactly; near misses must not slip through + // to the server. + for bad in ["medium", "basic.Medium", "BASIC.MEDIUM", " basic.medium"] { + let toml = format!( + r#" + [app] + name = "test" + script = "./script.py" + run_size = "{}" + "#, + bad + ); + + assert!( + crate::Towerfile::from_toml(&toml).is_err(), + "expected {:?} to be rejected", + bad + ); + } + } + + #[test] + fn test_run_size_survives_read_modify_write() { + // Regression: every MCP write round-trips the Towerfile through this + // struct, so a field that isn't modelled is silently dropped from the + // user's file. Editing an unrelated field must leave run_size intact. + let toml = r#" + [app] + name = "test" + script = "./script.py" + run_size = "basic.large" + "#; + + let mut towerfile = crate::Towerfile::from_toml(toml).unwrap(); + towerfile.app.description = Some("edited".to_string()); + + let serialized = toml::to_string_pretty(&towerfile).unwrap(); + let reparsed = crate::Towerfile::from_toml(&serialized).unwrap(); + + assert_eq!(reparsed.app.run_size.as_deref(), Some("basic.large")); + assert_eq!(reparsed.app.description.as_deref(), Some("edited")); + } + + #[test] + fn test_save_refuses_to_persist_an_invalid_run_size() { + // MCP edits mutate the struct and write it straight back, so save() has + // to hold the mutation to the same rules as a read. Otherwise a bad + // value lands on disk and the user only finds out on the next read. + let toml = r#" + [app] + name = "test" + script = "./script.py" + run_size = "basic.small" + "#; + + let mut towerfile = crate::Towerfile::from_toml(toml).unwrap(); + towerfile.app.run_size = Some("basic.enormous".to_string()); + + let tempfile = + TestFile::new("Towerfile-save-invalid").expect("Failed to create temporary file"); + let path = PathBuf::from("Towerfile-save-invalid"); + + let err = towerfile.save(Some(&path)).unwrap_err(); + assert!(err.to_string().contains("basic.enormous"), "was: {}", err); + + // The file must be untouched, not half-written. + assert_eq!(std::fs::read_to_string(&path).unwrap(), ""); + + drop(tempfile); + } + #[test] fn test_roundtrip_serialization() { let original_toml = r#"[app] diff --git a/crates/tower-package/tests/package_test.rs b/crates/tower-package/tests/package_test.rs index e39a020f..0ef8ef74 100644 --- a/crates/tower-package/tests/package_test.rs +++ b/crates/tower-package/tests/package_test.rs @@ -57,6 +57,11 @@ async fn it_creates_package() { "files {:?} was missing MANIFEST", files ); + + let manifest = &files["MANIFEST"]; + let json: serde_json::Value = serde_json::from_str(manifest).unwrap(); + assert!(json.get("run_size").is_none()); + assert_eq!(Manifest::from_json(manifest).unwrap().run_size, None); } #[tokio::test] @@ -605,3 +610,38 @@ where .expect("Failed to read string from stream"); content } + +#[tokio::test] +async fn it_preserves_run_size_in_the_manifest_and_towerfile() { + // The deploy handler reads run_size from MANIFEST when creating the app version. + let tmp_dir = TmpDir::new("example") + .await + .expect("Failed to create temp dir"); + + let towerfile = "[app]\nname = \"test\"\nscript = \"main.py\"\nrun_size = \"basic.large\"\n"; + create_test_file(tmp_dir.to_path_buf(), "Towerfile", towerfile).await; + create_test_file(tmp_dir.to_path_buf(), "main.py", "print('Hello, world!')").await; + + let spec = PackageSpec { + base_dir: tmp_dir.to_path_buf(), + towerfile_path: tmp_dir.to_path_buf().join("Towerfile").to_path_buf(), + file_globs: vec!["*.py".to_string()], + import_paths: vec![], + }; + + let package = Package::build(spec).await.expect("Failed to build package"); + let files = read_package_files(package).await; + + let packaged = files + .get("Towerfile") + .expect("package was missing the Towerfile entry"); + assert_eq!(packaged, towerfile); + + let manifest = files.get("MANIFEST").expect("package was missing MANIFEST"); + let json: serde_json::Value = serde_json::from_str(manifest).unwrap(); + assert_eq!(json["run_size"], "basic.large"); + assert_eq!( + Manifest::from_json(manifest).unwrap().run_size.as_deref(), + Some("basic.large") + ); +} diff --git a/plugin/skills/tower/SKILL.md b/plugin/skills/tower/SKILL.md index a6fe0ad7..476f4b4e 100644 --- a/plugin/skills/tower/SKILL.md +++ b/plugin/skills/tower/SKILL.md @@ -90,6 +90,8 @@ tower_file_generate → tower_file_update → tower_file_add/edit/remove_paramet Always use `tower_file_update` or `tower_file_add/edit/remove_parameter` to modify. Never edit the TOML directly. +To give an app more compute, set `run_size` via `tower_file_update`: one of `basic.xsmall`, `basic.small`, `basic.medium` (the default), `basic.large`. It is versioned with the code, and self-hosted runners ignore it. + ### 2. Local development (preferred) ``` diff --git a/pyproject.toml b/pyproject.toml index afb00418..d1265b9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "tower" -version = "0.3.75" +version = "0.3.76" description = "Tower CLI and runtime environment for Tower." authors = [{ name = "Tower Computing GmbH", email = "brad@tower.dev" }] readme = "README.md" diff --git a/tests/integration/features/mcp_app_management.feature b/tests/integration/features/mcp_app_management.feature index ccae5790..03762225 100644 --- a/tests/integration/features/mcp_app_management.feature +++ b/tests/integration/features/mcp_app_management.feature @@ -34,6 +34,24 @@ Feature: MCP App Management When I call tower_file_read via MCP Then I should receive the parsed Towerfile configuration + Scenario: Set the app's compute size through the Towerfile + Given I have a valid Towerfile in the current directory + When I call tower_file_update with run_size "basic.large" + Then I should receive a success response + And the Towerfile on disk should contain run_size "basic.large" + + Scenario: Reject an invalid compute size + Given I have a valid Towerfile in the current directory + When I call tower_file_update with run_size "basic.enormous" + Then I should receive an error response + And the Towerfile on disk should contain run_size "basic.small" + + Scenario: Editing an unrelated field preserves the compute size + Given I have a valid Towerfile in the current directory + When I call tower_file_update with description "an edited description" + Then I should receive a success response + And the Towerfile on disk should contain run_size "basic.small" + Scenario: Run simple application successfully locally Given I have a simple hello world application When I call tower_run_local via MCP diff --git a/tests/integration/features/steps/mcp_steps.py b/tests/integration/features/steps/mcp_steps.py index b9afa60e..1f097ba2 100644 --- a/tests/integration/features/steps/mcp_steps.py +++ b/tests/integration/features/steps/mcp_steps.py @@ -249,6 +249,26 @@ async def step_call_mcp_tool_with_unique_app_name(context, tool_name, app_name): ) +@step('I call tower_file_update with run_size "{run_size}"') +@async_run_until_complete +async def step_update_run_size(context, run_size): + await call_mcp_tool(context, "tower_file_update", {"run_size": run_size}) + + +@step('I call tower_file_update with description "{description}"') +@async_run_until_complete +async def step_update_description(context, description): + await call_mcp_tool(context, "tower_file_update", {"description": description}) + + +@then('the Towerfile on disk should contain run_size "{run_size}"') +def step_check_towerfile_run_size(context, run_size): + contents = Path("Towerfile").read_text() + assert ( + f'run_size = "{run_size}"' in contents + ), f"Towerfile should keep run_size {run_size!r}, got: {contents}" + + @step("I should receive a response") def step_check_response_exists(context): assert hasattr(context, "mcp_response") and context.mcp_response is not None diff --git a/tests/integration/templates/Towerfile.j2 b/tests/integration/templates/Towerfile.j2 index a6e926ef..11a521fb 100644 --- a/tests/integration/templates/Towerfile.j2 +++ b/tests/integration/templates/Towerfile.j2 @@ -3,6 +3,7 @@ name = "{{ app_name }}" script = "./{{ script_name }}" description = "{{ description }}" source = ["./{{ script_name }}"] +run_size = "basic.small" [build] python = "3.11" \ No newline at end of file diff --git a/uv.lock b/uv.lock index c20f1de8..a10f890f 100644 --- a/uv.lock +++ b/uv.lock @@ -2264,7 +2264,7 @@ wheels = [ [[package]] name = "tower" -version = "0.3.75" +version = "0.3.76" source = { editable = "." } dependencies = [ { name = "attrs" },