From cbda01aa6dba178a6f20d035db1fcef501126ec2 Mon Sep 17 00:00:00 2001 From: mfsiega <93014743+mfsiega@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:36:47 +0000 Subject: [PATCH] fix(core): Re-register the n8n Trigger on every publish through the publication service (#38153) Co-authored-by: Claude Fable 5.1 --- .../__tests__/trigger-diff.test.ts | 40 +++++++++++++++++ .../workflow-publication-applier.test.ts | 45 +++++++++++++++++++ .../src/workflows/publication/trigger-diff.ts | 21 ++++++++- .../workflow-publication-applier.ts | 4 +- .../tests/e2e/nodes/n8n-trigger.spec.ts | 15 +++++++ 5 files changed, 123 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/workflows/publication/__tests__/trigger-diff.test.ts b/packages/cli/src/workflows/publication/__tests__/trigger-diff.test.ts index a1a2b3cedc6..e7b7e2ba4cf 100644 --- a/packages/cli/src/workflows/publication/__tests__/trigger-diff.test.ts +++ b/packages/cli/src/workflows/publication/__tests__/trigger-diff.test.ts @@ -85,4 +85,44 @@ describe('computeTriggerDiff', () => { test('returns empty diff for two empty trigger sets', () => { expect(computeTriggerDiff([], [])).toEqual({ toAdd: new Set(), toRemove: new Set() }); }); + + describe('triggers that observe the publication itself', () => { + const n8nTrigger = makeNode('n8n', { + type: 'n8n-nodes-base.n8nTrigger', + parameters: { events: ['update'] }, + }); + + test('re-registers an unchanged n8n Trigger when the published version changed', () => { + const diff = computeTriggerDiff([n8nTrigger], [{ ...n8nTrigger }], { versionChanged: true }); + + expect(diff).toEqual({ toAdd: new Set(['n8n']), toRemove: new Set(['n8n']) }); + }); + + test('leaves an unchanged n8n Trigger running when the published version is the same', () => { + const diff = computeTriggerDiff([n8nTrigger], [{ ...n8nTrigger }], { versionChanged: false }); + + expect(diff).toEqual({ toAdd: new Set(), toRemove: new Set() }); + }); + + test('re-registers the deprecated Workflow Trigger the same way', () => { + const workflowTrigger = makeNode('legacy', { + type: 'n8n-nodes-base.workflowTrigger', + parameters: { events: ['update'] }, + }); + + const diff = computeTriggerDiff([workflowTrigger], [{ ...workflowTrigger }], { + versionChanged: true, + }); + + expect(diff).toEqual({ toAdd: new Set(['legacy']), toRemove: new Set(['legacy']) }); + }); + + test('does not force other unchanged trigger types when the published version changed', () => { + const schedule = makeNode('a'); + + const diff = computeTriggerDiff([schedule], [{ ...schedule }], { versionChanged: true }); + + expect(diff).toEqual({ toAdd: new Set(), toRemove: new Set() }); + }); + }); }); diff --git a/packages/cli/src/workflows/publication/__tests__/workflow-publication-applier.test.ts b/packages/cli/src/workflows/publication/__tests__/workflow-publication-applier.test.ts index c1e2ff681d6..24c6f16bcf4 100644 --- a/packages/cli/src/workflows/publication/__tests__/workflow-publication-applier.test.ts +++ b/packages/cli/src/workflows/publication/__tests__/workflow-publication-applier.test.ts @@ -757,6 +757,51 @@ describe('WorkflowPublicationApplier', () => { expect(callOrder).toEqual(['remove', 'invalidate', 'advance', 'refresh', 'add']); }); + describe('n8n Trigger', () => { + // Emits "Published Workflow Updated" from `trigger()` itself, so it only fires + // when re-registered: the diff must re-apply it on every version change. + const n8nTrigger = triggerNode('n8n', { + type: 'n8n-nodes-base.n8nTrigger', + parameters: { events: ['update'] }, + }); + + test('re-applies an unchanged n8n Trigger when the published version changes', async () => { + setTriggerSets([n8nTrigger], [{ ...n8nTrigger }]); + workflowTriggerActivator.activate.mockResolvedValue({ activated: ['n8n'], failures: [] }); + + const result = await applier.apply(makeRecord(), abort); + + expect(result.type).toBe('completed'); + expect(workflowTriggerActivator.deactivate).toHaveBeenCalledWith( + expect.objectContaining({ id: 'wf-1' }), + oldVersion, + new Set(['n8n']), + abort, + ); + expect(workflowTriggerActivator.activate).toHaveBeenCalledWith( + expect.objectContaining({ id: 'wf-1' }), + newVersion, + new Set(['n8n']), + 'update', + abort, + ); + }); + + test('leaves an unchanged n8n Trigger running when the record targets the already-published version', async () => { + setTriggerSets([n8nTrigger], [{ ...n8nTrigger }]); + workflowRepository.findOneBy.mockResolvedValue(makeWorkflow({ activeVersionId: 'v-2' })); + workflowPublishedVersionRepository.findOne.mockResolvedValue( + makePublishedVersion(newVersion), + ); + + const result = await applier.apply(makeRecord(), abort); + + expect(result.type).toBe('completed'); + expect(workflowTriggerActivator.deactivate).not.toHaveBeenCalled(); + expect(workflowTriggerActivator.activate).not.toHaveBeenCalled(); + }); + }); + test('completes carrying external teardown failures from removed triggers, still advancing', async () => { setTriggerSets([triggerNode('a'), triggerNode('b')], [triggerNode('a')]); const failure = { nodeName: 'b', error: new Error('remote unreachable') }; diff --git a/packages/cli/src/workflows/publication/trigger-diff.ts b/packages/cli/src/workflows/publication/trigger-diff.ts index a77462841c3..31d31414481 100644 --- a/packages/cli/src/workflows/publication/trigger-diff.ts +++ b/packages/cli/src/workflows/publication/trigger-diff.ts @@ -12,6 +12,20 @@ function registrationEqual(base: INode | undefined, target: INode | undefined): return isEqual(pick(base, registrationProps), pick(target, registrationProps)); } +// Before the workflow publication service, we re-registered every trigger on +// publication. With the new flow, we only re-register triggers that have been +// modified. However, some triggers relied on the old behaviour, so we force it +// for those triggers. +const ALWAYS_REREGISTER_TRIGGER_TYPES: ReadonlySet = new Set([ + 'n8n-nodes-base.n8nTrigger', + 'n8n-nodes-base.workflowTrigger', +]); + +export interface TriggerDiffOptions { + /** Whether the publish moves to a version other than the one currently running. */ + versionChanged?: boolean; +} + /** * The trigger nodes that need to be deregistered (`toRemove`) and registered * (`toAdd`) to move a workflow's active triggers from `oldTriggerNodes` to @@ -32,13 +46,14 @@ export interface TriggerDiff { export function computeTriggerDiff( oldTriggerNodes: INode[], newTriggerNodes: INode[], + { versionChanged = false }: TriggerDiffOptions = {}, ): TriggerDiff { const diff = compareWorkflowsNodes(oldTriggerNodes, newTriggerNodes, registrationEqual); const toAdd: Set = new Set(); const toRemove: Set = new Set(); - for (const [nodeId, { status }] of diff) { + for (const [nodeId, { status, node }] of diff) { switch (status) { case NodeDiffStatus.Added: toAdd.add(nodeId); @@ -51,6 +66,10 @@ export function computeTriggerDiff( toAdd.add(nodeId); break; case NodeDiffStatus.Eq: + if (versionChanged && ALWAYS_REREGISTER_TRIGGER_TYPES.has(node.type)) { + toRemove.add(nodeId); + toAdd.add(nodeId); + } break; } } diff --git a/packages/cli/src/workflows/publication/workflow-publication-applier.ts b/packages/cli/src/workflows/publication/workflow-publication-applier.ts index dcaef852487..8dd1614edf1 100644 --- a/packages/cli/src/workflows/publication/workflow-publication-applier.ts +++ b/packages/cli/src/workflows/publication/workflow-publication-applier.ts @@ -149,7 +149,9 @@ export class WorkflowPublicationApplier { const desiredTriggerNodes = this.workflowTriggerActivator.getEnabledTriggerNodes(newVersion); const triggerKinds = this.workflowTriggerActivator.getTriggerKinds(desiredTriggerNodes); - const { toAdd, toRemove } = computeTriggerDiff(oldTriggerNodes, desiredTriggerNodes); + const { toAdd, toRemove } = computeTriggerDiff(oldTriggerNodes, desiredTriggerNodes, { + versionChanged: oldVersion !== null && oldVersion.versionId !== newVersion.versionId, + }); this.logger.debug( `Calculated trigger diff for workflow publication: ${toAdd.size} to add, ${toRemove.size} to remove`, diff --git a/packages/testing/playwright/tests/e2e/nodes/n8n-trigger.spec.ts b/packages/testing/playwright/tests/e2e/nodes/n8n-trigger.spec.ts index 76557677336..8d8771ae810 100644 --- a/packages/testing/playwright/tests/e2e/nodes/n8n-trigger.spec.ts +++ b/packages/testing/playwright/tests/e2e/nodes/n8n-trigger.spec.ts @@ -4,6 +4,21 @@ import { nanoid } from 'nanoid'; import { test, expect } from '../../../fixtures/base'; +// The node emits from `trigger()` at registration time, so its events depend on +// how the publication path (re)registers triggers. Run against the publication +// service, which becomes the default. +test.use({ + capability: { + env: { + TEST_ISOLATION: 'n8n-trigger-publication-service', + N8N_USE_WORKFLOW_PUBLICATION_SERVICE: 'true', + // Activation is applied asynchronously by the publication outbox + // consumer, so poll frequently to keep the test fast. + N8N_WORKFLOW_PUBLICATION_OUTBOX_POLL_INTERVAL_MS: '250', + }, + }, +}); + type TriggerEventType = 'activate' | 'update'; const makeN8nTriggerWorkflow = (events: TriggerEventType[]) => {