Merge pull request #12 from Traction-Rec/release/0.4.2

Release 0.4.2

Author: JonnyPower

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "n8n-unihook"
-version = "0.4.1"
+version = "0.4.2"
 edition = "2024"
 description = "A Slack event router for n8n that enables multiple workflows to share a single Slack app webhook"
 license = "MIT"

scripts/run-integration-tests.sh

@@ -177,9 +177,12 @@ setup_n8n() {
     # Calculate expiration timestamp (1 year from now in milliseconds)
     local expires_at=$(($(date +%s) * 1000 + 365 * 24 * 60 * 60 * 1000))
 
+    # Scopes must match n8n's allowed API key scopes for the instance owner role
+    # (see n8n packages/@n8n/permissions/src/public-api-permissions.ee.ts — there is
+    # no workflow:execute; use workflow:activate/deactivate and execution:* instead).
     local api_key_response=$(curl -s -b "$cookie_jar" -X POST http://localhost:6789/rest/api-keys \
         -H "Content-Type: application/json" \
-        -d "{\"label\":\"integration-test-key\",\"scopes\":[\"workflow:create\",\"workflow:delete\",\"workflow:read\",\"workflow:update\",\"workflow:list\",\"workflow:execute\"],\"expiresAt\":$expires_at}")
+        -d "{\"label\":\"integration-test-key\",\"scopes\":[\"workflow:create\",\"workflow:delete\",\"workflow:read\",\"workflow:update\",\"workflow:list\",\"workflow:activate\",\"workflow:deactivate\",\"execution:read\",\"execution:list\"],\"expiresAt\":$expires_at}")
 
     # Extract raw API key from response
     N8N_API_KEY=$(echo "$api_key_response" | grep -o '"rawApiKey":"[^"]*"' | cut -d'"' -f4)

src/db.rs

@@ -1,6 +1,7 @@
 use parking_lot::Mutex;
 use rusqlite::{Connection, OptionalExtension};
-use tracing::{debug, info};
+use std::collections::HashSet;
+use tracing::{debug, info, warn};
 
 use crate::github::GitHubTriggerConfig;
 use crate::jira::JiraTriggerConfig;
@@ -49,6 +50,8 @@ pub struct SlackTriggerRow {
     pub watch_whole_workspace: bool,
 }
 
+type TriggerDedupSortKey = (String, bool, String, String);
+
 impl Database {
     /// Open (or create) the database at `path` and run migrations.
     /// Use `":memory:"` for an in-memory database (useful for tests).
@@ -203,10 +206,15 @@ impl Database {
 
     /// Replace all GitHub trigger rows with the supplied set (inside a
     /// transaction). This is called by the periodic sync job.
+    ///
+    /// If n8n returns multiple trigger nodes with the same `webhook_id`, only one
+    /// row per id is kept (active workflows win, then lexicographic `workflow_id`)
+    /// so the sync transaction does not abort with a UNIQUE constraint error.
     pub fn sync_github_triggers(
         &self,
         triggers: &[GitHubTriggerConfig],
     ) -> Result<(), rusqlite::Error> {
+        let triggers = dedupe_github_triggers(triggers);
         let mut conn = self.conn.lock();
         let tx = conn.transaction()?;
         tx.execute("DELETE FROM github_triggers", [])?;
@@ -216,7 +224,7 @@ impl Database {
                  (webhook_id, workflow_id, workflow_name, workflow_active, owner, repository, events) \
                  VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
             )?;
-            for t in triggers {
+            for t in &triggers {
                 let events_json =
                     serde_json::to_string(&t.events).unwrap_or_else(|_| "[]".to_string());
                 stmt.execute(rusqlite::params![
@@ -299,10 +307,14 @@ impl Database {
     // ── Jira triggers ───────────────────────────────────────────────────
 
     /// Replace all Jira trigger rows with the supplied set.
+    ///
+    /// Duplicate `webhook_id` values from n8n are collapsed to one row each so
+    /// the SQLite UNIQUE constraint cannot roll back the entire sync.
     pub fn sync_jira_triggers(
         &self,
         triggers: &[JiraTriggerConfig],
     ) -> Result<(), rusqlite::Error> {
+        let triggers = dedupe_jira_triggers(triggers);
         let mut conn = self.conn.lock();
         let tx = conn.transaction()?;
         tx.execute("DELETE FROM jira_triggers", [])?;
@@ -312,7 +324,7 @@ impl Database {
                  (webhook_id, workflow_id, workflow_name, workflow_active, events) \
                  VALUES (?1, ?2, ?3, ?4, ?5)",
             )?;
-            for t in triggers {
+            for t in &triggers {
                 let events_json =
                     serde_json::to_string(&t.events).unwrap_or_else(|_| "[]".to_string());
                 stmt.execute(rusqlite::params![
@@ -362,10 +374,14 @@ impl Database {
     // ── Slack triggers ──────────────────────────────────────────────────
 
     /// Replace all Slack trigger rows with the supplied set.
+    ///
+    /// Duplicate `webhook_id` values from n8n are collapsed to one row each so
+    /// the SQLite UNIQUE constraint cannot roll back the entire sync.
     pub fn sync_slack_triggers(
         &self,
         triggers: &[SlackTriggerConfig],
     ) -> Result<(), rusqlite::Error> {
+        let triggers = dedupe_slack_triggers(triggers);
         let mut conn = self.conn.lock();
         let tx = conn.transaction()?;
         tx.execute("DELETE FROM slack_triggers", [])?;
@@ -375,7 +391,7 @@ impl Database {
                  (webhook_id, workflow_id, workflow_name, workflow_active, event_type, channels, watch_whole_workspace) \
                  VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
             )?;
-            for t in triggers {
+            for t in &triggers {
                 let channels_json =
                     serde_json::to_string(&t.channels).unwrap_or_else(|_| "[]".to_string());
                 stmt.execute(rusqlite::params![
@@ -429,6 +445,96 @@ impl Database {
     }
 }
 
+fn dedupe_slack_triggers(triggers: &[SlackTriggerConfig]) -> Vec<SlackTriggerConfig> {
+    dedupe_by_webhook_id(
+        triggers.to_vec(),
+        |t| {
+            (
+                t.webhook_id.clone(),
+                t.workflow_active,
+                t.workflow_id.clone(),
+                t.workflow_name.clone(),
+            )
+        },
+        "Slack",
+    )
+}
+
+fn dedupe_github_triggers(triggers: &[GitHubTriggerConfig]) -> Vec<GitHubTriggerConfig> {
+    dedupe_by_webhook_id(
+        triggers.to_vec(),
+        |t| {
+            (
+                t.webhook_id.clone(),
+                t.workflow_active,
+                t.workflow_id.clone(),
+                t.workflow_name.clone(),
+            )
+        },
+        "GitHub",
+    )
+}
+
+fn dedupe_jira_triggers(triggers: &[JiraTriggerConfig]) -> Vec<JiraTriggerConfig> {
+    dedupe_by_webhook_id(
+        triggers.to_vec(),
+        |t| {
+            (
+                t.webhook_id.clone(),
+                t.workflow_active,
+                t.workflow_id.clone(),
+                t.workflow_name.clone(),
+            )
+        },
+        "Jira",
+    )
+}
+
+/// Stable sort then keep first row per `webhook_id` so SQLite `UNIQUE` sync never fails.
+fn dedupe_by_webhook_id<T, F>(mut rows: Vec<T>, key_fn: F, provider: &'static str) -> Vec<T>
+where
+    F: Fn(&T) -> TriggerDedupSortKey,
+{
+    let original = rows.len();
+    // Same webhook_id together; prefer active workflow, then stable workflow id/name.
+    rows.sort_by(|a, b| {
+        let (wa, active_a, ida, na) = key_fn(a);
+        let (wb, active_b, idb, nb) = key_fn(b);
+        wa.cmp(&wb)
+            .then_with(|| active_b.cmp(&active_a))
+            .then_with(|| ida.cmp(&idb))
+            .then_with(|| na.cmp(&nb))
+    });
+
+    let mut seen = HashSet::new();
+    let mut out = Vec::new();
+    for row in rows {
+        let (wid, _, wf_id, wf_name) = key_fn(&row);
+        if seen.contains(&wid) {
+            warn!(
+                provider = %provider,
+                webhook_id = %wid,
+                workflow_id = %wf_id,
+                workflow_name = %wf_name,
+                "Skipping trigger: duplicate webhook_id (another workflow kept for this id)"
+            );
+        } else {
+            seen.insert(wid);
+            out.push(row);
+        }
+    }
+    if out.len() < original {
+        warn!(
+            provider = %provider,
+            loaded = original,
+            kept = out.len(),
+            dropped = original - out.len(),
+            "Duplicate webhook_id across trigger nodes; assign unique webhook IDs in n8n if multiple workflows must receive the same provider stream"
+        );
+    }
+    out
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -659,4 +765,36 @@ mod tests {
         assert_eq!(rows[0].channels, vec!["C123"]);
         assert!(!rows[0].watch_whole_workspace);
     }
+
+    #[test]
+    fn test_sync_slack_triggers_dedupes_duplicate_webhook_id() {
+        let db = open_memory_db();
+
+        let triggers = vec![
+            SlackTriggerConfig {
+                webhook_id: "same".to_string(),
+                workflow_id: "wf-inactive".to_string(),
+                workflow_name: "Inactive dup".to_string(),
+                workflow_active: false,
+                event_type: "message".to_string(),
+                channels: vec![],
+                watch_whole_workspace: true,
+            },
+            SlackTriggerConfig {
+                webhook_id: "same".to_string(),
+                workflow_id: "wf-active".to_string(),
+                workflow_name: "Active dup".to_string(),
+                workflow_active: true,
+                event_type: "any_event".to_string(),
+                channels: vec![],
+                watch_whole_workspace: true,
+            },
+        ];
+        db.sync_slack_triggers(&triggers).unwrap();
+
+        let rows = db.query_slack_triggers().unwrap();
+        assert_eq!(rows.len(), 1);
+        assert_eq!(rows[0].workflow_name, "Active dup");
+        assert!(rows[0].workflow_active);
+    }
 }

tests/integration/common/n8n_client.rs

@@ -139,6 +139,8 @@ impl N8nTestClient {
             .as_millis() as i64
             + (365 * 24 * 60 * 60 * 1000);
 
+        // Must align with n8n OWNER_API_KEY_SCOPES (no workflow:execute — use
+        // workflow:activate/deactivate and execution:* for our test client).
         let api_key_payload = serde_json::json!({
             "label": "integration-test-key",
             "scopes": [
@@ -147,7 +149,10 @@ impl N8nTestClient {
                 "workflow:read",
                 "workflow:update",
                 "workflow:list",
-                "workflow:execute"
+                "workflow:activate",
+                "workflow:deactivate",
+                "execution:read",
+                "execution:list"
             ],
             "expiresAt": expires_at
         });