From dd3553dee548a998b354f6f7e5d6caddf5134a31 Mon Sep 17 00:00:00 2001 From: Ben Lovell Date: Tue, 10 Mar 2026 16:17:18 +0100 Subject: [PATCH] feat: add ability to edit/remove towerfile params in MCP server --- crates/config/src/lib.rs | 2 +- crates/config/src/towerfile.rs | 63 +++++++++--- crates/tower-cmd/src/mcp.rs | 177 ++++++++++++++++++++++++--------- 3 files changed, 179 insertions(+), 63 deletions(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index a923147e..c66affd5 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -9,7 +9,7 @@ mod towerfile; pub use error::Error; pub use session::{default_tower_url, Session, Team, Token, User}; -pub use towerfile::Towerfile; +pub use towerfile::{Parameter, Towerfile}; pub use session::{get_last_version_check_timestamp, set_last_version_check_timestamp}; diff --git a/crates/config/src/towerfile.rs b/crates/config/src/towerfile.rs index 703c999d..b0ab1d3c 100644 --- a/crates/config/src/towerfile.rs +++ b/crates/config/src/towerfile.rs @@ -119,14 +119,21 @@ impl Towerfile { Ok(()) } - /// add_parameter adds a new parameter to the Towerfile - pub fn add_parameter(&mut self, name: String, description: String, default: String) { - self.parameters.push(Parameter { - name, - description, - default, - hidden: false, - }); + /// set_parameter upserts a parameter by lookup name. If a parameter with the given name + /// exists, it is replaced. Otherwise, the parameter is appended. + pub fn set_parameter(&mut self, lookup_name: &str, param: Parameter) { + if let Some(existing) = self.parameters.iter_mut().find(|p| p.name == lookup_name) { + *existing = param; + } else { + self.parameters.push(param); + } + } + + /// remove_parameter removes a parameter by name, returning true if it was found + pub fn remove_parameter(&mut self, name: &str) -> bool { + let len_before = self.parameters.len(); + self.parameters.retain(|p| p.name != name); + self.parameters.len() < len_before } } @@ -299,20 +306,48 @@ mod test { } #[test] - fn test_add_parameter() { + fn test_set_parameter() { let mut towerfile = crate::Towerfile::default(); assert_eq!(towerfile.parameters.len(), 0); - towerfile.add_parameter( - "test-param".to_string(), - "A test parameter".to_string(), - "default-value".to_string(), - ); + towerfile.set_parameter("test-param", crate::Parameter { + name: "test-param".to_string(), + description: "A test parameter".to_string(), + default: "default-value".to_string(), + hidden: false, + }); assert_eq!(towerfile.parameters.len(), 1); assert_eq!(towerfile.parameters[0].name, "test-param"); assert_eq!(towerfile.parameters[0].description, "A test parameter"); assert_eq!(towerfile.parameters[0].default, "default-value"); + assert!(!towerfile.parameters[0].hidden); + + // upsert should replace, not duplicate + towerfile.set_parameter("test-param", crate::Parameter { + name: "test-param".to_string(), + description: "Updated".to_string(), + default: "new-value".to_string(), + hidden: false, + }); + + assert_eq!(towerfile.parameters.len(), 1); + assert_eq!(towerfile.parameters[0].description, "Updated"); + } + + #[test] + fn test_remove_parameter() { + let mut towerfile = crate::Towerfile::default(); + towerfile.set_parameter("param1", crate::Parameter { + name: "param1".to_string(), + description: "".to_string(), + default: "".to_string(), + hidden: false, + }); + + assert!(towerfile.remove_parameter("param1")); + assert_eq!(towerfile.parameters.len(), 0); + assert!(!towerfile.remove_parameter("param1")); } #[test] diff --git a/crates/tower-cmd/src/mcp.rs b/crates/tower-cmd/src/mcp.rs index c2c18e47..f41d9dc6 100644 --- a/crates/tower-cmd/src/mcp.rs +++ b/crates/tower-cmd/src/mcp.rs @@ -2,7 +2,7 @@ use std::future::Future; use axum::Router; use clap::Command; -use config::{Session, Towerfile}; +use config::{Parameter, Session, Towerfile}; use crypto; use rmcp::{ handler::server::tool::{Parameters, ToolRouter}, @@ -87,8 +87,37 @@ struct AddParameterRequest { #[serde(flatten)] common: CommonParams, name: String, - description: String, - default: String, + /// Description of the parameter. Required for regular parameters, omit for hidden parameters. + description: Option, + /// Default value for the parameter. Mutually exclusive with hidden. + default: Option, + /// Whether the parameter is hidden (value comes from secrets). Mutually exclusive with default. + #[serde(default)] + hidden: bool, +} + +#[derive(Debug, Deserialize, JsonSchema)] +struct EditParameterRequest { + #[serde(flatten)] + common: CommonParams, + /// The name of the existing parameter to edit + name: String, + /// New name for the parameter + new_name: Option, + /// New description for the parameter + description: Option, + /// New default value for the parameter. Mutually exclusive with hidden. + default: Option, + /// Set to true to make hidden, or false to make visible. Mutually exclusive with default. + hidden: Option, +} + +#[derive(Debug, Deserialize, JsonSchema)] +struct RemoveParameterRequest { + #[serde(flatten)] + common: CommonParams, + /// The name of the parameter to remove + name: String, } #[derive(Debug, Deserialize, JsonSchema)] @@ -259,6 +288,10 @@ impl TowerService { Ok(CallToolResult::success(vec![Content::text(message)])) } + fn text_error(message: String) -> Result { + Ok(CallToolResult::error(vec![Content::text(message)])) + } + fn error_result( prefix: &str, error: impl std::fmt::Display + std::fmt::Debug, @@ -269,6 +302,28 @@ impl TowerService { ))])) } + fn modify_towerfile( + common: &CommonParams, + f: impl FnOnce(&mut Towerfile) -> Result, + ) -> Result { + let working_dir = Self::resolve_working_directory(common); + let mut towerfile = match Towerfile::from_dir_str(working_dir.to_str().unwrap()) { + Ok(tf) => tf, + Err(e) => return Self::error_result("Failed to read Towerfile", e), + }; + + let message = match f(&mut towerfile) { + Ok(msg) => msg, + Err(msg) => return Self::text_error(msg), + }; + + let towerfile_path = working_dir.join("Towerfile"); + match towerfile.save(Some(&towerfile_path)) { + Ok(_) => Self::text_success(message), + Err(e) => Self::error_result("Failed to save Towerfile", e), + } + } + fn resolve_working_directory(common: &CommonParams) -> std::path::PathBuf { common .working_directory @@ -749,59 +804,85 @@ impl TowerService { &self, Parameters(request): Parameters, ) -> Result { - let working_dir = Self::resolve_working_directory(&request.common); - let mut towerfile = match Towerfile::from_dir_str(working_dir.to_str().unwrap()) { - Ok(tf) => tf, - Err(e) => return Self::error_result("Failed to read Towerfile", e), - }; - - if let Some(name) = request.app_name { - towerfile.app.name = name; - } - if let Some(script) = request.script { - towerfile.app.script = script; - } - if let Some(description) = request.description { - towerfile.app.description = Some(description); - } - if let Some(source) = request.source { - towerfile.app.source = source; - } - - let towerfile_path = working_dir.join("Towerfile"); - match towerfile.save(Some(&towerfile_path)) { - Ok(_) => { - Self::text_success(format!("Towerfile updated at {}", towerfile_path.display())) + Self::modify_towerfile(&request.common, |tf| { + if let Some(name) = request.app_name { + tf.app.name = name; } - Err(e) => Self::error_result("Failed to save Towerfile", e), - } + if let Some(script) = request.script { + tf.app.script = script; + } + if let Some(description) = request.description { + tf.app.description = Some(description); + } + if let Some(source) = request.source { + tf.app.source = source; + } + Ok("Towerfile updated".into()) + }) } #[tool( - description = "Add parameter to Towerfile. Use this instead of editing TOML. Optional: working_directory." + description = "Add parameter to Towerfile. For regular params provide description and default. For hidden params set hidden=true (no default). Optional: working_directory." )] async fn tower_file_add_parameter( &self, Parameters(request): Parameters, ) -> Result { - let working_dir = Self::resolve_working_directory(&request.common); - let mut towerfile = match Towerfile::from_dir_str(working_dir.to_str().unwrap()) { - Ok(tf) => tf, - Err(e) => return Self::error_result("Failed to read Towerfile", e), - }; + if request.hidden && request.default.is_some() { + return Self::text_error("hidden and default are mutually exclusive".into()); + } + let name = request.name.clone(); + Self::modify_towerfile(&request.common, |tf| { + tf.set_parameter(&name, Parameter { + name: name.clone(), + description: request.description.unwrap_or_default(), + default: request.default.unwrap_or_default(), + hidden: request.hidden, + }); + Ok(format!("Added parameter '{name}'")) + }) + } - let param_name = request.name.clone(); - towerfile.add_parameter(request.name, request.description, request.default); + #[tool( + description = "Edit an existing parameter in the Towerfile. Provide only the fields to change. hidden and default are mutually exclusive. Optional: working_directory." + )] + async fn tower_file_edit_parameter( + &self, + Parameters(request): Parameters, + ) -> Result { + let name = request.name.clone(); + Self::modify_towerfile(&request.common, |tf| { + let existing = tf.parameters.iter().find(|p| p.name == name) + .ok_or_else(|| format!("Parameter '{name}' not found"))?; + let param = Parameter { + name: request.new_name.unwrap_or_else(|| existing.name.clone()), + description: request.description.unwrap_or_else(|| existing.description.clone()), + default: request.default.unwrap_or_else(|| existing.default.clone()), + hidden: request.hidden.unwrap_or(existing.hidden), + }; + if param.hidden && !param.default.is_empty() { + return Err("hidden and default are mutually exclusive".into()); + } + tf.set_parameter(&name, param); + Ok(format!("Updated parameter '{name}'")) + }) + } - let towerfile_path = working_dir.join("Towerfile"); - match towerfile.save(Some(&towerfile_path)) { - Ok(_) => Self::text_success(format!( - "Added parameter '{}' to {}", - param_name, - towerfile_path.display() - )), - Err(e) => Self::error_result("Failed to save Towerfile", e), - } + #[tool( + description = "Remove a parameter from the Towerfile. Optional: working_directory." + )] + async fn tower_file_remove_parameter( + &self, + Parameters(request): Parameters, + ) -> Result { + let name = request.name.clone(); + Self::modify_towerfile(&request.common, |tf| { + if tf.remove_parameter(&name) { + Ok(format!("Removed parameter '{name}'")) + } else { + Err(format!("Parameter '{name}' not found")) + } + }) } #[tool(description = "Validate Towerfile configuration. Optional: working_directory.")] @@ -870,8 +951,8 @@ All tools accept optional working_directory parameter to specify which project t Skip this step if project with pyproject.toml already exists 1. TOWERFILE (required for all Tower operations): - tower_file_generate → tower_file_update → tower_file_add_parameter → tower_file_validate - CRITICAL: Always use tower_file_update or tower_file_add_parameter to modify + tower_file_generate → tower_file_update → tower_file_add/edit/remove_parameter → tower_file_validate + CRITICAL: Always use tower_file_update or tower_file_add/edit/remove_parameter to modify NEVER edit Towerfile TOML directly 2. LOCAL DEVELOPMENT (preferred during development): @@ -1008,7 +1089,7 @@ impl ServerHandler for TowerService { Rules: - MCP tools are the authoritative Tower interface (not wrappers) -- Use tower_file_update/add_parameter to modify Towerfiles (never edit TOML directly) +- Use tower_file_update/add/edit/remove_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