diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts index ed0bb66f6..529a4e2f9 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts @@ -57,6 +57,21 @@ export function useChangeDetection({ } } + if (block.triggerMode) { + const triggerConfigValue = blockSubValues?.triggerConfig + if ( + triggerConfigValue && + typeof triggerConfigValue === 'object' && + !subBlocks.triggerConfig + ) { + subBlocks.triggerConfig = { + id: 'triggerConfig', + type: 'short-input', + value: triggerConfigValue, + } + } + } + blocksWithSubBlocks[blockId] = { ...block, subBlocks, diff --git a/apps/sim/lib/workflows/comparison/compare.test.ts b/apps/sim/lib/workflows/comparison/compare.test.ts index be7b6e9c5..5fe6e5923 100644 --- a/apps/sim/lib/workflows/comparison/compare.test.ts +++ b/apps/sim/lib/workflows/comparison/compare.test.ts @@ -2364,6 +2364,261 @@ describe('hasWorkflowChanged', () => { }) }) + describe('Trigger Config Normalization (False Positive Prevention)', () => { + it.concurrent( + 'should not detect change when deployed has null fields but current has values from triggerConfig', + () => { + // Core scenario: deployed state has null individual fields, current state has + // values populated from triggerConfig at runtime by populateTriggerFieldsFromConfig + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + botToken: { id: 'botToken', type: 'short-input', value: null }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123', botToken: 'token456' }, + }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'secret123' }, + botToken: { id: 'botToken', type: 'short-input', value: 'token456' }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123', botToken: 'token456' }, + }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + } + ) + + it.concurrent( + 'should detect change when user edits a trigger field to a different value', + () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'old-secret' }, + }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'new-secret' }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'old-secret' }, + }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(true) + } + ) + + it.concurrent('should not detect change when both sides have no triggerConfig', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + }) + + it.concurrent( + 'should not detect change when deployed has empty fields and triggerConfig populates them', + () => { + // Empty string is also treated as "empty" by normalizeTriggerConfigValues + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: '' }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'secret123' }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + } + ) + + it.concurrent('should not detect change when triggerId differs', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + model: { value: 'gpt-4' }, + triggerId: { value: null }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + model: { value: 'gpt-4' }, + triggerId: { value: 'slack_webhook' }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + }) + + it.concurrent( + 'should not detect change for namespaced system subBlock IDs like samplePayload_slack_webhook', + () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + model: { value: 'gpt-4' }, + samplePayload_slack_webhook: { value: 'old payload' }, + triggerInstructions_slack_webhook: { value: 'old instructions' }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + model: { value: 'gpt-4' }, + samplePayload_slack_webhook: { value: 'new payload' }, + triggerInstructions_slack_webhook: { value: 'new instructions' }, + }, + }), + }, + }) + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + } + ) + + it.concurrent( + 'should handle mixed scenario: some fields from triggerConfig, some user-edited', + () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + botToken: { id: 'botToken', type: 'short-input', value: null }, + includeFiles: { id: 'includeFiles', type: 'switch', value: false }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123', botToken: 'token456' }, + }, + }, + }), + }, + }) + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1', { + type: 'starter', + subBlocks: { + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'secret123' }, + botToken: { id: 'botToken', type: 'short-input', value: 'token456' }, + includeFiles: { id: 'includeFiles', type: 'switch', value: true }, + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123', botToken: 'token456' }, + }, + }, + }), + }, + }) + + // includeFiles changed from false to true — this IS a real change + expect(hasWorkflowChanged(currentState, deployedState)).toBe(true) + } + ) + }) + describe('Trigger Runtime Metadata (Should Not Trigger Change)', () => { it.concurrent('should not detect change when webhookId differs', () => { const deployedState = createWorkflowState({ diff --git a/apps/sim/lib/workflows/comparison/compare.ts b/apps/sim/lib/workflows/comparison/compare.ts index ce37dd86a..f739f3f20 100644 --- a/apps/sim/lib/workflows/comparison/compare.ts +++ b/apps/sim/lib/workflows/comparison/compare.ts @@ -9,6 +9,7 @@ import { normalizeLoop, normalizeParallel, normalizeSubBlockValue, + normalizeTriggerConfigValues, normalizeValue, normalizeVariables, sanitizeVariable, @@ -172,14 +173,18 @@ export function generateWorkflowDiffSummary( } } + // Normalize trigger config values for both states before comparison + const normalizedCurrentSubs = normalizeTriggerConfigValues(currentSubBlocks) + const normalizedPreviousSubs = normalizeTriggerConfigValues(previousSubBlocks) + // Compare subBlocks using shared helper for filtering (single source of truth) const allSubBlockIds = filterSubBlockIds([ - ...new Set([...Object.keys(currentSubBlocks), ...Object.keys(previousSubBlocks)]), + ...new Set([...Object.keys(normalizedCurrentSubs), ...Object.keys(normalizedPreviousSubs)]), ]) for (const subId of allSubBlockIds) { - const currentSub = currentSubBlocks[subId] as Record | undefined - const previousSub = previousSubBlocks[subId] as Record | undefined + const currentSub = normalizedCurrentSubs[subId] as Record | undefined + const previousSub = normalizedPreviousSubs[subId] as Record | undefined if (!currentSub || !previousSub) { changes.push({ diff --git a/apps/sim/lib/workflows/comparison/normalize.test.ts b/apps/sim/lib/workflows/comparison/normalize.test.ts index 6c2cc5eb1..2cf9b925a 100644 --- a/apps/sim/lib/workflows/comparison/normalize.test.ts +++ b/apps/sim/lib/workflows/comparison/normalize.test.ts @@ -4,10 +4,12 @@ import { describe, expect, it } from 'vitest' import type { Loop, Parallel } from '@/stores/workflows/workflow/types' import { + filterSubBlockIds, normalizedStringify, normalizeEdge, normalizeLoop, normalizeParallel, + normalizeTriggerConfigValues, normalizeValue, sanitizeInputFormat, sanitizeTools, @@ -584,4 +586,214 @@ describe('Workflow Normalization Utilities', () => { expect(result2).toBe(result3) }) }) + + describe('filterSubBlockIds', () => { + it.concurrent('should exclude exact SYSTEM_SUBBLOCK_IDS', () => { + const ids = ['signingSecret', 'samplePayload', 'triggerInstructions', 'botToken'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['botToken', 'signingSecret']) + }) + + it.concurrent('should exclude namespaced SYSTEM_SUBBLOCK_IDS (prefix matching)', () => { + const ids = [ + 'signingSecret', + 'samplePayload_slack_webhook', + 'triggerInstructions_slack_webhook', + 'webhookUrlDisplay_slack_webhook', + 'botToken', + ] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['botToken', 'signingSecret']) + }) + + it.concurrent('should exclude exact TRIGGER_RUNTIME_SUBBLOCK_IDS', () => { + const ids = ['webhookId', 'triggerPath', 'triggerConfig', 'triggerId', 'signingSecret'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['signingSecret']) + }) + + it.concurrent('should not exclude IDs that merely contain a system ID substring', () => { + const ids = ['mySamplePayload', 'notSamplePayload'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['mySamplePayload', 'notSamplePayload']) + }) + + it.concurrent('should return sorted results', () => { + const ids = ['zebra', 'alpha', 'middle'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['alpha', 'middle', 'zebra']) + }) + + it.concurrent('should handle empty array', () => { + expect(filterSubBlockIds([])).toEqual([]) + }) + + it.concurrent('should handle all IDs being excluded', () => { + const ids = ['webhookId', 'triggerPath', 'samplePayload', 'triggerConfig'] + const result = filterSubBlockIds(ids) + expect(result).toEqual([]) + }) + + it.concurrent('should exclude setupScript and scheduleInfo namespaced variants', () => { + const ids = ['setupScript_google_sheets_row', 'scheduleInfo_cron_trigger', 'realField'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['realField']) + }) + + it.concurrent('should exclude triggerCredentials namespaced variants', () => { + const ids = ['triggerCredentials_slack_webhook', 'signingSecret'] + const result = filterSubBlockIds(ids) + expect(result).toEqual(['signingSecret']) + }) + }) + + describe('normalizeTriggerConfigValues', () => { + it.concurrent('should return subBlocks unchanged when no triggerConfig exists', () => { + const subBlocks = { + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'secret123' }, + botToken: { id: 'botToken', type: 'short-input', value: 'token456' }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect(result).toEqual(subBlocks) + }) + + it.concurrent('should return subBlocks unchanged when triggerConfig value is null', () => { + const subBlocks = { + triggerConfig: { id: 'triggerConfig', type: 'short-input', value: null }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect(result).toEqual(subBlocks) + }) + + it.concurrent( + 'should return subBlocks unchanged when triggerConfig value is not an object', + () => { + const subBlocks = { + triggerConfig: { id: 'triggerConfig', type: 'short-input', value: 'string-value' }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect(result).toEqual(subBlocks) + } + ) + + it.concurrent('should populate null individual fields from triggerConfig', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123', botToken: 'token456' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + botToken: { id: 'botToken', type: 'short-input', value: null }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect((result.signingSecret as Record).value).toBe('secret123') + expect((result.botToken as Record).value).toBe('token456') + }) + + it.concurrent('should populate undefined individual fields from triggerConfig', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: undefined }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect((result.signingSecret as Record).value).toBe('secret123') + }) + + it.concurrent('should populate empty string individual fields from triggerConfig', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: '' }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect((result.signingSecret as Record).value).toBe('secret123') + }) + + it.concurrent('should NOT overwrite existing non-empty individual field values', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'old-secret' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: 'user-edited-secret' }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect((result.signingSecret as Record).value).toBe('user-edited-secret') + }) + + it.concurrent('should skip triggerConfig fields that are null/undefined', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: null, botToken: undefined }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + botToken: { id: 'botToken', type: 'short-input', value: null }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect((result.signingSecret as Record).value).toBe(null) + expect((result.botToken as Record).value).toBe(null) + }) + + it.concurrent('should skip fields from triggerConfig that have no matching subBlock', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { nonExistentField: 'value123' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + } + const result = normalizeTriggerConfigValues(subBlocks) + expect(result.nonExistentField).toBeUndefined() + expect((result.signingSecret as Record).value).toBe(null) + }) + + it.concurrent('should not mutate the original subBlocks object', () => { + const original = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + signingSecret: { id: 'signingSecret', type: 'short-input', value: null }, + } + normalizeTriggerConfigValues(original) + expect((original.signingSecret as Record).value).toBe(null) + }) + + it.concurrent('should preserve other subBlock properties when populating value', () => { + const subBlocks = { + triggerConfig: { + id: 'triggerConfig', + type: 'short-input', + value: { signingSecret: 'secret123' }, + }, + signingSecret: { + id: 'signingSecret', + type: 'short-input', + value: null, + placeholder: 'Enter signing secret', + }, + } + const result = normalizeTriggerConfigValues(subBlocks) + const normalized = result.signingSecret as Record + expect(normalized.value).toBe('secret123') + expect(normalized.id).toBe('signingSecret') + expect(normalized.type).toBe('short-input') + expect(normalized.placeholder).toBe('Enter signing secret') + }) + }) }) diff --git a/apps/sim/lib/workflows/comparison/normalize.ts b/apps/sim/lib/workflows/comparison/normalize.ts index dc414e25b..4a8ce18a2 100644 --- a/apps/sim/lib/workflows/comparison/normalize.ts +++ b/apps/sim/lib/workflows/comparison/normalize.ts @@ -418,10 +418,48 @@ export function extractBlockFieldsForComparison(block: BlockState): ExtractedBlo */ export function filterSubBlockIds(subBlockIds: string[]): string[] { return subBlockIds - .filter((id) => !SYSTEM_SUBBLOCK_IDS.includes(id) && !TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(id)) + .filter((id) => { + if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(id)) return false + if (SYSTEM_SUBBLOCK_IDS.some((sysId) => id === sysId || id.startsWith(`${sysId}_`))) + return false + return true + }) .sort() } +/** + * Normalizes trigger block subBlocks by populating null/empty individual fields + * from the triggerConfig aggregate subBlock. This compensates for the runtime + * population done by populateTriggerFieldsFromConfig, ensuring consistent + * comparison between client state (with populated values) and deployed state + * (with null values from DB). + */ +export function normalizeTriggerConfigValues( + subBlocks: Record +): Record { + const triggerConfigSub = subBlocks.triggerConfig as Record | undefined + const triggerConfigValue = triggerConfigSub?.value + if (!triggerConfigValue || typeof triggerConfigValue !== 'object') { + return subBlocks + } + + const result = { ...subBlocks } + for (const [fieldId, configValue] of Object.entries( + triggerConfigValue as Record + )) { + if (configValue === null || configValue === undefined) continue + const existingSub = result[fieldId] as Record | undefined + if ( + existingSub && + (existingSub.value === null || existingSub.value === undefined || existingSub.value === '') + ) { + result[fieldId] = { ...existingSub, value: configValue } + } + } + + return result +} + /** * Normalizes a subBlock value with sanitization for specific subBlock types. * Sanitizes: tools (removes isExpanded), inputFormat (removes collapsed) diff --git a/apps/sim/triggers/constants.ts b/apps/sim/triggers/constants.ts index 4cceb5439..d7fcdc997 100644 --- a/apps/sim/triggers/constants.ts +++ b/apps/sim/triggers/constants.ts @@ -23,7 +23,12 @@ export const SYSTEM_SUBBLOCK_IDS: string[] = [ * with default values from the trigger definition on load, which aren't present in * the deployed state, causing false positive change detection. */ -export const TRIGGER_RUNTIME_SUBBLOCK_IDS: string[] = ['webhookId', 'triggerPath', 'triggerConfig'] +export const TRIGGER_RUNTIME_SUBBLOCK_IDS: string[] = [ + 'webhookId', + 'triggerPath', + 'triggerConfig', + 'triggerId', +] /** * Maximum number of consecutive failures before a trigger (schedule/webhook) is auto-disabled.