From c6f6254c0e5197d4c0ba19aa52a1714e991a33b2 Mon Sep 17 00:00:00 2001
From: Omar Ajoue <krynble@gmail.com>
Date: Thu, 7 Mar 2024 09:25:12 +0000
Subject: [PATCH] fix: Always register webhooks on startup (#8830)

---
 .../cli/src/services/orchestration.service.ts |  6 +++-
 .../services/orchestration.service.test.ts    | 35 +++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts
index 45100bbe2bbb3..d80f4dee19d71 100644
--- a/packages/cli/src/services/orchestration.service.ts
+++ b/packages/cli/src/services/orchestration.service.ts
@@ -128,7 +128,11 @@ export class OrchestrationService {
 	 * Whether this instance may add webhooks to the `webhook_entity` table.
 	 */
 	shouldAddWebhooks(activationMode: WorkflowActivateMode) {
-		if (activationMode === 'init') return false;
+		// Always try to populate the webhook entity table as well as register the webhooks
+		// to prevent issues with users upgrading from a version < 1.15, where the webhook entity
+		// was cleared on shutdown to anything past 1.28.0, where we stopped populating it on init,
+		// causing all webhooks to break
+		if (activationMode === 'init') return true;
 
 		if (activationMode === 'leadershipChange') return false;
 
diff --git a/packages/cli/test/unit/services/orchestration.service.test.ts b/packages/cli/test/unit/services/orchestration.service.test.ts
index d755c73fcb17b..35ad3a53bef57 100644
--- a/packages/cli/test/unit/services/orchestration.service.test.ts
+++ b/packages/cli/test/unit/services/orchestration.service.test.ts
@@ -13,6 +13,7 @@ import { Logger } from '@/Logger';
 import { Push } from '@/push';
 import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
 import { mockInstance } from '../../shared/mocking';
+import type { WorkflowActivateMode } from 'n8n-workflow';
 
 const os = Container.get(OrchestrationService);
 const handler = Container.get(OrchestrationHandlerMainService);
@@ -149,4 +150,38 @@ describe('Orchestration Service', () => {
 		expect(res1!.payload).toBeUndefined();
 		expect(res2!.payload!.result).toEqual('debounced');
 	});
+
+	describe('shouldAddWebhooks', () => {
+		beforeEach(() => {
+			config.set('multiMainSetup.instanceType', 'leader');
+		});
+		test('should return true for init', () => {
+			// We want to ensure that webhooks are populated on init
+			// more https://github.com/n8n-io/n8n/pull/8830
+			const result = os.shouldAddWebhooks('init');
+			expect(result).toBe(true);
+		});
+
+		test('should return false for leadershipChange', () => {
+			const result = os.shouldAddWebhooks('leadershipChange');
+			expect(result).toBe(false);
+		});
+
+		test('should return true for update or activate when is leader', () => {
+			const modes = ['update', 'activate'] as WorkflowActivateMode[];
+			for (const mode of modes) {
+				const result = os.shouldAddWebhooks(mode);
+				expect(result).toBe(true);
+			}
+		});
+
+		test('should return false for update or activate when not leader', () => {
+			config.set('multiMainSetup.instanceType', 'follower');
+			const modes = ['update', 'activate'] as WorkflowActivateMode[];
+			for (const mode of modes) {
+				const result = os.shouldAddWebhooks(mode);
+				expect(result).toBe(false);
+			}
+		});
+	});
 });