From 7efec492fc4f15d7aa1f4d8901324232bc9f0c16 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Thu, 4 Dec 2025 14:25:19 +0100 Subject: [PATCH 01/13] add telemetry events --- .../ai/assistant/builder.store.test.ts | 89 +++++++++++++++ .../features/ai/assistant/builder.store.ts | 72 +++++++++++- .../components/Agent/ExecuteMessage.test.ts | 33 +++++- .../components/Agent/ExecuteMessage.vue | 9 ++ .../components/Agent/NodeIssueItem.test.ts | 19 ++++ .../components/Agent/NodeIssueItem.vue | 7 ++ .../components/ParameterInput.test.ts | 105 ++++++++++++++++++ .../parameters/components/ParameterInput.vue | 15 +++ 8 files changed, 347 insertions(+), 2 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts index e328b275926ab..051c7cf58ff5d 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts @@ -1708,4 +1708,93 @@ describe('AI Builder store', () => { expect(builderStore.creditsRemaining).toBe(80); }); }); + + describe('trackWorkflowBuilderJourney', () => { + it('tracks event with workflow_id, session_id, and event_type', () => { + const builderStore = useBuilderStore(); + + builderStore.trackWorkflowBuilderJourney('user_clicked_todo'); + + expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'user_clicked_todo', + }); + }); + + it('includes event_properties when provided', () => { + const builderStore = useBuilderStore(); + + builderStore.trackWorkflowBuilderJourney('user_clicked_todo', { + node_type: 'n8n-nodes-base.httpRequest', + type: 'parameters', + }); + + expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'user_clicked_todo', + event_properties: { + node_type: 'n8n-nodes-base.httpRequest', + type: 'parameters', + }, + }); + }); + + it('omits event_properties when empty object provided', () => { + const builderStore = useBuilderStore(); + + builderStore.trackWorkflowBuilderJourney('field_focus_placeholder_in_ndv', {}); + + expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'field_focus_placeholder_in_ndv', + }); + }); + + it('omits event_properties when not provided', () => { + const builderStore = useBuilderStore(); + + builderStore.trackWorkflowBuilderJourney('no_placeholder_values_left'); + + expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'no_placeholder_values_left', + }); + }); + }); + + describe('isPlaceholderValue', () => { + it('returns true for placeholder values', () => { + const builderStore = useBuilderStore(); + + expect(builderStore.isPlaceholderValue('<__PLACEHOLDER_VALUE__API endpoint URL__>')).toBe( + true, + ); + expect(builderStore.isPlaceholderValue('<__PLACEHOLDER_VALUE__label__>')).toBe(true); + expect(builderStore.isPlaceholderValue('<__PLACEHOLDER_VALUE____>')).toBe(true); + }); + + it('returns false for non-placeholder strings', () => { + const builderStore = useBuilderStore(); + + expect(builderStore.isPlaceholderValue('regular string')).toBe(false); + expect(builderStore.isPlaceholderValue('')).toBe(false); + expect(builderStore.isPlaceholderValue('https://api.example.com')).toBe(false); + expect(builderStore.isPlaceholderValue('={{ $json.field }}')).toBe(false); + }); + + it('returns false for non-string values', () => { + const builderStore = useBuilderStore(); + + expect(builderStore.isPlaceholderValue(123)).toBe(false); + expect(builderStore.isPlaceholderValue(null)).toBe(false); + expect(builderStore.isPlaceholderValue(undefined)).toBe(false); + expect(builderStore.isPlaceholderValue({ key: 'value' })).toBe(false); + expect(builderStore.isPlaceholderValue(['array'])).toBe(false); + expect(builderStore.isPlaceholderValue(true)).toBe(false); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index ed5d4e5f28822..9f28b35a2f3e7 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -23,7 +23,7 @@ import { generateMessageId, createBuilderPayload } from './builder.utils'; import { useRootStore } from '@n8n/stores/useRootStore'; import type { WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; import pick from 'lodash/pick'; -import { type INodeExecutionData, jsonParse } from 'n8n-workflow'; +import { type INodeExecutionData, type ITelemetryTrackProperties, jsonParse } from 'n8n-workflow'; import { useToast } from '@/app/composables/useToast'; import { injectWorkflowState } from '@/app/composables/useWorkflowState'; import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; @@ -38,6 +38,26 @@ export const ENABLED_VIEWS = BUILDER_ENABLED_VIEWS; const PLACEHOLDER_PREFIX = '<__PLACEHOLDER_VALUE__'; const PLACEHOLDER_SUFFIX = '__>'; +/** + * Event types for the workflow_builder_journey telemetry event + */ +export type WorkflowBuilderJourneyEventType = + | 'user_clicked_todo' + | 'field_focus_placeholder_in_ndv' + | 'no_placeholder_values_left'; + +interface WorkflowBuilderJourneyEventProperties { + node_type?: string; + type?: string; +} + +interface WorkflowBuilderJourneyPayload extends ITelemetryTrackProperties { + workflow_id: string; + session_id: string; + event_type: WorkflowBuilderJourneyEventType; + event_properties?: WorkflowBuilderJourneyEventProperties; +} + interface PlaceholderDetail { path: string[]; label: string; @@ -59,6 +79,9 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { const creditsClaimed = ref(); const hasMessages = ref(false); + // Track the first time todos are cleared (no_placeholder_values_left) + const hadTodosTracked = ref(false); + const currentStreamingMessage = ref(); // Store dependencies @@ -762,6 +785,51 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { }, ); + /** + * Tracks workflow builder journey events for telemetry + * @param eventType - The type of event being tracked + * @param eventProperties - Optional event-specific attributes + */ + function trackWorkflowBuilderJourney( + eventType: WorkflowBuilderJourneyEventType, + eventProperties?: WorkflowBuilderJourneyEventProperties, + ) { + const payload: WorkflowBuilderJourneyPayload = { + workflow_id: workflowsStore.workflowId, + session_id: trackingSessionId.value, + event_type: eventType, + }; + + if (eventProperties && Object.keys(eventProperties).length > 0) { + payload.event_properties = eventProperties; + } + + telemetry.track('workflow_builder_journey', payload); + } + + watch( + workflowTodos, + (newTodos, oldTodos) => { + // Only track if we had todos before and now we don't + if (oldTodos && oldTodos.length > 0 && newTodos.length === 0 && hadTodosTracked.value) { + trackWorkflowBuilderJourney('no_placeholder_values_left'); + } + // Mark that we've seen todos (for tracking purposes) + if (newTodos.length > 0) { + hadTodosTracked.value = true; + } + }, + { deep: true }, + ); + + /** + * Checks if a value is a placeholder value + */ + function isPlaceholderValue(value: unknown): boolean { + if (typeof value !== 'string') return false; + return value.startsWith(PLACEHOLDER_PREFIX); + } + // Public API return { // State @@ -794,5 +862,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { updateBuilderCredits, getRunningTools, fetchSessionsMetadata, + trackWorkflowBuilderJourney, + isPlaceholderValue, }; }); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts index 022146d61feeb..3ad4a659e8244 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts @@ -18,6 +18,7 @@ import { useBuilderStore } from '../../builder.store'; const workflowValidationIssuesRef = ref< Array<{ node: string; type: string; value: string | string[] }> >([]); +const workflowTodosRef = ref>([]); const executionWaitingForWebhookRef = ref(false); const selectedTriggerNodeNameRef = ref(undefined); const hasNoCreditsRemainingRef = ref(false); @@ -68,7 +69,8 @@ const renderComponent = createComponentRenderer(ExecuteMessage); vi.mock('./NodeIssueItem.vue', () => ({ default: { - template: '
  • ', + template: '
  • ', + emits: ['click'], }, })); @@ -84,6 +86,7 @@ describe('ExecuteMessage', () => { runWorkflowMock.mockReset(); showMessageMock.mockReset(); workflowValidationIssuesRef.value = []; + workflowTodosRef.value = []; executionWaitingForWebhookRef.value = false; selectedTriggerNodeNameRef.value = undefined; hasNoCreditsRemainingRef.value = false; @@ -139,6 +142,10 @@ describe('ExecuteMessage', () => { Object.defineProperty(builderStore, 'hasNoCreditsRemaining', { get: () => hasNoCreditsRemainingRef.value, }); + Object.defineProperty(builderStore, 'workflowTodos', { + get: () => workflowTodosRef.value, + }); + builderStore.trackWorkflowBuilderJourney = vi.fn(); renderExecuteMessage = () => renderComponent({ pinia }); }); @@ -337,4 +344,28 @@ describe('ExecuteMessage', () => { // Should have both the existing issue and the placeholder issue expect(issueItems.length).toBeGreaterThan(0); }); + + it('tracks user_clicked_todo when clicking on an issue item', async () => { + const todoIssue = { node: 'HTTP Request', type: 'parameters', value: 'Missing URL' }; + workflowTodosRef.value = [todoIssue]; + workflowNodes.push({ + id: '2', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + position: [100, 0], + parameters: {}, + typeVersion: 1, + issues: {}, + }); + + const { getAllByTestId } = renderExecuteMessage(); + const issueItem = getAllByTestId('node-issue-item')[0]; + + await fireEvent.click(issueItem); + + expect(builderStore.trackWorkflowBuilderJourney).toHaveBeenCalledWith('user_clicked_todo', { + node_type: 'n8n-nodes-base.httpRequest', + type: 'parameters', + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue index 41bc7e216ef2a..75249e661c0e1 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue @@ -16,6 +16,7 @@ import { useToast } from '@/app/composables/useToast'; import { N8nTooltip } from '@n8n/design-system'; import { nextTick } from 'vue'; import { useBuilderStore } from '@/features/ai/assistant/builder.store'; +import { WorkflowValidationIssue } from '@/Interface'; interface Emits { /** Emitted when workflow execution completes */ @@ -141,6 +142,13 @@ function scrollIntoView() { }); } +function trackBuilderPlaceholders(issue: WorkflowValidationIssue) { + builderStore.trackWorkflowBuilderJourney('user_clicked_todo', { + node_type: workflowsStore.getNodeByName(issue.node)?.type, + type: issue.type, + }); +} + onMounted(scrollIntoView); onBeforeUnmount(() => { @@ -173,6 +181,7 @@ onBeforeUnmount(() => { :issue="issue" :get-node-type="getNodeTypeByName" :format-issue-message="formatIssueMessage" + @click="() => trackBuilderPlaceholders(issue)" /> diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.test.ts index fa5a07cda1118..07e7617a42f7d 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.test.ts @@ -63,4 +63,23 @@ describe('NodeIssueItem', () => { await fireEvent.click(getByLabelText('Edit Linear node')); expect(ndvStore.setActiveNodeName).toHaveBeenCalledWith('Linear', 'other'); }); + + it('emits click event when item is clicked', async () => { + const nodeType = { + name: 'n8n-nodes-base.linear', + displayName: 'Linear', + } as INodeTypeDescription; + const { getByLabelText, emitted } = renderComponent({ + pinia, + props: { + issue: { node: 'Linear', type: 'parameters', value: 'Missing API key' }, + getNodeType: vi.fn(() => nodeType), + formatIssueMessage, + }, + }); + + await fireEvent.click(getByLabelText('Edit Linear node')); + + expect(emitted().click).toHaveLength(1); + }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.vue b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.vue index 6f60bc6d13633..6a85dd22fa377 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.vue +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/NodeIssueItem.vue @@ -19,12 +19,19 @@ interface Props { formatIssueMessage: (value: WorkflowNodeIssue['value']) => string; } +interface Emits { + click: []; +} + const props = defineProps(); +const emit = defineEmits(); const ndvStore = useNDVStore(); function handleEditClick() { ndvStore.setActiveNodeName(props.issue.node, 'other'); + + emit('click'); } diff --git a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts index 705d99106ceea..71b020deede66 100644 --- a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts +++ b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts @@ -61,6 +61,9 @@ beforeEach(() => { mockNdvState = getNdvStateMock(); mockNodeTypesState = getNodeTypesStateMock(); mockCompletionResult = {}; + mockBuilderState.trackWorkflowBuilderJourney.mockClear(); + mockBuilderState.isPlaceholderValue.mockClear(); + mockBuilderState.isAIBuilderEnabled = true; }); vi.mock('@/features/ndv/shared/ndv.store', () => { @@ -95,6 +98,18 @@ vi.mock('vue-router', () => { }; }); +const mockBuilderState = { + trackWorkflowBuilderJourney: vi.fn(), + isPlaceholderValue: vi.fn(), + isAIBuilderEnabled: true, +}; + +vi.mock('@/features/ai/assistant/builder.store', () => { + return { + useBuilderStore: vi.fn(() => mockBuilderState), + }; +}); + const renderComponent = createComponentRenderer(ParameterInput, { pinia: createTestingPinia(), }); @@ -758,4 +773,94 @@ describe('ParameterInput.vue', () => { expect(rendered.queryByTestId('ndv-input-panel')).not.toBeInTheDocument(); }); }); + + describe('placeholder tracking', () => { + it('tracks field_focus_placeholder_in_ndv when focusing placeholder value', async () => { + mockBuilderState.isPlaceholderValue.mockReturnValue(true); + mockNdvState.activeNode = { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }; + + const rendered = renderComponent({ + props: { + path: 'url', + parameter: createTestNodeProperties({ name: 'url', type: 'string' }), + modelValue: '<__PLACEHOLDER_VALUE__API URL__>', + }, + }); + + await nextTick(); + const input = rendered.container.querySelector('input'); + if (input) { + await fireEvent.focus(input); + } + + expect(mockBuilderState.trackWorkflowBuilderJourney).toHaveBeenCalledWith( + 'field_focus_placeholder_in_ndv', + { node_type: 'n8n-nodes-base.httpRequest' }, + ); + }); + + it('does not track when value is not a placeholder', async () => { + mockBuilderState.isPlaceholderValue.mockReturnValue(false); + mockNdvState.activeNode = { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }; + + const rendered = renderComponent({ + props: { + path: 'url', + parameter: createTestNodeProperties({ name: 'url', type: 'string' }), + modelValue: 'https://api.example.com', + }, + }); + + await nextTick(); + const input = rendered.container.querySelector('input'); + if (input) { + await fireEvent.focus(input); + } + + expect(mockBuilderState.trackWorkflowBuilderJourney).not.toHaveBeenCalled(); + }); + + it('does not track when AI builder is disabled', async () => { + mockBuilderState.isPlaceholderValue.mockReturnValue(true); + mockBuilderState.isAIBuilderEnabled = false; + mockNdvState.activeNode = { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }; + + const rendered = renderComponent({ + props: { + path: 'url', + parameter: createTestNodeProperties({ name: 'url', type: 'string' }), + modelValue: '<__PLACEHOLDER_VALUE__API URL__>', + }, + }); + + await nextTick(); + const input = rendered.container.querySelector('input'); + if (input) { + await fireEvent.focus(input); + } + + expect(mockBuilderState.trackWorkflowBuilderJourney).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.vue b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.vue index d32c082127f34..4f5ababf48ab0 100644 --- a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.vue +++ b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.vue @@ -95,6 +95,7 @@ import ExperimentalEmbeddedNdvMapper from '@/features/workflows/canvas/experimen import { useExperimentalNdvStore } from '@/features/workflows/canvas/experimental/experimentalNdv.store'; import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; import { getParameterDisplayableOptions } from '@/app/utils/nodes/nodeTransforms'; +import { useBuilderStore } from '@/features/ai/assistant/builder.store'; import { ElColorPicker, ElDatePicker, ElDialog, ElSwitch } from 'element-plus'; import { N8nIcon, N8nInput, N8nInputNumber, N8nOption, N8nSelect } from '@n8n/design-system'; @@ -165,6 +166,7 @@ const uiStore = useUIStore(); const focusPanelStore = useFocusPanelStore(); const experimentalNdvStore = useExperimentalNdvStore(); const projectsStore = useProjectsStore(); +const builderStore = useBuilderStore(); const expressionLocalResolveCtx = inject(ExpressionLocalResolveContextSymbol, undefined); @@ -801,6 +803,18 @@ function selectInput() { } } +function trackBuilderPlaceholders() { + if ( + node.value && + builderStore.isPlaceholderValue(props.modelValue) && + builderStore.isAIBuilderEnabled + ) { + builderStore.trackWorkflowBuilderJourney('field_focus_placeholder_in_ndv', { + node_type: node.value.type, + }); + } +} + async function setFocus() { if (['json'].includes(props.parameter.type) && getTypeOption('alwaysOpenEditWindow')) { displayEditDialog(); @@ -830,6 +844,7 @@ async function setFocus() { } emit('focus'); + trackBuilderPlaceholders(); } function rgbaToHex(value: string): string | null { From 5de862022a92f706ec45c05a6b621d99d5d1d31d Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Thu, 4 Dec 2025 15:01:52 +0100 Subject: [PATCH 02/13] update tests --- .../components/Agent/ExecuteMessage.test.ts | 13 +++-- .../components/ParameterInput.test.ts | 51 +++++++++++-------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts index 3ad4a659e8244..300d09a530fd5 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.test.ts @@ -151,9 +151,9 @@ describe('ExecuteMessage', () => { }); it('disables execution when validation issues exist', () => { - workflowValidationIssuesRef.value = [ - { node: 'Start Trigger', type: 'parameters', value: 'Missing field' }, - ]; + const issue = { node: 'Start Trigger', type: 'parameters', value: 'Missing field' }; + workflowValidationIssuesRef.value = [issue]; + workflowTodosRef.value = [issue]; const { getAllByTestId, getByText } = renderExecuteMessage(); @@ -166,6 +166,9 @@ describe('ExecuteMessage', () => { workflowNodes[0].parameters = { url: '<__PLACEHOLDER_VALUE__API endpoint URL__>', }; + workflowTodosRef.value = [ + { node: 'Start Trigger', type: 'parameters', value: 'Fill in placeholder value' }, + ]; const { getAllByTestId, getByText } = renderExecuteMessage(); @@ -334,6 +337,10 @@ describe('ExecuteMessage', () => { url: ['Some other validation error'], }, }; + workflowTodosRef.value = [ + { node: 'Start Trigger', type: 'parameters', value: 'Some other validation error' }, + { node: 'Start Trigger', type: 'parameters', value: 'Fill in placeholder value' }, + ]; const { getAllByTestId, container } = renderExecuteMessage(); const button = getAllByTestId('execute-workflow-button')[0] as HTMLButtonElement; diff --git a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts index 71b020deede66..a509fc5a93122 100644 --- a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts +++ b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInput.test.ts @@ -777,13 +777,16 @@ describe('ParameterInput.vue', () => { describe('placeholder tracking', () => { it('tracks field_focus_placeholder_in_ndv when focusing placeholder value', async () => { mockBuilderState.isPlaceholderValue.mockReturnValue(true); - mockNdvState.activeNode = { - id: faker.string.uuid(), - name: 'Test Node', - parameters: {}, - position: [0, 0], - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, + mockNdvState = { + ...getNdvStateMock(), + activeNode: { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }, }; const rendered = renderComponent({ @@ -808,13 +811,16 @@ describe('ParameterInput.vue', () => { it('does not track when value is not a placeholder', async () => { mockBuilderState.isPlaceholderValue.mockReturnValue(false); - mockNdvState.activeNode = { - id: faker.string.uuid(), - name: 'Test Node', - parameters: {}, - position: [0, 0], - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, + mockNdvState = { + ...getNdvStateMock(), + activeNode: { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }, }; const rendered = renderComponent({ @@ -837,13 +843,16 @@ describe('ParameterInput.vue', () => { it('does not track when AI builder is disabled', async () => { mockBuilderState.isPlaceholderValue.mockReturnValue(true); mockBuilderState.isAIBuilderEnabled = false; - mockNdvState.activeNode = { - id: faker.string.uuid(), - name: 'Test Node', - parameters: {}, - position: [0, 0], - type: 'n8n-nodes-base.httpRequest', - typeVersion: 1, + mockNdvState = { + ...getNdvStateMock(), + activeNode: { + id: faker.string.uuid(), + name: 'Test Node', + parameters: {}, + position: [0, 0], + type: 'n8n-nodes-base.httpRequest', + typeVersion: 1, + }, }; const rendered = renderComponent({ From c033dc435157cb4e99a324dc0819be24f9ec4070 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Thu, 4 Dec 2025 16:53:17 +0100 Subject: [PATCH 03/13] add event --- .../__tests__/telemetry-event-relay.test.ts | 4 ++ .../cli/src/events/maps/relay.event-map.ts | 2 + .../events/relays/telemetry.event-relay.ts | 26 ++++++- .../cli/src/workflows/workflow.request.ts | 1 + .../cli/src/workflows/workflow.service.ts | 4 ++ .../cli/src/workflows/workflows.controller.ts | 3 +- .../@n8n/rest-api-client/src/api/workflows.ts | 1 + .../src/app/composables/useWorkflowSaving.ts | 5 ++ .../features/ai/assistant/builder.store.ts | 17 +++++ packages/workflow/src/workflow-diff.ts | 71 ++++++++++++++++++- 10 files changed, 130 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index 19b9dac30bd50..b48832c6c5d2c 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -882,6 +882,10 @@ describe('TelemetryEventRelay', () => { num_tags: 0, public_api: false, sharing_role: undefined, + meta: undefined, // workflow.meta is undefined in mock + workflow_edited_no_pos: false, + credential_edited: false, + ai_builder_assisted: false, }); }); diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index b2c3414eb7a1d..26a2813f7efa4 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -86,6 +86,8 @@ export type RelayEventMap = { user: UserLike; workflow: IWorkflowDb; publicApi: boolean; + previousWorkflow?: IWorkflowDb; + aiBuilderAssisted?: boolean; }; 'workflow-activated': { diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index 4428103c88317..e0aa042494f43 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -10,7 +10,7 @@ import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; import { snakeCase } from 'change-case'; import { BinaryDataConfig, InstanceSettings } from 'n8n-core'; import type { ExecutionStatus, INodesGraphResult, ITelemetryTrackProperties } from 'n8n-workflow'; -import { TelemetryHelpers } from 'n8n-workflow'; +import { hasCredentialChanges, hasNonPositionalChanges, TelemetryHelpers } from 'n8n-workflow'; import os from 'node:os'; import { get as pslGet } from 'psl'; @@ -621,7 +621,13 @@ export class TelemetryEventRelay extends EventRelay { }); } - private async workflowSaved({ user, workflow, publicApi }: RelayEventMap['workflow-saved']) { + private async workflowSaved({ + user, + workflow, + publicApi, + previousWorkflow, + aiBuilderAssisted, + }: RelayEventMap['workflow-saved']) { const isCloudDeployment = this.globalConfig.deployment.type === 'cloud'; const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes, { @@ -654,6 +660,19 @@ export class TelemetryEventRelay extends EventRelay { (note) => note.overlapping, ).length; + // Compute workflow_edited_no_pos and credential_edited if previous workflow is available + let workflowEditedNoPos = false; + let credentialEdited = false; + if (previousWorkflow) { + workflowEditedNoPos = hasNonPositionalChanges( + previousWorkflow.nodes, + workflow.nodes, + previousWorkflow.connections, + workflow.connections, + ); + credentialEdited = hasCredentialChanges(previousWorkflow.nodes, workflow.nodes); + } + this.telemetry.track('User saved workflow', { user_id: user.id, workflow_id: workflow.id, @@ -665,6 +684,9 @@ export class TelemetryEventRelay extends EventRelay { public_api: publicApi, sharing_role: userRole, meta: JSON.stringify(workflow.meta), + workflow_edited_no_pos: workflowEditedNoPos, + credential_edited: credentialEdited, + ai_builder_assisted: aiBuilderAssisted ?? false, }); } diff --git a/packages/cli/src/workflows/workflow.request.ts b/packages/cli/src/workflows/workflow.request.ts index 2d63c8f654c2f..738e027237d42 100644 --- a/packages/cli/src/workflows/workflow.request.ts +++ b/packages/cli/src/workflows/workflow.request.ts @@ -27,6 +27,7 @@ export declare namespace WorkflowRequest { projectId: string; parentFolderId?: string; uiContext?: string; + aiBuilderAssisted?: boolean; }>; // TODO: Use a discriminator when CAT-1809 lands diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index d9c683b470269..5fae59af30f14 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -226,6 +226,7 @@ export class WorkflowService { forceSave?: boolean; publicApi?: boolean; publishIfActive?: boolean; + aiBuilderAssisted?: boolean; } = {}, ): Promise { const { @@ -234,6 +235,7 @@ export class WorkflowService { forceSave = false, publicApi = false, publishIfActive = false, + aiBuilderAssisted = false, } = options; const workflow = await this.workflowFinderService.findWorkflowForUser( workflowId, @@ -414,6 +416,8 @@ export class WorkflowService { user, workflow: updatedWorkflow, publicApi, + previousWorkflow: workflow, + aiBuilderAssisted, }); // Activate workflow if requested, or diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index f528b645ea2b2..b4b7e7cdc7f6e 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -420,7 +420,7 @@ export class WorkflowsController { const forceSave = req.query.forceSave === 'true'; let updateData = new WorkflowEntity(); - const { tags, parentFolderId, ...rest } = req.body; + const { tags, parentFolderId, aiBuilderAssisted, ...rest } = req.body; // TODO: Add zod validation for entire `rest` object before assigning to `updateData` if ( @@ -445,6 +445,7 @@ export class WorkflowsController { tagIds: tags, parentFolderId, forceSave: isSharingEnabled ? forceSave : true, + aiBuilderAssisted, }); const scopes = await this.workflowService.getWorkflowScopes(req.user, workflowId); diff --git a/packages/frontend/@n8n/rest-api-client/src/api/workflows.ts b/packages/frontend/@n8n/rest-api-client/src/api/workflows.ts index 264f3e9b7f768..b1244541f3242 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/workflows.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/workflows.ts @@ -38,6 +38,7 @@ export interface WorkflowDataUpdate { meta?: WorkflowMetadata; parentFolderId?: string; uiContext?: string; + aiBuilderAssisted?: boolean; } export interface WorkflowDataCreate extends WorkflowDataUpdate { diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts index 0fe169ff1e05c..0db41e1f844ee 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts @@ -30,6 +30,7 @@ import { useTemplatesStore } from '@/features/workflows/templates/templates.stor import { useFocusPanelStore } from '@/app/stores/focusPanel.store'; import { injectWorkflowState, type WorkflowState } from '@/app/composables/useWorkflowState'; import { getResourcePermissions } from '@n8n/permissions'; +import { useBuilderStore } from '@/features/ai/assistant/builder.store'; export function useWorkflowSaving({ router, @@ -47,6 +48,7 @@ export function useWorkflowSaving({ const telemetry = useTelemetry(); const nodeHelpers = useNodeHelpers(); const templatesStore = useTemplatesStore(); + const builderStore = useBuilderStore(); const { getWorkflowDataToSave, checkConflictingWebhooks, getWorkflowProjectRole } = useWorkflowHelpers(); @@ -220,6 +222,9 @@ export function useWorkflowSaving({ workflowDataRequest.versionId = workflowsStore.workflowVersionId; + // Check if AI Builder made edits since last save (consumes and resets the flag) + workflowDataRequest.aiBuilderAssisted = builderStore.consumeAiBuilderMadeEdits(); + const deactivateReason = await getWorkflowDeactivationInfo( currentWorkflow, workflowDataRequest, diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index 9f28b35a2f3e7..ae7892b96e994 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -82,6 +82,9 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { // Track the first time todos are cleared (no_placeholder_values_left) const hadTodosTracked = ref(false); + // Track whether AI Builder made edits since last save (resets after each save) + const aiBuilderMadeEdits = ref(false); + const currentStreamingMessage = ref(); // Store dependencies @@ -722,6 +725,9 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { workflowData.pinData = restoredPinData; } + // Mark that AI Builder made edits (will be reset after save) + aiBuilderMadeEdits.value = true; + return { success: true, workflowData, @@ -734,6 +740,16 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { return JSON.stringify(pick(workflowsStore.workflow, ['nodes', 'connections'])); } + /** + * Returns true if AI Builder made edits since the last save, then resets the flag. + * This should be called when saving the workflow to include in telemetry. + */ + function consumeAiBuilderMadeEdits(): boolean { + const value = aiBuilderMadeEdits.value; + aiBuilderMadeEdits.value = false; + return value; + } + function updateBuilderCredits(quota?: number, claimed?: number) { creditsQuota.value = quota; creditsClaimed.value = claimed; @@ -864,5 +880,6 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { fetchSessionsMetadata, trackWorkflowBuilderJourney, isPlaceholderValue, + consumeAiBuilderMadeEdits, }; }); diff --git a/packages/workflow/src/workflow-diff.ts b/packages/workflow/src/workflow-diff.ts index a4a2375288643..d770954053d48 100644 --- a/packages/workflow/src/workflow-diff.ts +++ b/packages/workflow/src/workflow-diff.ts @@ -1,6 +1,7 @@ import isEqual from 'lodash/isEqual'; import pick from 'lodash/pick'; -import type { INode, IWorkflowBase } from '.'; + +import type { IConnections, INode, IWorkflowBase } from '.'; export type DiffableNode = Pick; export type DiffableWorkflow = { @@ -260,3 +261,71 @@ export function groupWorkflows( return diffs; } + +/** + * Checks if workflows have non-positional differences (changes to nodes or connections, + * excluding position changes). + * Returns true if there are meaningful changes, false if only positions changed. + */ +export function hasNonPositionalChanges( + oldNodes: INode[], + newNodes: INode[], + oldConnections: IConnections, + newConnections: IConnections, +): boolean { + // Check for node changes (compareNodes already excludes position) + const nodesDiff = compareWorkflowsNodes(oldNodes, newNodes); + for (const diff of nodesDiff.values()) { + if (diff.status !== NodeDiffStatus.Eq) { + return true; + } + } + + // Check for connection changes (connections don't have position data) + if (!isEqual(oldConnections, newConnections)) { + return true; + } + + return false; +} + +/** + * Checks if any credential IDs changed between old and new workflow nodes. + * Compares node by node - returns true if for any node: + * - A credential was added (new credential type not in old node) + * - A credential was removed (old credential type not in new node) + * - A credential was changed (same credential type but different credential ID) + */ +export function hasCredentialChanges(oldNodes: INode[], newNodes: INode[]): boolean { + const newNodesMap = new Map(newNodes.map((node) => [node.id, node])); + + for (const oldNode of oldNodes) { + const newNode = newNodesMap.get(oldNode.id); + + const oldCreds = oldNode.credentials ?? {}; + const newCreds = newNode?.credentials ?? {}; + + const oldCredTypes = Object.keys(oldCreds); + const newCredTypes = Object.keys(newCreds); + + // Check for removed credentials (in old but not in new) + for (const credType of oldCredTypes) { + if (!(credType in newCreds)) { + return true; // Credential removed + } + // Check for changed credentials (same type but different ID) + if (oldCreds[credType]?.id !== newCreds[credType]?.id) { + return true; // Credential changed + } + } + + // Check for added credentials (in new but not in old) + for (const credType of newCredTypes) { + if (!(credType in oldCreds)) { + return true; // Credential added + } + } + } + + return false; +} From ecaefdda8fc8319cd59d1fb625cb62dc29b03865 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 08:45:43 +0100 Subject: [PATCH 04/13] add unit tests --- packages/workflow/test/workflow-diff.test.ts | 300 ++++++++++++++++++- 1 file changed, 299 insertions(+), 1 deletion(-) diff --git a/packages/workflow/test/workflow-diff.test.ts b/packages/workflow/test/workflow-diff.test.ts index 301f37c45da37..d7c78d405e945 100644 --- a/packages/workflow/test/workflow-diff.test.ts +++ b/packages/workflow/test/workflow-diff.test.ts @@ -1,10 +1,12 @@ import { mock } from 'vitest-mock-extended'; -import { type INodeParameters, type IWorkflowBase } from '../src'; +import { type IConnections, type INode, type INodeParameters, type IWorkflowBase } from '../src'; import { compareNodes, compareWorkflowsNodes, groupWorkflows, + hasCredentialChanges, + hasNonPositionalChanges, NodeDiffStatus, RULES, WorkflowChangeSet, @@ -513,3 +515,299 @@ describe('groupWorkflows', () => { }); }); }); + +describe('hasNonPositionalChanges', () => { + const createNode = (id: string, overrides: Partial = {}): INode => ({ + id, + name: `Node ${id}`, + type: 'test-type', + typeVersion: 1, + position: [100, 200], + parameters: {}, + ...overrides, + }); + + it('should return false when only positions changed', () => { + const oldNodes = [createNode('1', { position: [100, 200] })]; + const newNodes = [createNode('1', { position: [300, 400] })]; + + const result = hasNonPositionalChanges(oldNodes, newNodes, {}, {}); + + expect(result).toBe(false); + }); + + it('should return true when a node is added', () => { + const oldNodes = [createNode('1')]; + const newNodes = [createNode('1'), createNode('2')]; + + const result = hasNonPositionalChanges(oldNodes, newNodes, {}, {}); + + expect(result).toBe(true); + }); + + it('should return true when a node is deleted', () => { + const oldNodes = [createNode('1'), createNode('2')]; + const newNodes = [createNode('1')]; + + const result = hasNonPositionalChanges(oldNodes, newNodes, {}, {}); + + expect(result).toBe(true); + }); + + it('should return true when node name changes', () => { + const oldNodes = [createNode('1', { name: 'Original Name' })]; + const newNodes = [createNode('1', { name: 'New Name' })]; + + const result = hasNonPositionalChanges(oldNodes, newNodes, {}, {}); + + expect(result).toBe(true); + }); + + it('should return true when node parameters change', () => { + const oldNodes = [createNode('1', { parameters: { param1: 'value1' } })]; + const newNodes = [createNode('1', { parameters: { param1: 'value2' } })]; + + const result = hasNonPositionalChanges(oldNodes, newNodes, {}, {}); + + expect(result).toBe(true); + }); + + it('should return true when connections change', () => { + const oldNodes = [createNode('1'), createNode('2')]; + const newNodes = [createNode('1'), createNode('2')]; + const oldConnections: IConnections = {}; + const newConnections: IConnections = { + 'Node 1': { + main: [[{ node: 'Node 2', type: 'main', index: 0 }]], + }, + }; + + const result = hasNonPositionalChanges(oldNodes, newNodes, oldConnections, newConnections); + + expect(result).toBe(true); + }); + + it('should return false when nodes and connections are identical', () => { + const oldNodes = [createNode('1'), createNode('2')]; + const newNodes = [createNode('1'), createNode('2')]; + const connections: IConnections = { + 'Node 1': { + main: [[{ node: 'Node 2', type: 'main', index: 0 }]], + }, + }; + + const result = hasNonPositionalChanges(oldNodes, newNodes, connections, connections); + + expect(result).toBe(false); + }); + + it('should return false for empty workflows', () => { + const result = hasNonPositionalChanges([], [], {}, {}); + + expect(result).toBe(false); + }); +}); + +describe('hasCredentialChanges', () => { + const createNode = (id: string, overrides: Partial = {}): INode => ({ + id, + name: `Node ${id}`, + type: 'test-type', + typeVersion: 1, + position: [100, 200], + parameters: {}, + ...overrides, + }); + + it('should return false when no credentials exist', () => { + const oldNodes = [createNode('1')]; + const newNodes = [createNode('1')]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(false); + }); + + it('should return false when credentials are identical', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(false); + }); + + it('should return true when credential is added to a node', () => { + const oldNodes = [createNode('1')]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should return true when credential is removed from a node', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + const newNodes = [createNode('1')]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should return true when credential ID changes', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-2', name: 'Test Credential' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should return false when only credential name changes but ID stays the same', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Old Name' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'New Name' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(false); + }); + + it('should return true when a new credential type is added', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { + testApi: { id: 'cred-1', name: 'Test Credential' }, + anotherApi: { id: 'cred-2', name: 'Another Credential' }, + }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should return true when a credential type is removed', () => { + const oldNodes = [ + createNode('1', { + credentials: { + testApi: { id: 'cred-1', name: 'Test Credential' }, + anotherApi: { id: 'cred-2', name: 'Another Credential' }, + }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should handle multiple nodes correctly', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + createNode('2', { + credentials: { testApi: { id: 'cred-2', name: 'Test Credential 2' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + createNode('2', { + credentials: { testApi: { id: 'cred-2', name: 'Test Credential 2' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(false); + }); + + it('should return true when credential changes in second node', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + createNode('2', { + credentials: { testApi: { id: 'cred-2', name: 'Test Credential 2' } }, + }), + ]; + const newNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + createNode('2', { + credentials: { testApi: { id: 'cred-3', name: 'Test Credential 3' } }, + }), + ]; + + const result = hasCredentialChanges(oldNodes, newNodes); + + expect(result).toBe(true); + }); + + it('should return false for empty node arrays', () => { + const result = hasCredentialChanges([], []); + + expect(result).toBe(false); + }); + + it('should handle node being deleted (not found in new nodes)', () => { + const oldNodes = [ + createNode('1', { + credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, + }), + ]; + const newNodes: INode[] = []; + + const result = hasCredentialChanges(oldNodes, newNodes); + + // When a node is deleted, its credentials are effectively removed + expect(result).toBe(true); + }); +}); From 8dfa4efb8b66bf7656d532fc690eff5264d82fe0 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 08:47:42 +0100 Subject: [PATCH 05/13] fix type --- .../features/ai/assistant/components/Agent/ExecuteMessage.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue index 75249e661c0e1..7e0faf96c2317 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue +++ b/packages/frontend/editor-ui/src/features/ai/assistant/components/Agent/ExecuteMessage.vue @@ -16,7 +16,7 @@ import { useToast } from '@/app/composables/useToast'; import { N8nTooltip } from '@n8n/design-system'; import { nextTick } from 'vue'; import { useBuilderStore } from '@/features/ai/assistant/builder.store'; -import { WorkflowValidationIssue } from '@/Interface'; +import type { WorkflowValidationIssue } from '@/Interface'; interface Emits { /** Emitted when workflow execution completes */ From 509021ca0bebdb9c89dad11f1b87f2afaba29072 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 08:51:54 +0100 Subject: [PATCH 06/13] fix type --- .../editor-ui/src/app/composables/useRunWorkflow.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts b/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts index 2f506a6a0801d..39293cd064d34 100644 --- a/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts @@ -48,6 +48,8 @@ vi.mock('@/app/stores/workflows.store', () => { nodesIssuesExist: false, executionWaitingForWebhook: false, workflowObject: { id: '123' } as Workflow, + workflowValidationIssues: [], + workflow: { nodes: [] }, getNodeByName: vi .fn() .mockImplementation((name) => From 9f749ca5a298c7417d8b0a3f04de2175ed146207 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 09:30:37 +0100 Subject: [PATCH 07/13] update tests --- .../src/app/composables/useRunWorkflow.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts b/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts index ca6c13a3c79df..c34fed6301f30 100644 --- a/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useRunWorkflow.test.ts @@ -46,7 +46,18 @@ vi.mock('@/app/stores/workflows.store', () => { executionWaitingForWebhook: false, workflowObject: { id: '123' } as Workflow, workflowValidationIssues: [], - workflow: { nodes: [] }, + workflow: { + nodes: [], + id: '', + name: '', + active: false, + isArchived: false, + createdAt: '', + updatedAt: '', + connections: {}, + versionId: '', + activeVersionId: null, + }, getNodeByName: vi .fn() .mockImplementation((name) => From 483982f7db1b5df737593cfe76e4206322eb6c5c Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 12:56:00 +0100 Subject: [PATCH 08/13] add event --- .../events/relays/telemetry.event-relay.ts | 14 +- packages/workflow/src/telemetry-helpers.ts | 89 ++++++++- .../workflow/test/telemetry-helpers.test.ts | 183 ++++++++++++++++++ 3 files changed, 280 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index e0aa042494f43..77345d1e91dce 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -835,13 +835,17 @@ export class TelemetryEventRelay extends EventRelay { manualExecEventProperties.is_managed = credential.isManaged; } } + const destinationNodeName = runData.data.startData?.destinationNode.nodeName; const telemetryPayload: ITelemetryTrackProperties = { ...manualExecEventProperties, - node_type: TelemetryHelpers.getNodeTypeForName( - workflow, - runData.data.startData?.destinationNode.nodeName, - )?.type, - node_id: nodeGraphResult.nameIndices[runData.data.startData?.destinationNode.nodeName], + node_type: TelemetryHelpers.getNodeTypeForName(workflow, destinationNodeName)?.type, + node_id: nodeGraphResult.nameIndices[destinationNodeName], + node_role: TelemetryHelpers.getNodeRole( + destinationNodeName, + workflow.connections, + this.nodeTypes, + workflow.nodes, + ), }; this.telemetry.track('Manual node exec finished', telemetryPayload); diff --git a/packages/workflow/src/telemetry-helpers.ts b/packages/workflow/src/telemetry-helpers.ts index 551adc0c5897e..612d2c5412c79 100644 --- a/packages/workflow/src/telemetry-helpers.ts +++ b/packages/workflow/src/telemetry-helpers.ts @@ -28,6 +28,7 @@ import { ApplicationError } from '@n8n/errors'; import type { NodeApiError } from './errors/node-api.error'; import type { IConnection, + IConnections, INode, INodeNameIndex, INodesGraph, @@ -42,7 +43,7 @@ import type { INodeParameterResourceLocator, } from './interfaces'; import { NodeConnectionTypes } from './interfaces'; -import { getNodeParameters } from './node-helpers'; +import { getNodeParameters, isSubNodeType } from './node-helpers'; import { jsonParse } from './utils'; import { DEFAULT_EVALUATION_METRIC } from './evaluation-helpers'; @@ -808,3 +809,89 @@ export function extractLastExecutedNodeStructuredOutputErrorInfo( return info; } + +export type NodeRole = 'trigger' | 'terminal' | 'internal'; + +/** + * Determines the role of a node in a workflow based on its connections. + * + * @param nodeName - The name of the node to check + * @param connections - The workflow connections (connectionsBySourceNode format) + * @param nodeTypes - The node types registry + * @param nodes - The workflow nodes + * @returns The role of the node: + * - 'trigger': Has no incoming main connections and is not a subnode + * - 'terminal': Has no outgoing connections + * - 'internal': Has both incoming and outgoing connections, or is a subnode + */ +export function getNodeRole( + nodeName: string, + connections: IConnections, + nodeTypes: INodeTypes, + nodes: INode[], +): NodeRole { + const hasOutgoingConnections = hasOutgoing(nodeName, connections); + const hasIncomingMainConnections = hasIncomingMain(nodeName, connections); + + // Check if node is a subnode based on its type description + const node = nodes.find((n) => n.name === nodeName); + const nodeTypeDescription = node + ? (nodeTypes.getByNameAndVersion(node.type, node.typeVersion)?.description ?? null) + : null; + const isSubnode = isSubNodeType(nodeTypeDescription); + + // Subnodes are always internal (they connect via AI connection types, not main) + if (isSubnode) { + return 'internal'; + } + + // A trigger has no incoming main connections + if (!hasIncomingMainConnections) { + return 'trigger'; + } + + // A terminal node has no outgoing connections + if (!hasOutgoingConnections) { + return 'terminal'; + } + + // Otherwise it's internal + return 'internal'; +} + +function hasOutgoing(nodeName: string, connections: IConnections): boolean { + const mainConnections = connections[nodeName]?.[NodeConnectionTypes.Main]; + if (!mainConnections) { + return false; + } + + for (const outputIndex of mainConnections) { + if (outputIndex && outputIndex.length > 0) { + return true; + } + } + + return false; +} + +function hasIncomingMain(nodeName: string, connections: IConnections): boolean { + // Check all source nodes to see if any connect to this node via main connection + for (const sourceNode of Object.keys(connections)) { + const sourceConnections = connections[sourceNode]; + const mainConnections = sourceConnections?.[NodeConnectionTypes.Main]; + + if (!mainConnections) continue; + + for (const outputIndex of mainConnections) { + if (!outputIndex) continue; + + for (const conn of outputIndex) { + if (conn.node === nodeName) { + return true; + } + } + } + } + + return false; +} diff --git a/packages/workflow/test/telemetry-helpers.test.ts b/packages/workflow/test/telemetry-helpers.test.ts index d6e51ad271500..1baeeb0514659 100644 --- a/packages/workflow/test/telemetry-helpers.test.ts +++ b/packages/workflow/test/telemetry-helpers.test.ts @@ -6,8 +6,10 @@ import type { NodeTypes } from './node-types'; import { STICKY_NODE_TYPE } from '../src/constants'; import { ApplicationError, ExpressionError, NodeApiError } from '../src/errors'; import type { + IConnections, INode, INodeTypeDescription, + INodeTypes, IRun, IRunData, NodeConnectionType, @@ -23,6 +25,7 @@ import { generateNodesGraph, getDomainBase, getDomainPath, + getNodeRole, resolveAIMetrics, resolveVectorStoreMetrics, userInInstanceRanOutOfFreeAiCredits, @@ -3306,3 +3309,183 @@ describe('extractLastExecutedNodeStructuredOutputErrorInfo', () => { }); }); }); + +describe('getNodeRole', () => { + const makeNode = (name: string, type: string, typeVersion = 1): INode => ({ + id: `${name}-id`, + name, + type, + typeVersion, + position: [0, 0], + parameters: {}, + }); + + describe('trigger role', () => { + it('should return trigger for node with no incoming main connections', () => { + const nodes = [ + makeNode('Trigger', 'n8n-nodes-base.manualTrigger'), + makeNode('Set', 'n8n-nodes-base.set'), + ]; + + const connections: IConnections = { + Trigger: { + [NodeConnectionTypes.Main]: [[{ node: 'Set', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const result = getNodeRole('Trigger', connections, nodeTypes, nodes); + expect(result).toBe('trigger'); + }); + + it('should return trigger for isolated node with no connections', () => { + const nodes = [makeNode('Trigger', 'n8n-nodes-base.manualTrigger')]; + + const connections: IConnections = {}; + + const result = getNodeRole('Trigger', connections, nodeTypes, nodes); + expect(result).toBe('trigger'); + }); + }); + + describe('terminal role', () => { + it('should return terminal for node with incoming but no outgoing connections', () => { + const nodes = [ + makeNode('Trigger', 'n8n-nodes-base.manualTrigger'), + makeNode('Set', 'n8n-nodes-base.set'), + ]; + + const connections: IConnections = { + Trigger: { + [NodeConnectionTypes.Main]: [[{ node: 'Set', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const result = getNodeRole('Set', connections, nodeTypes, nodes); + expect(result).toBe('terminal'); + }); + }); + + describe('internal role', () => { + it('should return internal for node with both incoming and outgoing connections', () => { + const nodes = [ + makeNode('Trigger', 'n8n-nodes-base.manualTrigger'), + makeNode('Middle', 'n8n-nodes-base.set'), + makeNode('End', 'n8n-nodes-base.set'), + ]; + + const connections: IConnections = { + Trigger: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Middle', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + Middle: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + const result = getNodeRole('Middle', connections, nodeTypes, nodes); + expect(result).toBe('internal'); + }); + + it('should return internal for subnode even without main connections', () => { + const nodes = [ + makeNode('Agent', '@n8n/n8n-nodes-langchain.agent'), + makeNode('Wikipedia', '@n8n/n8n-nodes-langchain.toolWikipedia'), + ]; + + const connections: IConnections = { + Wikipedia: { + [NodeConnectionTypes.AiTool]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiTool, index: 0 }], + ], + }, + }; + + const result = getNodeRole('Wikipedia', connections, nodeTypes, nodes); + expect(result).toBe('internal'); + }); + + it('should return internal for calculator tool subnode', () => { + const nodes = [ + makeNode('Agent', '@n8n/n8n-nodes-langchain.agent'), + makeNode('Calculator', '@n8n/n8n-nodes-langchain.toolCalculator'), + ]; + + const connections: IConnections = { + Calculator: { + [NodeConnectionTypes.AiTool]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiTool, index: 0 }], + ], + }, + }; + + const result = getNodeRole('Calculator', connections, nodeTypes, nodes); + expect(result).toBe('internal'); + }); + }); + + describe('edge cases', () => { + it('should return trigger for node not in connections', () => { + const nodes = [makeNode('Isolated', 'n8n-nodes-base.set')]; + const connections: IConnections = {}; + + const result = getNodeRole('Isolated', connections, nodeTypes, nodes); + expect(result).toBe('trigger'); + }); + + it('should handle node with empty output arrays', () => { + const nodes = [ + makeNode('Trigger', 'n8n-nodes-base.manualTrigger'), + makeNode('Set', 'n8n-nodes-base.set'), + ]; + + const connections: IConnections = { + Trigger: { + [NodeConnectionTypes.Main]: [[{ node: 'Set', type: NodeConnectionTypes.Main, index: 0 }]], + }, + Set: { + [NodeConnectionTypes.Main]: [[]], + }, + }; + + const result = getNodeRole('Set', connections, nodeTypes, nodes); + expect(result).toBe('terminal'); + }); + + it('should handle multiple output branches', () => { + const nodes = [ + makeNode('Trigger', 'n8n-nodes-base.manualTrigger'), + makeNode('Set1', 'n8n-nodes-base.set'), + makeNode('TrueBranch', 'n8n-nodes-base.set'), + makeNode('FalseBranch', 'n8n-nodes-base.set'), + ]; + + const connections: IConnections = { + Trigger: { + [NodeConnectionTypes.Main]: [ + [{ node: 'Set1', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + Set1: { + [NodeConnectionTypes.Main]: [ + [{ node: 'TrueBranch', type: NodeConnectionTypes.Main, index: 0 }], + [{ node: 'FalseBranch', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }; + + const result = getNodeRole('Set1', connections, nodeTypes, nodes); + expect(result).toBe('internal'); + }); + + it('should handle node not found in nodes array', () => { + const nodes = [makeNode('Trigger', 'n8n-nodes-base.manualTrigger')]; + const connections: IConnections = {}; + + // Node 'NotInArray' is not in the nodes array + const result = getNodeRole('NotInArray', connections, nodeTypes, nodes); + expect(result).toBe('trigger'); + }); + }); +}); From 82d2d6cb9a6e683d4ba1876e33f7f9b090d5ddee Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 13:24:49 +0100 Subject: [PATCH 09/13] update logic --- packages/cli/src/events/relays/telemetry.event-relay.ts | 1 - .../editor-ui/src/features/ai/assistant/builder.store.ts | 2 +- packages/workflow/src/workflow-diff.ts | 5 ++++- packages/workflow/test/workflow-diff.test.ts | 6 +++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index 77345d1e91dce..61610342110ae 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -660,7 +660,6 @@ export class TelemetryEventRelay extends EventRelay { (note) => note.overlapping, ).length; - // Compute workflow_edited_no_pos and credential_edited if previous workflow is available let workflowEditedNoPos = false; let credentialEdited = false; if (previousWorkflow) { diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index 3b0953db6b614..b151cf8f74590 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -839,7 +839,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { payload.event_properties = eventProperties; } - telemetry.track('workflow_builder_journey', payload); + telemetry.track('Workflow builder journey', payload); } watch( diff --git a/packages/workflow/src/workflow-diff.ts b/packages/workflow/src/workflow-diff.ts index d770954053d48..f9ce63e44f290 100644 --- a/packages/workflow/src/workflow-diff.ts +++ b/packages/workflow/src/workflow-diff.ts @@ -302,8 +302,11 @@ export function hasCredentialChanges(oldNodes: INode[], newNodes: INode[]): bool for (const oldNode of oldNodes) { const newNode = newNodesMap.get(oldNode.id); + // Skip nodes that were deleted - deletion is not a credential change + if (!newNode) continue; + const oldCreds = oldNode.credentials ?? {}; - const newCreds = newNode?.credentials ?? {}; + const newCreds = newNode.credentials ?? {}; const oldCredTypes = Object.keys(oldCreds); const newCredTypes = Object.keys(newCreds); diff --git a/packages/workflow/test/workflow-diff.test.ts b/packages/workflow/test/workflow-diff.test.ts index d7c78d405e945..912dc37a362a3 100644 --- a/packages/workflow/test/workflow-diff.test.ts +++ b/packages/workflow/test/workflow-diff.test.ts @@ -797,7 +797,7 @@ describe('hasCredentialChanges', () => { expect(result).toBe(false); }); - it('should handle node being deleted (not found in new nodes)', () => { + it('should return false when node is deleted (deletion is not a credential change)', () => { const oldNodes = [ createNode('1', { credentials: { testApi: { id: 'cred-1', name: 'Test Credential' } }, @@ -807,7 +807,7 @@ describe('hasCredentialChanges', () => { const result = hasCredentialChanges(oldNodes, newNodes); - // When a node is deleted, its credentials are effectively removed - expect(result).toBe(true); + // When a node is deleted, it's not considered a credential change + expect(result).toBe(false); }); }); From df760624a2cd70c4ff5311a9f7d6e6f2a3b78ffd Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 13:48:41 +0100 Subject: [PATCH 10/13] handle edge case of tools --- packages/workflow/src/telemetry-helpers.ts | 9 +++++++-- packages/workflow/test/telemetry-helpers.test.ts | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/workflow/src/telemetry-helpers.ts b/packages/workflow/src/telemetry-helpers.ts index 612d2c5412c79..81d78d323054f 100644 --- a/packages/workflow/src/telemetry-helpers.ts +++ b/packages/workflow/src/telemetry-helpers.ts @@ -830,8 +830,10 @@ export function getNodeRole( nodeTypes: INodeTypes, nodes: INode[], ): NodeRole { - const hasOutgoingConnections = hasOutgoing(nodeName, connections); - const hasIncomingMainConnections = hasIncomingMain(nodeName, connections); + // partial executions of tools get a special name + if (nodeName === 'PartialExecutionToolExecutor') { + return 'internal'; + } // Check if node is a subnode based on its type description const node = nodes.find((n) => n.name === nodeName); @@ -845,6 +847,9 @@ export function getNodeRole( return 'internal'; } + const hasOutgoingConnections = hasOutgoing(nodeName, connections); + const hasIncomingMainConnections = hasIncomingMain(nodeName, connections); + // A trigger has no incoming main connections if (!hasIncomingMainConnections) { return 'trigger'; diff --git a/packages/workflow/test/telemetry-helpers.test.ts b/packages/workflow/test/telemetry-helpers.test.ts index 1baeeb0514659..48dbdc9bdb325 100644 --- a/packages/workflow/test/telemetry-helpers.test.ts +++ b/packages/workflow/test/telemetry-helpers.test.ts @@ -3423,6 +3423,14 @@ describe('getNodeRole', () => { const result = getNodeRole('Calculator', connections, nodeTypes, nodes); expect(result).toBe('internal'); }); + + it('should return internal for PartialExecutionToolExecutor', () => { + const nodes: INode[] = []; + const connections: IConnections = {}; + + const result = getNodeRole('PartialExecutionToolExecutor', connections, nodeTypes, nodes); + expect(result).toBe('internal'); + }); }); describe('edge cases', () => { From b509108a50e7cef3a2e71e4406babe31ff5d55d3 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 14:11:46 +0100 Subject: [PATCH 11/13] update tests --- .../src/features/ai/assistant/builder.store.test.ts | 8 ++++---- .../editor-ui/src/features/ai/assistant/builder.store.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts index 3390c3b0c350f..5537585777cdf 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts @@ -1715,7 +1715,7 @@ describe('AI Builder store', () => { builderStore.trackWorkflowBuilderJourney('user_clicked_todo'); - expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + expect(track).toHaveBeenCalledWith('Workflow builder journey', { workflow_id: 'test-workflow-id', session_id: expect.any(String), event_type: 'user_clicked_todo', @@ -1730,7 +1730,7 @@ describe('AI Builder store', () => { type: 'parameters', }); - expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + expect(track).toHaveBeenCalledWith('Workflow builder journey', { workflow_id: 'test-workflow-id', session_id: expect.any(String), event_type: 'user_clicked_todo', @@ -1746,7 +1746,7 @@ describe('AI Builder store', () => { builderStore.trackWorkflowBuilderJourney('field_focus_placeholder_in_ndv', {}); - expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + expect(track).toHaveBeenCalledWith('Workflow builder journey', { workflow_id: 'test-workflow-id', session_id: expect.any(String), event_type: 'field_focus_placeholder_in_ndv', @@ -1758,7 +1758,7 @@ describe('AI Builder store', () => { builderStore.trackWorkflowBuilderJourney('no_placeholder_values_left'); - expect(track).toHaveBeenCalledWith('workflow_builder_journey', { + expect(track).toHaveBeenCalledWith('Workflow builder journey', { workflow_id: 'test-workflow-id', session_id: expect.any(String), event_type: 'no_placeholder_values_left', diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index b151cf8f74590..be46d1ac66e7c 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -39,7 +39,7 @@ const PLACEHOLDER_PREFIX = '<__PLACEHOLDER_VALUE__'; const PLACEHOLDER_SUFFIX = '__>'; /** - * Event types for the workflow_builder_journey telemetry event + * Event types for the Workflow builder journey telemetry event */ export type WorkflowBuilderJourneyEventType = | 'user_clicked_todo' From 66612597b9cf372672339fdc9285d7998bd94f95 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 14:25:38 +0100 Subject: [PATCH 12/13] add last message user id to journey event --- .../ai/assistant/builder.store.test.ts | 73 ++++++++++++++++++- .../features/ai/assistant/builder.store.ts | 10 +++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts index 5537585777cdf..f32f7e681e010 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts @@ -430,6 +430,20 @@ describe('AI Builder store', () => { builderStore.resetBuilderChat(); expect(builderStore.chatMessages).toEqual([]); expect(builderStore.builderThinkingMessage).toBeUndefined(); + + // Verify last_user_message_id is reset (tracked via trackWorkflowBuilderJourney) + track.mockClear(); + builderStore.trackWorkflowBuilderJourney('user_clicked_todo'); + expect(track).toHaveBeenCalledWith('Workflow builder journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'user_clicked_todo', + }); + // Should NOT have last_user_message_id after reset + expect(track).not.toHaveBeenCalledWith( + 'Workflow builder journey', + expect.objectContaining({ last_user_message_id: expect.any(String) }), + ); }); describe('isAIBuilderEnabled computed property', () => { @@ -1710,15 +1724,40 @@ describe('AI Builder store', () => { }); describe('trackWorkflowBuilderJourney', () => { - it('tracks event with workflow_id, session_id, and event_type', () => { + it('tracks event with workflow_id, session_id, and event_type (without last_user_message_id when no message sent)', () => { + const builderStore = useBuilderStore(); + + builderStore.trackWorkflowBuilderJourney('user_clicked_todo'); + + expect(track).toHaveBeenCalledWith('Workflow builder journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'user_clicked_todo', + }); + }); + + it('includes last_user_message_id after user sends a message', async () => { const builderStore = useBuilderStore(); + apiSpy.mockImplementationOnce((_ctx, _payload, onMessage, onDone) => { + onMessage({ + messages: [{ type: 'message', role: 'assistant', text: 'Hello!' }], + sessionId: 'test-session', + }); + onDone(); + }); + + builderStore.sendChatMessage({ text: 'test' }); + await vi.waitFor(() => expect(builderStore.streaming).toBe(false)); + + track.mockClear(); builderStore.trackWorkflowBuilderJourney('user_clicked_todo'); expect(track).toHaveBeenCalledWith('Workflow builder journey', { workflow_id: 'test-workflow-id', session_id: expect.any(String), event_type: 'user_clicked_todo', + last_user_message_id: expect.any(String), }); }); @@ -1764,6 +1803,38 @@ describe('AI Builder store', () => { event_type: 'no_placeholder_values_left', }); }); + + it('includes both event_properties and last_user_message_id when both are present', async () => { + const builderStore = useBuilderStore(); + + apiSpy.mockImplementationOnce((_ctx, _payload, onMessage, onDone) => { + onMessage({ + messages: [{ type: 'message', role: 'assistant', text: 'Hello!' }], + sessionId: 'test-session', + }); + onDone(); + }); + + builderStore.sendChatMessage({ text: 'test' }); + await vi.waitFor(() => expect(builderStore.streaming).toBe(false)); + + track.mockClear(); + builderStore.trackWorkflowBuilderJourney('user_clicked_todo', { + node_type: 'n8n-nodes-base.httpRequest', + type: 'parameters', + }); + + expect(track).toHaveBeenCalledWith('Workflow builder journey', { + workflow_id: 'test-workflow-id', + session_id: expect.any(String), + event_type: 'user_clicked_todo', + event_properties: { + node_type: 'n8n-nodes-base.httpRequest', + type: 'parameters', + }, + last_user_message_id: expect.any(String), + }); + }); }); describe('isPlaceholderValue', () => { diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index be46d1ac66e7c..b25c856fc9a3d 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -56,6 +56,7 @@ interface WorkflowBuilderJourneyPayload extends ITelemetryTrackProperties { session_id: string; event_type: WorkflowBuilderJourneyEventType; event_properties?: WorkflowBuilderJourneyEventProperties; + last_user_message_id?: string; } interface PlaceholderDetail { @@ -91,6 +92,9 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { const currentStreamingMessage = ref(); + // Track the last user message ID for telemetry + const lastUserMessageId = ref(); + // Store dependencies const settings = useSettingsStore(); const rootStore = useRootStore(); @@ -252,6 +256,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { chatMessages.value = clearMessages(); builderThinkingMessage.value = undefined; initialGeneration.value = false; + lastUserMessageId.value = undefined; } function incrementManualExecutionStats(type: 'success' | 'error') { @@ -508,6 +513,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { initialGeneration.value = options.initialGeneration; } const userMessageId = generateMessageId(); + lastUserMessageId.value = userMessageId; const currentWorkflowJson = getWorkflowSnapshot(); currentStreamingMessage.value = { @@ -839,6 +845,10 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { payload.event_properties = eventProperties; } + if (lastUserMessageId.value) { + payload.last_user_message_id = lastUserMessageId.value; + } + telemetry.track('Workflow builder journey', payload); } From 64d151d2f55b550faf9ae33e6f5cf1e86bcdffa0 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour Date: Fri, 5 Dec 2025 15:49:43 +0100 Subject: [PATCH 13/13] address cubic feedback --- .../src/app/composables/useWorkflowSaving.ts | 7 +++++-- .../ai/assistant/builder.store.test.ts | 18 ++++++++++++++++ .../features/ai/assistant/builder.store.ts | 21 ++++++++++++------- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts index 0db41e1f844ee..842c5cd17a5a6 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts @@ -222,8 +222,8 @@ export function useWorkflowSaving({ workflowDataRequest.versionId = workflowsStore.workflowVersionId; - // Check if AI Builder made edits since last save (consumes and resets the flag) - workflowDataRequest.aiBuilderAssisted = builderStore.consumeAiBuilderMadeEdits(); + // Check if AI Builder made edits since last save + workflowDataRequest.aiBuilderAssisted = builderStore.getAiBuilderMadeEdits(); const deactivateReason = await getWorkflowDeactivationInfo( currentWorkflow, @@ -260,6 +260,9 @@ export function useWorkflowSaving({ uiStore.removeActiveAction('workflowSaving'); void useExternalHooks().run('workflow.afterUpdate', { workflowData }); + // Reset AI Builder edits flag only after successful save + builderStore.resetAiBuilderMadeEdits(); + return true; } catch (error) { console.error(error); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts index f32f7e681e010..1df6ca1f5cfea 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.test.ts @@ -1857,6 +1857,24 @@ describe('AI Builder store', () => { expect(builderStore.isPlaceholderValue('={{ $json.field }}')).toBe(false); }); + it('returns false for malformed placeholders missing suffix', () => { + const builderStore = useBuilderStore(); + + // Has prefix but missing suffix - should be false + expect(builderStore.isPlaceholderValue('<__PLACEHOLDER_VALUE__missing suffix')).toBe(false); + expect(builderStore.isPlaceholderValue('<__PLACEHOLDER_VALUE__some text without end')).toBe( + false, + ); + }); + + it('returns false for malformed placeholders missing prefix', () => { + const builderStore = useBuilderStore(); + + // Has suffix but missing prefix - should be false + expect(builderStore.isPlaceholderValue('missing prefix__>')).toBe(false); + expect(builderStore.isPlaceholderValue('some text without start__>')).toBe(false); + }); + it('returns false for non-string values', () => { const builderStore = useBuilderStore(); diff --git a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts index b25c856fc9a3d..2e1301e777b69 100644 --- a/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts +++ b/packages/frontend/editor-ui/src/features/ai/assistant/builder.store.ts @@ -766,13 +766,19 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { } /** - * Returns true if AI Builder made edits since the last save, then resets the flag. - * This should be called when saving the workflow to include in telemetry. + * Returns true if AI Builder made edits since the last save. + * Use resetAiBuilderMadeEdits() after successful save to clear the flag. */ - function consumeAiBuilderMadeEdits(): boolean { - const value = aiBuilderMadeEdits.value; + function getAiBuilderMadeEdits(): boolean { + return aiBuilderMadeEdits.value; + } + + /** + * Resets the AI Builder edits flag. + * Should only be called after a successful workflow save. + */ + function resetAiBuilderMadeEdits(): void { aiBuilderMadeEdits.value = false; - return value; } function updateBuilderCredits(quota?: number, claimed?: number) { @@ -872,7 +878,7 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { */ function isPlaceholderValue(value: unknown): boolean { if (typeof value !== 'string') return false; - return value.startsWith(PLACEHOLDER_PREFIX); + return value.startsWith(PLACEHOLDER_PREFIX) && value.endsWith(PLACEHOLDER_SUFFIX); } // Public API @@ -908,7 +914,8 @@ export const useBuilderStore = defineStore(STORES.BUILDER, () => { fetchSessionsMetadata, trackWorkflowBuilderJourney, isPlaceholderValue, - consumeAiBuilderMadeEdits, + getAiBuilderMadeEdits, + resetAiBuilderMadeEdits, incrementManualExecutionStats, resetManualExecutionStats, };