From f414ffc4849cf4b92fcd5ca9611ecd246e59a7bd Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 14 May 2026 13:29:56 +0000 Subject: [PATCH] fix: never mark failure/trigger/approval scripts as auto_kind=lib (#9168) --- ...ind_lib_for_runnable_script_kinds.down.sql | 3 + ..._kind_lib_for_runnable_script_kinds.up.sql | 9 ++ backend/tests/script_auto_kind_failure.rs | 122 ++++++++++++++++++ backend/windmill-api-scripts/src/scripts.rs | 16 +++ 4 files changed, 150 insertions(+) create mode 100644 backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql create mode 100644 backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql create mode 100644 backend/tests/script_auto_kind_failure.rs diff --git a/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql new file mode 100644 index 0000000000..bb9b57ac0b --- /dev/null +++ b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql @@ -0,0 +1,3 @@ +-- No-op: clearing a stray `auto_kind = 'lib'` value on failure/trigger/approval +-- scripts is not reversible (the original NULL/'lib' distinction is lost), and +-- restoring `'lib'` here would re-hide these scripts from their pickers. diff --git a/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql new file mode 100644 index 0000000000..e2038bb78c --- /dev/null +++ b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql @@ -0,0 +1,9 @@ +-- Failure, Trigger, and Approval scripts are runnable entrypoints by +-- definition. A prior parser regression occasionally classified them as +-- `auto_kind = 'lib'`, which hid them from the flow error-handler / +-- trigger / approval pickers. Clear those stray values so existing +-- affected scripts re-appear without requiring a redeploy. +UPDATE script +SET auto_kind = NULL +WHERE auto_kind = 'lib' + AND kind IN ('failure', 'trigger', 'approval'); diff --git a/backend/tests/script_auto_kind_failure.rs b/backend/tests/script_auto_kind_failure.rs new file mode 100644 index 0000000000..4110bd4ba5 --- /dev/null +++ b/backend/tests/script_auto_kind_failure.rs @@ -0,0 +1,122 @@ +use std::collections::HashMap; + +use sqlx::{Pool, Postgres}; +use windmill_api_client::types::{NewScript, ScriptLang}; +use windmill_test_utils::init_client; + +fn quick_ns(content: &str, path: &str, kind: Option<&str>) -> NewScript { + NewScript { + content: content.into(), + language: ScriptLang::Bun, + lock: None, + parent_hash: None, + path: path.into(), + concurrent_limit: None, + concurrency_time_window_s: None, + cache_ttl: None, + dedicated_worker: None, + description: "".to_string(), + draft_only: None, + envs: vec![], + is_template: None, + kind: kind.map(|s| s.to_string()), + summary: "".to_string(), + tag: None, + schema: HashMap::new(), + ws_error_handler_muted: Some(false), + priority: None, + delete_after_secs: None, + timeout: None, + restart_unless_cancelled: None, + deployment_message: None, + concurrency_key: None, + visible_to_runner_only: None, + auto_kind: None, + codebase: None, + has_preprocessor: None, + on_behalf_of_email: None, + assets: vec![], + modules: None, + } +} + +/// Regression: a `failure`-kind script must never be marked `auto_kind = 'lib'` +/// even if the parser fails to detect a `main` function, because the flow +/// error-handler picker filters out lib scripts and would otherwise hide it. +#[sqlx::test(fixtures("base"))] +async fn failure_kind_script_without_main_is_not_marked_lib( + db: Pool, +) -> anyhow::Result<()> { + let (client, _port, _s) = init_client(db.clone()).await; + + // Content with no `main` — TS parser would normally set auto_kind = 'lib'. + client + .create_script( + "test-workspace", + &quick_ns( + "export function notMain() { return 42 }", + "u/test-user/failure_no_main", + Some("failure"), + ), + ) + .await + .unwrap(); + + let auto_kind: Option = sqlx::query_scalar( + "SELECT auto_kind FROM script \ + WHERE workspace_id = $1 AND path = $2", + ) + .bind("test-workspace") + .bind("u/test-user/failure_no_main") + .fetch_one(&db) + .await?; + + assert_ne!( + auto_kind.as_deref(), + Some("lib"), + "failure-kind script must not be marked as 'lib' auto_kind, got {:?}", + auto_kind + ); + + Ok(()) +} + +/// Sibling: a normal `script` kind WITHOUT main should still be marked `lib` +/// (so it stays hidden from the regular script picker). Guards against an +/// over-broad sanitizer accidentally clearing the value for plain scripts. +#[sqlx::test(fixtures("base"))] +async fn regular_script_without_main_is_still_marked_lib( + db: Pool, +) -> anyhow::Result<()> { + let (client, _port, _s) = init_client(db.clone()).await; + + client + .create_script( + "test-workspace", + &quick_ns( + "export function notMain() { return 42 }", + "u/test-user/script_no_main", + Some("script"), + ), + ) + .await + .unwrap(); + + let auto_kind: Option = sqlx::query_scalar( + "SELECT auto_kind FROM script \ + WHERE workspace_id = $1 AND path = $2", + ) + .bind("test-workspace") + .bind("u/test-user/script_no_main") + .fetch_one(&db) + .await?; + + assert_eq!( + auto_kind.as_deref(), + Some("lib"), + "regular script without main should be marked 'lib', got {:?}", + auto_kind + ); + + Ok(()) +} diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 93c524f10a..2ca655ef32 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -1214,6 +1214,22 @@ async fn create_script_internal<'c>( } }; + // Failure, Trigger, and Approval scripts are runnable entrypoints by + // definition. They must never be marked `auto_kind = 'lib'`, or they + // disappear from the flow error-handler / trigger / approval pickers + // (which filter out lib scripts). Strip a stray `lib` here so a parser + // misclassification — e.g. failing to detect `main` after a deno_ast + // bump — cannot orphan these scripts in the UI. + let auto_kind = if matches!( + ns.kind, + Some(ScriptKind::Failure) | Some(ScriptKind::Trigger) | Some(ScriptKind::Approval) + ) && auto_kind.as_deref() == Some("lib") + { + None + } else { + auto_kind + }; + let ci_test_refs = windmill_common::schema::parse_ci_test_annotation(&ns.content, &lang.as_comment_lit()); let auto_kind = if ci_test_refs.is_some() {