From d1c0d4b9e0a33aadb6ff662442eb59f2e1ef78fb Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 17 Jul 2026 16:32:02 +0000 Subject: [PATCH] fix(flows): default update_flow body path from URL via EditFlow Harmonizes updateFlow with the EditVariable/EditResource/EditApp convention: the flow to update is identified by the URL, so the body path is optional and only needed to rename. Fixes the 422 at the API layer for every client (MCP, the in-app AI chat, raw HTTP), not just the MCP tool schema. Co-Authored-By: Claude Opus 4.8 --- backend/windmill-api-flows/src/flows.rs | 6 ++- backend/windmill-types/src/flows.rs | 67 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index a9376d02ed..ff555113c3 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -47,7 +47,7 @@ use windmill_common::HUB_BASE_URL; use windmill_common::{ db::UserDB, error::{self, to_anyhow, Error, JsonResult, Result}, - flows::{Flow, FlowWithStarred, ListFlowQuery, ListableFlow, NewFlow}, + flows::{EditFlow, Flow, FlowWithStarred, ListFlowQuery, ListableFlow, NewFlow}, jobs::JobPayload, schedule::Schedule, utils::{http_get_from_hub, not_found_if_none, paginate, Pagination, RunnableKind, StripPath}, @@ -1007,7 +1007,7 @@ async fn update_flow( Extension(db): Extension, Extension(webhook): Extension, Path((w_id, flow_path)): Path<(String, StripPath)>, - Json(nf): Json, + Json(ef): Json, ) -> Result { if authed.is_operator { return Err(Error::NotAuthorized( @@ -1015,6 +1015,8 @@ async fn update_flow( )); } let flow_path = flow_path.to_path(); + // The URL identifies the flow being updated; the body path is only needed to rename. + let nf = ef.into_new_flow(flow_path); check_scopes(&authed, || format!("flows:write:{}", flow_path))?; if let RuleCheckResult::Blocked(msg) = check_deploy_rules( diff --git a/backend/windmill-types/src/flows.rs b/backend/windmill-types/src/flows.rs index 0c95b9612b..e23a6cba9d 100644 --- a/backend/windmill-types/src/flows.rs +++ b/backend/windmill-types/src/flows.rs @@ -145,6 +145,57 @@ impl NewFlow { } } +/// Body for updating an existing flow. Mirrors `NewFlow`, but `path` is optional: the +/// flow to update is identified by the URL, so the body only needs `path` to rename it. +/// This matches the `EditVariable` / `EditResource` / `EditApp` convention and lets a +/// caller update in place without restating the path. +#[derive(Debug, Deserialize)] +pub struct EditFlow { + #[serde(default)] + pub path: Option, + pub summary: String, + pub description: Option, + #[serde(deserialize_with = "validate_flow_value")] + pub value: Box, + pub schema: Option, + pub tag: Option, + pub dedicated_worker: Option, + pub timeout: Option, + pub deployment_message: Option, + pub visible_to_runner_only: Option, + pub on_behalf_of_email: Option, + pub preserve_on_behalf_of: Option, + pub ws_error_handler_muted: Option, + #[serde(default)] + pub labels: Option>, + #[serde(default)] + pub skip_draft_deletion: Option, +} + +impl EditFlow { + /// Resolve into a `NewFlow`, defaulting the target path to `current_path` (the flow's + /// URL path) when the body omits it. A body `path` that differs renames the flow. + pub fn into_new_flow(self, current_path: &str) -> NewFlow { + NewFlow { + path: self.path.unwrap_or_else(|| current_path.to_string()), + summary: self.summary, + description: self.description, + value: self.value, + schema: self.schema, + tag: self.tag, + dedicated_worker: self.dedicated_worker, + timeout: self.timeout, + deployment_message: self.deployment_message, + visible_to_runner_only: self.visible_to_runner_only, + on_behalf_of_email: self.on_behalf_of_email, + preserve_on_behalf_of: self.preserve_on_behalf_of, + ws_error_handler_muted: self.ws_error_handler_muted, + labels: self.labels, + skip_draft_deletion: self.skip_draft_deletion, + } + } +} + fn validate_retry(retry: &Retry, module_id: &str) -> anyhow::Result<()> { if retry.exponential.attempts > 0 && retry.exponential.seconds == 0 { return Err(anyhow::anyhow!( @@ -1233,6 +1284,22 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn edit_flow_defaults_path_from_url_and_renames_when_given() { + // Omitted body path: update in place at the URL path (GIT-925 — a body + // without `path` used to fail deserialization on the required NewFlow.path). + let ef: EditFlow = + serde_json::from_value(json!({ "summary": "s", "value": { "modules": [] } })).unwrap(); + assert_eq!(ef.into_new_flow("f/team/my_flow").path, "f/team/my_flow"); + + // Explicit body path: rename to it. + let ef: EditFlow = serde_json::from_value( + json!({ "path": "f/team/renamed", "summary": "s", "value": { "modules": [] } }), + ) + .unwrap(); + assert_eq!(ef.into_new_flow("f/team/my_flow").path, "f/team/renamed"); + } + #[test] fn flow_value_ignores_notes_and_groups() { // FlowValue should parse successfully even when notes/groups are present —