From 448fce93f743d5b2ef2a2d4496eb2ec238594a0a Mon Sep 17 00:00:00 2001 From: hugocasa Date: Tue, 8 Sep 2026 17:07:26 +0200 Subject: [PATCH] fix: make the native trigger disable/enable toggle actually save (#11024) * feat: let a native trigger be disabled without deleting it Co-Authored-By: Claude Opus 5 (1M context) * fix: show and control the native trigger pause outside the flow editor Co-Authored-By: Claude Opus 5 (1M context) * fix: create a native trigger already paused instead of pausing it after Co-Authored-By: Claude Opus 5 (1M context) * docs: correct the native trigger enabled comments for create-time init Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- ...d1f339670da3ed1c08203d54d45798d022dd.json} | 12 +- ...3dc94b9337a8d3b69805308258d1fa145e63.json} | 7 +- ...5f9322229bb2a1e104b6eabaf2a697c2776a.json} | 12 +- ...146d5275cdd4313961c76223ed0f3fec7dca3.json | 35 +++++ ...0af9f49ded98cd1824f1ac91cc5dbe709bc1.json} | 12 +- ...4203f1132aa5ae5eff9194c45e0259f4bee76.json | 28 ++++ ...0908113909_native_trigger_enabled.down.sql | 1 + ...260908113909_native_trigger_enabled.up.sql | 1 + backend/summarized_schema.txt | 2 +- .../tests/native_triggers.rs | 138 +++++++++++++++++- backend/windmill-api/openapi.yaml | 57 ++++++++ backend/windmill-api/src/jobs.rs | 45 +++++- backend/windmill-api/src/workspaces_export.rs | 8 +- .../windmill-native-triggers/src/handler.rs | 90 +++++++++++- backend/windmill-native-triggers/src/lib.rs | 101 ++++++++++++- .../windmill-native-triggers/src/rename.rs | 1 + cli/src/commands/trigger/trigger.ts | 6 +- .../native/NativeTriggerEditor.svelte | 40 ++++- .../triggers/native/NativeTriggerTable.svelte | 40 ++++- .../lib/components/triggers/native/utils.ts | 4 +- .../[service_name]/+page.svelte | 1 + 21 files changed, 594 insertions(+), 47 deletions(-) rename backend/.sqlx/{query-6dafcc89668fb0e5740f23264b515b1724c36031da39d53ae6c329a479bdf8aa.json => query-0987f164fe3d64bf0a6a4e9699c2d1f339670da3ed1c08203d54d45798d022dd.json} (83%) rename backend/.sqlx/{query-1048d1c95270ce1f36c02bce31a2bc8a88935c613bd213b7156299811377db8e.json => query-64dce306efc0d9989542dba5bf003dc94b9337a8d3b69805308258d1fa145e63.json} (63%) rename backend/.sqlx/{query-b0775af41a9b54cce040bf37cae770e63dab12a1383a0855d105c061e4e4ca48.json => query-a2122e8520268919e2ddc85ef46b5f9322229bb2a1e104b6eabaf2a697c2776a.json} (68%) create mode 100644 backend/.sqlx/query-a3871035319012d72679f132696146d5275cdd4313961c76223ed0f3fec7dca3.json rename backend/.sqlx/{query-15014ce696cf2af4f719a537a4e3ca5b322cc130a35a91f8b8854f5ebdf25ad2.json => query-c7766afaea3e187824698cae2b090af9f49ded98cd1824f1ac91cc5dbe709bc1.json} (84%) create mode 100644 backend/.sqlx/query-fa42f89d9494056e1e7ad904b844203f1132aa5ae5eff9194c45e0259f4bee76.json create mode 100644 backend/migrations/20260908113909_native_trigger_enabled.down.sql create mode 100644 backend/migrations/20260908113909_native_trigger_enabled.up.sql diff --git a/backend/.sqlx/query-6dafcc89668fb0e5740f23264b515b1724c36031da39d53ae6c329a479bdf8aa.json b/backend/.sqlx/query-0987f164fe3d64bf0a6a4e9699c2d1f339670da3ed1c08203d54d45798d022dd.json similarity index 83% rename from backend/.sqlx/query-6dafcc89668fb0e5740f23264b515b1724c36031da39d53ae6c329a479bdf8aa.json rename to backend/.sqlx/query-0987f164fe3d64bf0a6a4e9699c2d1f339670da3ed1c08203d54d45798d022dd.json index 1c0035f7cb..8aecbed333 100644 --- a/backend/.sqlx/query-6dafcc89668fb0e5740f23264b515b1724c36031da39d53ae6c329a479bdf8aa.json +++ b/backend/.sqlx/query-0987f164fe3d64bf0a6a4e9699c2d1f339670da3ed1c08203d54d45798d022dd.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n external_id,\n workspace_id,\n service_name AS \"service_name!: ServiceName\",\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n error,\n created_at,\n updated_at,\n summary\n FROM\n native_trigger\n WHERE\n workspace_id = $1\n AND service_name = $2\n AND script_path = $3\n AND is_flow = $4\n LIMIT 1\n ", + "query": "\n SELECT\n external_id,\n workspace_id,\n service_name AS \"service_name!: ServiceName\",\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n error,\n created_at,\n updated_at,\n summary,\n enabled\n FROM\n native_trigger\n WHERE\n workspace_id = $1\n AND service_name = $2\n AND script_path = $3\n AND is_flow = $4\n LIMIT 1\n ", "describe": { "columns": [ { @@ -68,6 +68,11 @@ "ordinal": 10, "name": "summary", "type_info": "Varchar" + }, + { + "ordinal": 11, + "name": "enabled", + "type_info": "Bool" } ], "parameters": { @@ -100,8 +105,9 @@ true, false, false, - true + true, + false ] }, - "hash": "6dafcc89668fb0e5740f23264b515b1724c36031da39d53ae6c329a479bdf8aa" + "hash": "0987f164fe3d64bf0a6a4e9699c2d1f339670da3ed1c08203d54d45798d022dd" } diff --git a/backend/.sqlx/query-1048d1c95270ce1f36c02bce31a2bc8a88935c613bd213b7156299811377db8e.json b/backend/.sqlx/query-64dce306efc0d9989542dba5bf003dc94b9337a8d3b69805308258d1fa145e63.json similarity index 63% rename from backend/.sqlx/query-1048d1c95270ce1f36c02bce31a2bc8a88935c613bd213b7156299811377db8e.json rename to backend/.sqlx/query-64dce306efc0d9989542dba5bf003dc94b9337a8d3b69805308258d1fa145e63.json index 89a02933e0..d234750140 100644 --- a/backend/.sqlx/query-1048d1c95270ce1f36c02bce31a2bc8a88935c613bd213b7156299811377db8e.json +++ b/backend/.sqlx/query-64dce306efc0d9989542dba5bf003dc94b9337a8d3b69805308258d1fa145e63.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO native_trigger (\n external_id,\n workspace_id,\n service_name,\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n summary\n ) VALUES (\n $1, $2, $3, $4, $5, $6, $7, $8\n )\n ON CONFLICT (external_id, workspace_id, service_name)\n DO UPDATE SET script_path = $4, is_flow = $5, webhook_token_hash = $6, service_config = $7, summary = $8, error = NULL, updated_at = NOW()\n ", + "query": "\n INSERT INTO native_trigger (\n external_id,\n workspace_id,\n service_name,\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n summary,\n enabled\n ) VALUES (\n $1, $2, $3, $4, $5, $6, $7, $8, $9\n )\n ON CONFLICT (external_id, workspace_id, service_name)\n DO UPDATE SET script_path = $4, is_flow = $5, webhook_token_hash = $6, service_config = $7, summary = $8, error = NULL, updated_at = NOW()\n ", "describe": { "columns": [], "parameters": { @@ -23,10 +23,11 @@ "Bool", "Varchar", "Jsonb", - "Varchar" + "Varchar", + "Bool" ] }, "nullable": [] }, - "hash": "1048d1c95270ce1f36c02bce31a2bc8a88935c613bd213b7156299811377db8e" + "hash": "64dce306efc0d9989542dba5bf003dc94b9337a8d3b69805308258d1fa145e63" } diff --git a/backend/.sqlx/query-b0775af41a9b54cce040bf37cae770e63dab12a1383a0855d105c061e4e4ca48.json b/backend/.sqlx/query-a2122e8520268919e2ddc85ef46b5f9322229bb2a1e104b6eabaf2a697c2776a.json similarity index 68% rename from backend/.sqlx/query-b0775af41a9b54cce040bf37cae770e63dab12a1383a0855d105c061e4e4ca48.json rename to backend/.sqlx/query-a2122e8520268919e2ddc85ef46b5f9322229bb2a1e104b6eabaf2a697c2776a.json index 5889ab4ee7..0f43afed39 100644 --- a/backend/.sqlx/query-b0775af41a9b54cce040bf37cae770e63dab12a1383a0855d105c061e4e4ca48.json +++ b/backend/.sqlx/query-a2122e8520268919e2ddc85ef46b5f9322229bb2a1e104b6eabaf2a697c2776a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n nt.external_id,\n nt.workspace_id,\n nt.service_name AS \"service_name!: ServiceName\",\n nt.script_path,\n nt.is_flow,\n nt.webhook_token_hash,\n nt.service_config,\n nt.error,\n nt.created_at,\n nt.updated_at,\n nt.summary\n FROM\n native_trigger nt\n WHERE\n nt.workspace_id = $1 AND\n nt.service_name = $2 AND\n ($5::text IS NULL OR nt.script_path = $5) AND\n ($6::bool IS NULL OR nt.is_flow = $6) AND\n (\n (nt.is_flow = false AND EXISTS (\n SELECT 1 FROM script s\n WHERE s.workspace_id = nt.workspace_id\n AND s.path = nt.script_path\n ))\n OR\n (nt.is_flow = true AND EXISTS (\n SELECT 1 FROM flow f\n WHERE f.workspace_id = nt.workspace_id\n AND f.path = nt.script_path\n ))\n )\n LIMIT $3\n OFFSET $4\n ", + "query": "\n SELECT\n nt.external_id,\n nt.workspace_id,\n nt.service_name AS \"service_name!: ServiceName\",\n nt.script_path,\n nt.is_flow,\n nt.webhook_token_hash,\n nt.service_config,\n nt.error,\n nt.created_at,\n nt.updated_at,\n nt.summary,\n nt.enabled\n FROM\n native_trigger nt\n WHERE\n nt.workspace_id = $1 AND\n nt.service_name = $2 AND\n ($5::text IS NULL OR nt.script_path = $5) AND\n ($6::bool IS NULL OR nt.is_flow = $6) AND\n (\n (nt.is_flow = false AND EXISTS (\n SELECT 1 FROM script s\n WHERE s.workspace_id = nt.workspace_id\n AND s.path = nt.script_path\n ))\n OR\n (nt.is_flow = true AND EXISTS (\n SELECT 1 FROM flow f\n WHERE f.workspace_id = nt.workspace_id\n AND f.path = nt.script_path\n ))\n )\n LIMIT $3\n OFFSET $4\n ", "describe": { "columns": [ { @@ -68,6 +68,11 @@ "ordinal": 10, "name": "summary", "type_info": "Varchar" + }, + { + "ordinal": 11, + "name": "enabled", + "type_info": "Bool" } ], "parameters": { @@ -102,8 +107,9 @@ true, false, false, - true + true, + false ] }, - "hash": "b0775af41a9b54cce040bf37cae770e63dab12a1383a0855d105c061e4e4ca48" + "hash": "a2122e8520268919e2ddc85ef46b5f9322229bb2a1e104b6eabaf2a697c2776a" } diff --git a/backend/.sqlx/query-a3871035319012d72679f132696146d5275cdd4313961c76223ed0f3fec7dca3.json b/backend/.sqlx/query-a3871035319012d72679f132696146d5275cdd4313961c76223ed0f3fec7dca3.json new file mode 100644 index 0000000000..5f2bb07e24 --- /dev/null +++ b/backend/.sqlx/query-a3871035319012d72679f132696146d5275cdd4313961c76223ed0f3fec7dca3.json @@ -0,0 +1,35 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT enabled\n FROM native_trigger\n WHERE\n workspace_id = $1\n AND service_name = $2\n AND external_id = $3\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "enabled", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + { + "Custom": { + "name": "native_trigger_service", + "kind": { + "Enum": [ + "nextcloud", + "google", + "github" + ] + } + } + }, + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "a3871035319012d72679f132696146d5275cdd4313961c76223ed0f3fec7dca3" +} diff --git a/backend/.sqlx/query-15014ce696cf2af4f719a537a4e3ca5b322cc130a35a91f8b8854f5ebdf25ad2.json b/backend/.sqlx/query-c7766afaea3e187824698cae2b090af9f49ded98cd1824f1ac91cc5dbe709bc1.json similarity index 84% rename from backend/.sqlx/query-15014ce696cf2af4f719a537a4e3ca5b322cc130a35a91f8b8854f5ebdf25ad2.json rename to backend/.sqlx/query-c7766afaea3e187824698cae2b090af9f49ded98cd1824f1ac91cc5dbe709bc1.json index 0af901b2b8..6a9de7e35b 100644 --- a/backend/.sqlx/query-15014ce696cf2af4f719a537a4e3ca5b322cc130a35a91f8b8854f5ebdf25ad2.json +++ b/backend/.sqlx/query-c7766afaea3e187824698cae2b090af9f49ded98cd1824f1ac91cc5dbe709bc1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n external_id,\n workspace_id,\n service_name AS \"service_name!: ServiceName\",\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n error,\n created_at,\n updated_at,\n summary\n FROM\n native_trigger\n WHERE\n workspace_id = $1\n AND service_name = $2\n AND external_id = $3\n ", + "query": "\n SELECT\n external_id,\n workspace_id,\n service_name AS \"service_name!: ServiceName\",\n script_path,\n is_flow,\n webhook_token_hash,\n service_config,\n error,\n created_at,\n updated_at,\n summary,\n enabled\n FROM\n native_trigger\n WHERE\n workspace_id = $1\n AND service_name = $2\n AND external_id = $3\n ", "describe": { "columns": [ { @@ -68,6 +68,11 @@ "ordinal": 10, "name": "summary", "type_info": "Varchar" + }, + { + "ordinal": 11, + "name": "enabled", + "type_info": "Bool" } ], "parameters": { @@ -99,8 +104,9 @@ true, false, false, - true + true, + false ] }, - "hash": "15014ce696cf2af4f719a537a4e3ca5b322cc130a35a91f8b8854f5ebdf25ad2" + "hash": "c7766afaea3e187824698cae2b090af9f49ded98cd1824f1ac91cc5dbe709bc1" } diff --git a/backend/.sqlx/query-fa42f89d9494056e1e7ad904b844203f1132aa5ae5eff9194c45e0259f4bee76.json b/backend/.sqlx/query-fa42f89d9494056e1e7ad904b844203f1132aa5ae5eff9194c45e0259f4bee76.json new file mode 100644 index 0000000000..bf8fa238a0 --- /dev/null +++ b/backend/.sqlx/query-fa42f89d9494056e1e7ad904b844203f1132aa5ae5eff9194c45e0259f4bee76.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE native_trigger\n SET enabled = $1\n WHERE\n workspace_id = $2\n AND service_name = $3\n AND external_id = $4\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Bool", + "Text", + { + "Custom": { + "name": "native_trigger_service", + "kind": { + "Enum": [ + "nextcloud", + "google", + "github" + ] + } + } + }, + "Text" + ] + }, + "nullable": [] + }, + "hash": "fa42f89d9494056e1e7ad904b844203f1132aa5ae5eff9194c45e0259f4bee76" +} diff --git a/backend/migrations/20260908113909_native_trigger_enabled.down.sql b/backend/migrations/20260908113909_native_trigger_enabled.down.sql new file mode 100644 index 0000000000..75f26b22c4 --- /dev/null +++ b/backend/migrations/20260908113909_native_trigger_enabled.down.sql @@ -0,0 +1 @@ +ALTER TABLE native_trigger DROP COLUMN IF EXISTS enabled; diff --git a/backend/migrations/20260908113909_native_trigger_enabled.up.sql b/backend/migrations/20260908113909_native_trigger_enabled.up.sql new file mode 100644 index 0000000000..15139498d7 --- /dev/null +++ b/backend/migrations/20260908113909_native_trigger_enabled.up.sql @@ -0,0 +1 @@ +ALTER TABLE native_trigger ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT true; diff --git a/backend/summarized_schema.txt b/backend/summarized_schema.txt index 67758cef32..a1e0b38518 100644 --- a/backend/summarized_schema.txt +++ b/backend/summarized_schema.txt @@ -149,7 +149,7 @@ mcp_oauth_server_code: code(char), client_id(char), user_email(char), workspace_ FK: (client_id) -> mcp_oauth_server_client(client_id) metrics: id(char), value(jsonb), created_at(ts) mqtt_trigger: mqtt_resource_path(char), subscribe_topics(jsonb[]), client_version(mqtt_client_version), v5_config(jsonb), v3_config(jsonb), client_id(char), path(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), server_id(char), last_server_ping(ts), error(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), mode(trigger_mode), labels(text[]) -native_trigger: external_id(char), workspace_id(char), service_name(native_trigger_service), script_path(char), is_flow(bool), webhook_token_hash(char), service_config(jsonb), error(text), created_at(ts), updated_at(ts) +native_trigger: external_id(char), workspace_id(char), service_name(native_trigger_service), script_path(char), is_flow(bool), webhook_token_hash(char), service_config(jsonb), error(text), created_at(ts), updated_at(ts), enabled(bool) FK: (workspace_id) -> workspace(id) nats_trigger: path(char), nats_resource_path(char), subjects(char), stream_name(char), consumer_name(char), use_jetstream(bool), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), server_id(char), last_server_ping(ts), error(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), mode(trigger_mode), labels(text[]) FK: (workspace_id) -> workspace(id) diff --git a/backend/windmill-api-integration-tests/tests/native_triggers.rs b/backend/windmill-api-integration-tests/tests/native_triggers.rs index 06c47cbb14..059b3aa2d2 100644 --- a/backend/windmill-api-integration-tests/tests/native_triggers.rs +++ b/backend/windmill-api-integration-tests/tests/native_triggers.rs @@ -18,15 +18,16 @@ use windmill_common::{ variables::{build_crypt, encrypt}, }; use windmill_native_triggers::{ - classify_read_failure, decrypt_oauth_data, delete_native_trigger, - delete_workspace_integration, get_workspace_integration, + classify_read_failure, decrypt_oauth_data, delete_native_trigger, delete_workspace_integration, + get_workspace_integration, github::GitHub, google::{parse_stop_channel_params, should_renew_channel}, - http_error_status, list_native_triggers, map_external_error, + grant_refused, http_error_status, list_native_triggers, map_external_error, + native_trigger_is_enabled, nextcloud::NextCloud, - grant_refused, require_native_integration_use, store_native_trigger, - store_workspace_integration, External, ExternalReadFailure, HttpRequestError, - NativeTriggerConfig, OAuthConfig, ServiceName, + require_native_integration_use, set_native_trigger_enabled, store_native_trigger, + store_workspace_integration, update_native_trigger, External, ExternalReadFailure, + HttpRequestError, NativeTriggerConfig, OAuthConfig, ServiceName, }; // ============================================================================ @@ -456,6 +457,7 @@ async fn test_delete_integration_full_cascade(db: Pool) -> anyhow::Res &trigger_config, json!({"triggerType": "drive"}), None, + true, ) .await?; @@ -546,6 +548,7 @@ async fn test_cleanup_preserves_triggers(db: Pool) -> anyhow::Result<( &trigger_config, json!({"triggerType": "drive"}), None, + true, ) .await?; @@ -603,6 +606,7 @@ async fn test_rename_moves_native_trigger(db: Pool) -> anyhow::Result< }, json!({"event": "OCP\\Files\\Events\\Node\\NodeCreatedEvent"}), None, + true, ) .await?; // An unrelated trigger already sitting on the target path must not be reported as moved. @@ -619,6 +623,7 @@ async fn test_rename_moves_native_trigger(db: Pool) -> anyhow::Result< }, json!({"event": "OCP\\Files\\Events\\Node\\NodeCreatedEvent"}), None, + true, ) .await?; @@ -784,7 +789,10 @@ fn test_refresh_failures_blame_only_the_grant_they_refuse() { let ok = Some(StatusCode::OK); assert!(grant_refused(ok, r#"{"error":"bad_refresh_token"}"#)); assert!(grant_refused(ok, r#"{"error":"invalid_grant"}"#)); - assert!(!grant_refused(ok, r#"{"access_token":"t","token_type":"bearer"}"#)); + assert!(!grant_refused( + ok, + r#"{"access_token":"t","token_type":"bearer"}"# + )); } /// A service that is busy or broken has not refused anything, and callers react differently to @@ -826,7 +834,9 @@ fn test_transient_service_failures_are_not_refusals() { body: r#"{"message":"Must have admin rights to Repository."}"#.to_string(), }); assert!( - map_external_error(refused).to_string().contains("admin rights"), + map_external_error(refused) + .to_string() + .contains("admin rights"), "a real 403 keeps its guidance" ); } @@ -850,3 +860,115 @@ fn test_only_service_failures_degrade_the_read() { "a non-provider error must pass through unmapped" ); } + +/// The pause switch a webhook delivery is gated on. A trigger arrives enabled, survives an +/// unrelated edit, and an unknown one reads as enabled so a delivery Windmill cannot place is +/// never silently dropped. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_native_trigger_enabled_toggle(db: Pool) -> anyhow::Result<()> { + insert_test_script(&db, "f/test/handler").await?; + let config = NativeTriggerConfig { + script_path: "f/test/handler".to_string(), + is_flow: false, + webhook_token: "abcdefghij1234567890".to_string(), + }; + store_native_trigger( + &db, + "test-workspace", + ServiceName::Nextcloud, + "ext-1", + &config, + json!({"event": "OCA\\Files\\Event\\LoadAdditionalScriptsEvent"}), + None, + true, + ) + .await?; + + assert!( + native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "ext-1").await?, + "a new trigger fires" + ); + + assert!( + set_native_trigger_enabled( + &db, + "test-workspace", + ServiceName::Nextcloud, + "ext-1", + false + ) + .await? + ); + assert!( + !native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "ext-1").await? + ); + + // Saving a configuration must not resume a trigger someone paused. + update_native_trigger( + &db, + "test-workspace", + ServiceName::Nextcloud, + "ext-1", + &config, + None, + Some("edited"), + ) + .await?; + assert!( + !native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "ext-1").await?, + "an edit leaves the pause in place" + ); + + // A recreate registers a fresh trigger and must be able to come up already paused, in one + // write, rather than being enabled for as long as it takes a second call to arrive. + store_native_trigger( + &db, + "test-workspace", + ServiceName::Nextcloud, + "ext-2", + &config, + json!({}), + None, + false, + ) + .await?; + assert!( + !native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "ext-2").await? + ); + + // The conflict branch is a re-registration of a trigger that already exists, so it carries no + // opinion about the pause. + store_native_trigger( + &db, + "test-workspace", + ServiceName::Nextcloud, + "ext-2", + &config, + json!({}), + None, + true, + ) + .await?; + assert!( + !native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "ext-2").await?, + "re-registering leaves the pause in place" + ); + + assert!( + !set_native_trigger_enabled( + &db, + "test-workspace", + ServiceName::Nextcloud, + "unknown", + true + ) + .await?, + "nothing to toggle" + ); + assert!( + native_trigger_is_enabled(&db, "test-workspace", ServiceName::Nextcloud, "unknown").await?, + "a trigger Windmill has no row for is not treated as paused" + ); + + Ok(()) +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 65a29c141d..ab10b9c19a 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -18952,6 +18952,49 @@ paths: schema: type: string + /w/{workspace}/native_triggers/{service_name}/setenabled/{external_id}: + post: + summary: set enabled state of native trigger + description: | + Enables or disables a native trigger. A disabled trigger stays registered on the + external service but starts no job when it fires. + Requires write access to the script or flow that the trigger is associated with. + operationId: setNativeTriggerEnabled + tags: + - native_trigger + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: service_name + in: path + required: true + schema: + $ref: "#/components/schemas/NativeServiceName" + - name: external_id + in: path + required: true + description: The external ID of the trigger from the external service + schema: + type: string + requestBody: + description: updated enabled state + required: true + content: + application/json: + schema: + type: object + properties: + enabled: + type: boolean + required: + - enabled + responses: + "200": + description: native trigger enabled state updated + content: + text/plain: + schema: + type: string + /w/{workspace}/native_triggers/{service_name}/list: get: summary: list native triggers @@ -35848,6 +35891,9 @@ components: type: string nullable: true description: Short summary to be displayed when listed + enabled: + type: boolean + description: Whether the trigger starts a job when it fires required: - external_id - workspace_id @@ -35855,6 +35901,7 @@ components: - script_path - is_flow - service_config + - enabled NativeTriggerWithExternal: type: object @@ -35886,6 +35933,9 @@ components: type: string nullable: true description: Short summary to be displayed when listed + enabled: + type: boolean + description: Whether the trigger starts a job when it fires external_data: type: object nullable: true @@ -35906,6 +35956,7 @@ components: - script_path - is_flow - service_config + - enabled - external_data WorkspaceIntegrations: @@ -35991,6 +36042,12 @@ components: type: string nullable: true description: Short summary to be displayed when listed + enabled: + type: boolean + description: >- + Whether the trigger starts a job when it fires. Honoured on create only, so a + trigger can be registered already paused; an update ignores it and setenabled is + the only way to change an existing trigger's state. Defaults to true. required: - script_path - is_flow diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 83a92bcd3c..9a3db7961f 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -6511,7 +6511,7 @@ pub async fn run_flow_by_path( Query(run_query): Query, args: RawWebhookArgs, ) -> error::Result<(StatusCode, String)> { - let (args, trigger_metadata) = get_args_and_trigger_metadata( + let (args, trigger_metadata) = match get_args_and_trigger_metadata( &db, &authed, RunnableId::from_flow_path(flow_path.to_path()), @@ -6519,7 +6519,15 @@ pub async fn run_flow_by_path( &w_id, args, ) - .await?; + .await? + { + WebhookRun::Run(args, trigger_metadata) => (args, trigger_metadata), + // 200 rather than an error: services drop a webhook that keeps failing, and disabling a + // trigger in Windmill must not cost it its registration. + WebhookRun::TriggerDisabled => { + return Ok((StatusCode::OK, NATIVE_TRIGGER_DISABLED_MSG.to_string())) + } + }; let (uuid, _, _, _) = push_flow_job_by_path_into_queue( authed, @@ -6944,7 +6952,7 @@ pub async fn run_script_by_path( Query(run_query): Query, args: RawWebhookArgs, ) -> error::Result<(StatusCode, String)> { - let (args, trigger_metadata) = get_args_and_trigger_metadata( + let (args, trigger_metadata) = match get_args_and_trigger_metadata( &db, &authed, RunnableId::from_script_path(script_path.to_path()), @@ -6952,7 +6960,13 @@ pub async fn run_script_by_path( &w_id, args, ) - .await?; + .await? + { + WebhookRun::Run(args, trigger_metadata) => (args, trigger_metadata), + WebhookRun::TriggerDisabled => { + return Ok((StatusCode::OK, NATIVE_TRIGGER_DISABLED_MSG.to_string())) + } + }; let (uuid, _, _) = push_script_job_by_path_into_queue( authed, @@ -6970,6 +6984,16 @@ pub async fn run_script_by_path( Ok((StatusCode::CREATED, uuid.to_string())) } +/// What a webhook delivery resolved to: the arguments to run with, or nothing to run. +pub enum WebhookRun { + Run(PushArgsOwned, Option), + /// The native trigger this delivery belongs to is disabled. + TriggerDisabled, +} + +const NATIVE_TRIGGER_DISABLED_MSG: &str = + "This trigger is disabled in Windmill, so no job was created"; + #[allow(unused)] pub async fn get_args_and_trigger_metadata( db: &DB, @@ -6978,14 +7002,21 @@ pub async fn get_args_and_trigger_metadata( run_query: &RunJobQuery, w_id: &str, args: RawWebhookArgs, -) -> error::Result<(PushArgsOwned, Option)> { +) -> error::Result { use windmill_common::triggers::TriggerMetadata; // Build trigger metadata if this is a native trigger request #[cfg(feature = "native_trigger")] let (trigger_metadata, native_args) = if let Some(service_name_str) = &run_query.service_name { - use crate::native_triggers::{prepare_native_trigger_args, ServiceName}; + use crate::native_triggers::{ + native_trigger_is_enabled, prepare_native_trigger_args, ServiceName, + }; let service_name = ServiceName::try_from(service_name_str.to_owned())?; + if let Some(external_id) = run_query.trigger_external_id.as_deref() { + if !native_trigger_is_enabled(db, w_id, service_name, external_id).await? { + return Ok(WebhookRun::TriggerDisabled); + } + } let metadata = Some(TriggerMetadata::new( run_query.trigger_external_id.clone(), service_name.as_job_trigger_kind(), @@ -7021,7 +7052,7 @@ pub async fn get_args_and_trigger_metadata( .await? }; - Ok((args, trigger_metadata)) + Ok(WebhookRun::Run(args, trigger_metadata)) } #[derive(Deserialize)] diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index 3a2a608369..e0128e9e7a 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -1434,9 +1434,11 @@ pub(crate) async fn tarball_workspace( // Native triggers (Nextcloud, Google Drive, GitHub) are never // cloned into a fork — a fork only has one if its owner created // it there, so it's always "fork-only" and keeps its own mode. - // No parent-value substitution applies; we only strip the - // webhook token hash. - let native_ignore_keys = vec!["webhook_token_hash"]; + // No parent-value substitution applies; we strip the webhook + // token hash, and `enabled`, which is operational state a sync + // deliberately does not carry — whether a trigger is paused + // belongs to the workspace it runs in, not to the code. + let native_ignore_keys = vec!["webhook_token_hash", "enabled"]; for trigger in native_triggers { let trigger_str = &to_string_without_metadata( diff --git a/backend/windmill-native-triggers/src/handler.rs b/backend/windmill-native-triggers/src/handler.rs index cc6c61be81..27a50ba07a 100644 --- a/backend/windmill-native-triggers/src/handler.rs +++ b/backend/windmill-native-triggers/src/handler.rs @@ -1,8 +1,8 @@ use crate::{ classify_read_failure, decrypt_oauth_data, delete_native_trigger, delete_token_by_hash, get_native_trigger, list_native_triggers, lock::TriggerLock, map_external_error, - map_external_error_with, rotate_webhook_token, store_native_trigger, - sync::EXTERNAL_TRIGGER_MISSING_ERROR, update_native_trigger_error, + map_external_error_with, rotate_webhook_token, set_native_trigger_enabled, + store_native_trigger, sync::EXTERNAL_TRIGGER_MISSING_ERROR, update_native_trigger_error, update_native_trigger_if_runnable_unchanged, webhook_token_label, webhook_token_scopes, External, ExternalReadFailure, NativeTrigger, NativeTriggerConfig, NativeTriggerData, ServiceName, @@ -239,6 +239,7 @@ async fn create_native_trigger( &config, service_config, data.summary.as_deref(), + data.enabled, ) .await?; @@ -603,6 +604,87 @@ async fn delete_native_trigger_handler( Ok(format!("Native trigger deleted")) } +#[derive(Debug, Deserialize)] +pub struct SetEnabledPayload { + pub enabled: bool, +} + +/// Pause or resume a trigger, without touching its registration on the external service. +/// +/// Leaving the webhook registered is what makes this reversible: services drop or deactivate a +/// subscription that keeps failing, so a paused trigger keeps answering deliveries normally and +/// simply starts no job. +async fn set_native_trigger_enabled_handler( + Extension(service_name): Extension, + authed: ApiAuthed, + Extension(db): Extension, + Extension(user_db): Extension, + Path((workspace_id, external_id)): Path<(String, String)>, + Json(payload): Json, +) -> Result { + let mut tx = user_db.begin(&authed).await?; + + let existing = get_native_trigger(&mut *tx, &workspace_id, service_name, &external_id) + .await? + .ok_or_else(|| Error::NotFound(format!("Native trigger not found: {}", external_id)))?; + + check_scopes(&authed, || { + format!("native_triggers:write:{}", &existing.script_path) + })?; + require_is_writer_on_runnable( + &authed, + &existing.script_path, + existing.is_flow, + &workspace_id, + db.clone(), + ) + .await?; + + let updated = set_native_trigger_enabled( + &mut *tx, + &workspace_id, + service_name, + &external_id, + payload.enabled, + ) + .await?; + + // The read above takes no row lock, so a concurrent delete can land in between; reporting + // success then would tell the caller a trigger that is gone had been paused. + if !updated { + return Err(Error::NotFound(format!( + "Native trigger not found: {}", + external_id + ))); + } + + audit_log( + &mut *tx, + &authed, + &format!( + "native_triggers.{}.{}", + service_name, + if payload.enabled { "enable" } else { "disable" } + ), + ActionKind::Update, + &workspace_id, + Some(&external_id), + None, + ) + .await?; + + tx.commit().await?; + + Ok(format!( + "Native trigger {}", + if payload.enabled { + "enabled" + } else { + "disabled" + } + )) +} + async fn list_native_triggers_handler( Extension(service_name): Extension, authed: ApiAuthed, @@ -642,6 +724,10 @@ pub fn service_routes(handler: T) -> Router { .route( "/delete/{external_id}", delete(delete_native_trigger_handler::), + ) + .route( + "/setenabled/{external_id}", + post(set_native_trigger_enabled_handler::), ); standard_routes diff --git a/backend/windmill-native-triggers/src/lib.rs b/backend/windmill-native-triggers/src/lib.rs index 4400091ed6..570319ee0e 100644 --- a/backend/windmill-native-triggers/src/lib.rs +++ b/backend/windmill-native-triggers/src/lib.rs @@ -226,6 +226,10 @@ pub struct NativeTrigger { pub created_at: DateTime, pub updated_at: DateTime, pub summary: Option, + /// Whether incoming webhooks for this trigger start a job. Operational state: a create sets + /// its initial value and `setenabled` is its only mutator afterwards, so saving a + /// configuration can never silently re-enable a trigger someone paused. + pub enabled: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -235,12 +239,20 @@ pub struct NativeTriggerConfig { pub webhook_token: String, } +fn default_true() -> bool { + true +} + #[derive(Debug, Serialize, Deserialize)] pub struct NativeTriggerData { pub script_path: String, pub is_flow: bool, pub service_config: C, pub summary: Option, + /// Honoured on create only, so a trigger can be registered already paused in one request. + /// An update ignores it: `setenabled` is the only way to change an existing trigger's state. + #[serde(default = "default_true")] + pub enabled: bool, } #[derive(Debug, Clone, FromRow, Serialize, Deserialize)] @@ -1179,11 +1191,15 @@ pub async fn store_native_trigger<'c, E: sqlx::Executor<'c, Database = Postgres> config: &NativeTriggerConfig, service_config: C, summary: Option<&str>, + enabled: bool, ) -> Result<()> { use windmill_common::auth::hash_token; let webhook_token_hash = hash_token(&config.webhook_token); + // `enabled` is set by the INSERT alone: writing it here rather than in a follow-up statement + // is what keeps a trigger created paused from ever being visible, and therefore runnable, in + // any other state. The conflict branch leaves it untouched for the mirror-image reason. sqlx::query!( r#" INSERT INTO native_trigger ( @@ -1194,9 +1210,10 @@ pub async fn store_native_trigger<'c, E: sqlx::Executor<'c, Database = Postgres> is_flow, webhook_token_hash, service_config, - summary + summary, + enabled ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8 + $1, $2, $3, $4, $5, $6, $7, $8, $9 ) ON CONFLICT (external_id, workspace_id, service_name) DO UPDATE SET script_path = $4, is_flow = $5, webhook_token_hash = $6, service_config = $7, summary = $8, error = NULL, updated_at = NOW() @@ -1209,6 +1226,7 @@ pub async fn store_native_trigger<'c, E: sqlx::Executor<'c, Database = Postgres> webhook_token_hash, sqlx::types::Json(service_config) as _, summary, + enabled, ) .execute(db) .await?; @@ -1405,7 +1423,8 @@ pub async fn get_native_trigger<'c, E: sqlx::Executor<'c, Database = Postgres>>( error, created_at, updated_at, - summary + summary, + enabled FROM native_trigger WHERE @@ -1444,7 +1463,8 @@ pub async fn get_native_trigger_by_script<'c, E: sqlx::Executor<'c, Database = P error, created_at, updated_at, - summary + summary, + enabled FROM native_trigger WHERE @@ -1491,7 +1511,8 @@ pub async fn list_native_triggers<'c, E: sqlx::Executor<'c, Database = Postgres> nt.error, nt.created_at, nt.updated_at, - nt.summary + nt.summary, + nt.enabled FROM native_trigger nt WHERE @@ -1555,6 +1576,71 @@ pub async fn update_native_trigger_error<'c, E: sqlx::Executor<'c, Database = Po Ok(()) } +/// Pause or resume a trigger. Returns `false` when there is no such trigger. +/// +/// Callers MUST have verified write access to the trigger's runnable: this writes operational +/// state and performs no authorization of its own. +pub async fn set_native_trigger_enabled<'c, E: sqlx::Executor<'c, Database = Postgres>>( + db: E, + workspace_id: &str, + service_name: ServiceName, + external_id: &str, + enabled: bool, +) -> Result { + // `updated_at` is the row version `record_reregistration` conditions on, so leave it alone: + // pausing a trigger must not make a registration that is mid-flight discard its result. + let updated = sqlx::query!( + r#" + UPDATE native_trigger + SET enabled = $1 + WHERE + workspace_id = $2 + AND service_name = $3 + AND external_id = $4 + "#, + enabled, + workspace_id, + service_name as ServiceName, + external_id, + ) + .execute(db) + .await? + .rows_affected(); + + Ok(updated > 0) +} + +/// Whether a webhook arriving for this trigger should start a job. +/// +/// A trigger Windmill no longer knows about counts as enabled: the token in the URL is what +/// authorizes the run, and this is a pause switch, not a second authorization check. It reads +/// nothing a caller could not already learn from the trigger it is delivering for, so it needs no +/// authorization of its own — but it also grants none, and must not be used as one. +pub async fn native_trigger_is_enabled<'c, E: sqlx::Executor<'c, Database = Postgres>>( + db: E, + workspace_id: &str, + service_name: ServiceName, + external_id: &str, +) -> Result { + let enabled = sqlx::query_scalar!( + r#" + SELECT enabled + FROM native_trigger + WHERE + workspace_id = $1 + AND service_name = $2 + AND external_id = $3 + "#, + workspace_id, + service_name as ServiceName, + external_id, + ) + .fetch_optional(db) + .await?; + + Ok(enabled.unwrap_or(true)) +} + pub async fn update_native_trigger_service_config< 'c, E: sqlx::Executor<'c, Database = Postgres>, @@ -1701,6 +1787,11 @@ pub async fn delete_workspace_integration( /// /// `external_id` is optional because during CREATE we don't have it yet /// (it's returned by the external service). During UPDATE, we have it. +/// +/// Every registered URL MUST end up carrying it: it is the only thing a delivery identifies its +/// trigger by, so a service that leaves it out ships a disable switch that silently does nothing. +/// A handler that returns `None` from `service_config_from_create_response` gets this for free — +/// `create_native_trigger` then runs the `update` cycle that re-registers with the assigned id. pub fn generate_webhook_service_url( base_url: &str, w_id: &str, diff --git a/backend/windmill-native-triggers/src/rename.rs b/backend/windmill-native-triggers/src/rename.rs index a7168f5c7e..caae99028c 100644 --- a/backend/windmill-native-triggers/src/rename.rs +++ b/backend/windmill-native-triggers/src/rename.rs @@ -135,6 +135,7 @@ async fn reregister_one( is_flow: trigger.is_flow, service_config, summary: trigger.summary.clone(), + enabled: trigger.enabled, }; // The token is scoped to the runnable path and only its hash is kept, so pointing the webhook diff --git a/cli/src/commands/trigger/trigger.ts b/cli/src/commands/trigger/trigger.ts index b011a0f0b7..4e6b0eeb16 100644 --- a/cli/src/commands/trigger/trigger.ts +++ b/cli/src/commands/trigger/trigger.ts @@ -224,9 +224,12 @@ export async function pushTrigger( } } +// `enabled` is operational state a sync deliberately does not carry: the server strips it from +// the workspace export and the push below never sends it, so a created trigger comes up enabled +// and pausing one stays a local decision. type NativeTriggerFile = Omit< NativeTrigger, - "external_id" | "workspace_id" | "error" + "external_id" | "workspace_id" | "error" | "enabled" >; export async function pushNativeTrigger( @@ -264,6 +267,7 @@ export async function pushNativeTrigger( service_config: result.service_config, error: result.error, summary: result.summary, + enabled: result.enabled, }; log.debug(`Native trigger ${serviceName}/${externalId} exists on remote`); } catch { diff --git a/frontend/src/lib/components/triggers/native/NativeTriggerEditor.svelte b/frontend/src/lib/components/triggers/native/NativeTriggerEditor.svelte index 66192f184d..b7bf3fa567 100644 --- a/frontend/src/lib/components/triggers/native/NativeTriggerEditor.svelte +++ b/frontend/src/lib/components/triggers/native/NativeTriggerEditor.svelte @@ -23,6 +23,7 @@ import GitHubTriggerForm from './services/github/GitHubTriggerForm.svelte' import TriggerEditorToolbar from '$lib/components/triggers/TriggerEditorToolbar.svelte' import { handleConfigChange, type Trigger } from '$lib/components/triggers/utils' + import type { TriggerMode } from '$lib/gen' import { deepEqual } from 'fast-equals' import type { Snippet } from 'svelte' import Alert from '$lib/components/common/alert/Alert.svelte' @@ -107,6 +108,7 @@ let loadError = $state(undefined) let externalError = $state(undefined) let retryEdit = $state<(() => void) | undefined>(undefined) + let enabled = $state(true) export function openNew( nis_flow?: boolean, @@ -136,6 +138,7 @@ loadError = undefined externalError = undefined retryEdit = undefined + enabled = true } export function openRecreate(nativeTrigger: ExtendedNativeTrigger) { @@ -163,6 +166,7 @@ loadError = undefined externalError = undefined retryEdit = undefined + enabled = nativeTrigger.enabled } export async function openEdit( @@ -196,6 +200,7 @@ scriptPath = '' initialScriptPath = '' summary = '' + enabled = true try { const fullTrigger = await NativeTriggerService.getNativeTrigger({ @@ -211,6 +216,7 @@ summary = fullTrigger.summary ?? '' externalData = fullTrigger.external_data externalError = fullTrigger.external_error ?? undefined + enabled = fullTrigger.enabled // Apply default values if provided (for draft triggers) if (defaultValues) { @@ -293,13 +299,41 @@ } }) + async function handleToggleMode(newMode: TriggerMode): Promise { + if (isNew || !externalId) { + return false + } + const previous = enabled + const next = newMode === 'enabled' + enabled = next + try { + await NativeTriggerService.setNativeTriggerEnabled({ + workspace: $workspaceStore!, + serviceName: service, + externalId, + requestBody: { enabled: next } + }) + } catch (err: any) { + enabled = previous + sendUserToast( + `Failed to ${next ? 'enable' : 'disable'} trigger: ${err.body ?? err.message}`, + true + ) + return false + } + sendUserToast(`${next ? 'Enabled' : 'Disabled'} ${serviceInfo?.serviceDisplayName} trigger`) + } + async function save(): Promise { loading = true const saveCfg = getSaveCfg() const newExternalId = await saveNativeTriggerFromCfg( service, externalId ?? '', - saveCfg, + // A recreate registers a fresh webhook under a new external id, so it would otherwise + // come back enabled: the create has to carry the pause, or the replacement is live + // before anything can pause it again. + isRecreate ? { ...saveCfg, enabled } : saveCfg, !isNew, $workspaceStore!, usedTriggerKinds @@ -396,7 +430,7 @@ {}} + onToggleMode={handleToggleMode} disableSuspendedMode={true} /> {/if} diff --git a/frontend/src/lib/components/triggers/native/NativeTriggerTable.svelte b/frontend/src/lib/components/triggers/native/NativeTriggerTable.svelte index 8f8b84cd57..6f28875b17 100644 --- a/frontend/src/lib/components/triggers/native/NativeTriggerTable.svelte +++ b/frontend/src/lib/components/triggers/native/NativeTriggerTable.svelte @@ -1,10 +1,11 @@