Skip to content

Commit 118c477

Browse files
Sg312Sg312
authored
fix(copilot): fix triggers unsave on edit (#1874)
* Fix copilot trigger unsave * Fix flushing * Lint * Fix test * Fix some tests * Fix lint
1 parent 2f9224c commit 118c477

File tree

10 files changed

+185
-27
lines changed

10 files changed

+185
-27
lines changed

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel-new/components/editor/components/sub-block/components/trigger-save/trigger-save.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ export function TriggerSave({
442442
/>
443443
) : (
444444
<p className='text-muted-foreground text-xs'>
445-
Generate a temporary URL that executes this webhook against the live (un-deployed)
445+
Generate a temporary URL that executes this webhook against the live (undeployed)
446446
workflow state.
447447
</p>
448448
)}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel-new/components/editor/components/sub-block/components/webhook/components/webhook-modal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,7 @@ export function WebhookModal({
897897
) : (
898898
<p className='text-muted-foreground text-xs'>
899899
Generate a temporary URL that executes this webhook against the live
900-
(un-deployed) workflow state.
900+
(undeployed) workflow state.
901901
</p>
902902
)}
903903
{testUrlExpiresAt && (

‎apps/sim/lib/copilot/tools/server/workflow/edit-workflow.ts‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/db-helpers'
1010
import { validateWorkflowState } from '@/lib/workflows/validation'
1111
import { getAllBlocks } from '@/blocks/registry'
1212
import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils'
13+
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/consts'
1314

1415
interface EditWorkflowOperation {
1516
operation_type: 'add' | 'edit' | 'delete' | 'insert_into_subflow' | 'extract_from_subflow'
@@ -307,6 +308,38 @@ function addConnectionsAsEdges(
307308
})
308309
}
309310

311+
function applyTriggerConfigToBlockSubblocks(block: any, triggerConfig: Record<string, any>) {
312+
if (!block?.subBlocks || !triggerConfig || typeof triggerConfig !== 'object') {
313+
return
314+
}
315+
316+
Object.entries(triggerConfig).forEach(([configKey, configValue]) => {
317+
const existingSubblock = block.subBlocks[configKey]
318+
if (existingSubblock) {
319+
const existingValue = existingSubblock.value
320+
const valuesEqual =
321+
typeof existingValue === 'object' || typeof configValue === 'object'
322+
? JSON.stringify(existingValue) === JSON.stringify(configValue)
323+
: existingValue === configValue
324+
325+
if (valuesEqual) {
326+
return
327+
}
328+
329+
block.subBlocks[configKey] = {
330+
...existingSubblock,
331+
value: configValue,
332+
}
333+
} else {
334+
block.subBlocks[configKey] = {
335+
id: configKey,
336+
type: 'short-input',
337+
value: configValue,
338+
}
339+
}
340+
})
341+
}
342+
310343
/**
311344
* Apply operations directly to the workflow JSON state
312345
*/
@@ -405,6 +438,9 @@ function applyOperationsToWorkflowState(
405438
if (params?.inputs) {
406439
if (!block.subBlocks) block.subBlocks = {}
407440
Object.entries(params.inputs).forEach(([key, value]) => {
441+
if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) {
442+
return
443+
}
408444
let sanitizedValue = value
409445

410446
// Special handling for inputFormat - ensure it's an array
@@ -432,10 +468,26 @@ function applyOperationsToWorkflowState(
432468
value: sanitizedValue,
433469
}
434470
} else {
435-
block.subBlocks[key].value = sanitizedValue
471+
const existingValue = block.subBlocks[key].value
472+
const valuesEqual =
473+
typeof existingValue === 'object' || typeof sanitizedValue === 'object'
474+
? JSON.stringify(existingValue) === JSON.stringify(sanitizedValue)
475+
: existingValue === sanitizedValue
476+
477+
if (!valuesEqual) {
478+
block.subBlocks[key].value = sanitizedValue
479+
}
436480
}
437481
})
438482

483+
if (
484+
Object.hasOwn(params.inputs, 'triggerConfig') &&
485+
block.subBlocks.triggerConfig &&
486+
typeof block.subBlocks.triggerConfig.value === 'object'
487+
) {
488+
applyTriggerConfigToBlockSubblocks(block, block.subBlocks.triggerConfig.value)
489+
}
490+
439491
// Update loop/parallel configuration in block.data
440492
if (block.type === 'loop') {
441493
block.data = block.data || {}

‎apps/sim/lib/workflows/db-helpers.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,11 @@ describe('Database Helpers', () => {
434434
it('should successfully save workflow data to normalized tables', async () => {
435435
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
436436
const tx = {
437+
select: vi.fn().mockReturnValue({
438+
from: vi.fn().mockReturnValue({
439+
where: vi.fn().mockResolvedValue([]),
440+
}),
441+
}),
437442
delete: vi.fn().mockReturnValue({
438443
where: vi.fn().mockResolvedValue([]),
439444
}),
@@ -470,6 +475,11 @@ describe('Database Helpers', () => {
470475

471476
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
472477
const tx = {
478+
select: vi.fn().mockReturnValue({
479+
from: vi.fn().mockReturnValue({
480+
where: vi.fn().mockResolvedValue([]),
481+
}),
482+
}),
473483
delete: vi.fn().mockReturnValue({
474484
where: vi.fn().mockResolvedValue([]),
475485
}),
@@ -526,6 +536,11 @@ describe('Database Helpers', () => {
526536

527537
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
528538
const tx = {
539+
select: vi.fn().mockReturnValue({
540+
from: vi.fn().mockReturnValue({
541+
where: vi.fn().mockResolvedValue([]),
542+
}),
543+
}),
529544
delete: vi.fn().mockReturnValue({
530545
where: vi.fn().mockResolvedValue([]),
531546
}),
@@ -644,6 +659,11 @@ describe('Database Helpers', () => {
644659
it('should successfully migrate workflow from JSON to normalized tables', async () => {
645660
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
646661
const tx = {
662+
select: vi.fn().mockReturnValue({
663+
from: vi.fn().mockReturnValue({
664+
where: vi.fn().mockResolvedValue([]),
665+
}),
666+
}),
647667
delete: vi.fn().mockReturnValue({
648668
where: vi.fn().mockResolvedValue([]),
649669
}),
@@ -687,6 +707,11 @@ describe('Database Helpers', () => {
687707

688708
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
689709
const tx = {
710+
select: vi.fn().mockReturnValue({
711+
from: vi.fn().mockReturnValue({
712+
where: vi.fn().mockResolvedValue([]),
713+
}),
714+
}),
690715
delete: vi.fn().mockReturnValue({
691716
where: vi.fn().mockResolvedValue([]),
692717
}),
@@ -751,6 +776,11 @@ describe('Database Helpers', () => {
751776

752777
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
753778
const tx = {
779+
select: vi.fn().mockReturnValue({
780+
from: vi.fn().mockReturnValue({
781+
where: vi.fn().mockResolvedValue([]),
782+
}),
783+
}),
754784
delete: vi.fn().mockReturnValue({
755785
where: vi.fn().mockResolvedValue([]),
756786
}),
@@ -980,6 +1010,11 @@ describe('Database Helpers', () => {
9801010
// Mock the transaction for save operation
9811011
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
9821012
const mockTx = {
1013+
select: vi.fn().mockReturnValue({
1014+
from: vi.fn().mockReturnValue({
1015+
where: vi.fn().mockResolvedValue([]),
1016+
}),
1017+
}),
9831018
delete: vi.fn().mockReturnValue({
9841019
where: vi.fn().mockResolvedValue(undefined),
9851020
}),
@@ -1111,6 +1146,11 @@ describe('Database Helpers', () => {
11111146
// Mock successful save
11121147
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
11131148
const mockTx = {
1149+
select: vi.fn().mockReturnValue({
1150+
from: vi.fn().mockReturnValue({
1151+
where: vi.fn().mockResolvedValue([]),
1152+
}),
1153+
}),
11141154
delete: vi.fn().mockReturnValue({
11151155
where: vi.fn().mockResolvedValue(undefined),
11161156
}),

‎apps/sim/lib/workflows/db-helpers.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import crypto from 'crypto'
22
import {
33
db,
4+
webhook,
45
workflow,
56
workflowBlocks,
67
workflowDeploymentVersion,
@@ -249,6 +250,17 @@ export async function saveWorkflowToNormalizedTables(
249250
try {
250251
// Start a transaction
251252
await db.transaction(async (tx) => {
253+
// Snapshot existing webhooks before deletion to preserve them through the cycle
254+
let existingWebhooks: any[] = []
255+
try {
256+
existingWebhooks = await tx.select().from(webhook).where(eq(webhook.workflowId, workflowId))
257+
} catch (webhookError) {
258+
// Webhook table might not be available in test environments
259+
logger.debug('Could not load webhooks before save, skipping preservation', {
260+
error: webhookError instanceof Error ? webhookError.message : String(webhookError),
261+
})
262+
}
263+
252264
// Clear existing data for this workflow
253265
await Promise.all([
254266
tx.delete(workflowBlocks).where(eq(workflowBlocks.workflowId, workflowId)),
@@ -320,6 +332,41 @@ export async function saveWorkflowToNormalizedTables(
320332
if (subflowInserts.length > 0) {
321333
await tx.insert(workflowSubflows).values(subflowInserts)
322334
}
335+
336+
// Re-insert preserved webhooks if any exist and their blocks still exist
337+
if (existingWebhooks.length > 0) {
338+
try {
339+
const webhookInserts = existingWebhooks
340+
.filter((wh) => !!state.blocks?.[wh.blockId ?? ''])
341+
.map((wh) => ({
342+
id: wh.id,
343+
workflowId: wh.workflowId,
344+
blockId: wh.blockId,
345+
path: wh.path,
346+
provider: wh.provider,
347+
providerConfig: wh.providerConfig,
348+
isActive: wh.isActive,
349+
createdAt: wh.createdAt,
350+
updatedAt: new Date(),
351+
}))
352+
353+
if (webhookInserts.length > 0) {
354+
await tx.insert(webhook).values(webhookInserts)
355+
logger.debug(`Preserved ${webhookInserts.length} webhook(s) through workflow save`, {
356+
workflowId,
357+
})
358+
}
359+
} catch (webhookInsertError) {
360+
// Webhook preservation is optional - don't fail the entire save if it errors
361+
logger.warn('Could not preserve webhooks during save', {
362+
error:
363+
webhookInsertError instanceof Error
364+
? webhookInsertError.message
365+
: String(webhookInsertError),
366+
workflowId,
367+
})
368+
}
369+
}
323370
})
324371

325372
return { success: true }

‎apps/sim/lib/workflows/json-sanitizer.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Edge } from 'reactflow'
22
import { sanitizeWorkflowForSharing } from '@/lib/workflows/credential-extractor'
33
import type { BlockState, Loop, Parallel, WorkflowState } from '@/stores/workflows/workflow/types'
4+
import { TRIGGER_PERSISTED_SUBBLOCK_IDS } from '@/triggers/consts'
45

56
/**
67
* Sanitized workflow state for copilot (removes all UI-specific data)
@@ -68,6 +69,10 @@ export interface ExportWorkflowState {
6869
* Check if a subblock contains sensitive/secret data
6970
*/
7071
function isSensitiveSubBlock(key: string, subBlock: BlockState['subBlocks'][string]): boolean {
72+
if (TRIGGER_PERSISTED_SUBBLOCK_IDS.includes(key)) {
73+
return false
74+
}
75+
7176
// Check if it's an OAuth input type
7277
if (subBlock.type === 'oauth-input') {
7378
return true

‎apps/sim/lib/workflows/training/compute-edit-sequence.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { CopilotWorkflowState } from '@/lib/workflows/json-sanitizer'
2+
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/consts'
23

34
export interface EditOperation {
45
operation_type: 'add' | 'edit' | 'delete' | 'insert_into_subflow' | 'extract_from_subflow'
@@ -502,6 +503,9 @@ function computeInputDelta(
502503
const delta: Record<string, any> = {}
503504

504505
for (const key in endInputs) {
506+
if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) {
507+
continue
508+
}
505509
if (
506510
!(key in startInputs) ||
507511
JSON.stringify(startInputs[key]) !== JSON.stringify(endInputs[key])

‎apps/sim/stores/workflows/server-utils.ts‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010

1111
import type { BlockState, SubBlockState } from '@/stores/workflows/workflow/types'
1212

13-
const WEBHOOK_SUBBLOCK_FIELDS = ['webhookId', 'triggerPath']
14-
1513
/**
1614
* Server-safe version of mergeSubblockState for API routes
1715
*
@@ -44,11 +42,10 @@ export function mergeSubblockState(
4442
const blockValues = subBlockValues[id] || {}
4543

4644
// Create a deep copy of the block's subBlocks to maintain structure
47-
// Exclude webhook-specific fields that should not be persisted
4845
const mergedSubBlocks = Object.entries(blockSubBlocks).reduce(
4946
(subAcc, [subBlockId, subBlock]) => {
50-
// Skip if subBlock is undefined or is a webhook-specific field
51-
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
47+
// Skip if subBlock is undefined
48+
if (!subBlock) {
5249
return subAcc
5350
}
5451

@@ -75,12 +72,7 @@ export function mergeSubblockState(
7572
// Add any values that exist in the provided values but aren't in the block structure
7673
// This handles cases where block config has been updated but values still exist
7774
Object.entries(blockValues).forEach(([subBlockId, value]) => {
78-
if (
79-
!mergedSubBlocks[subBlockId] &&
80-
value !== null &&
81-
value !== undefined &&
82-
!WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)
83-
) {
75+
if (!mergedSubBlocks[subBlockId] && value !== null && value !== undefined) {
8476
// Create a minimal subblock structure
8577
mergedSubBlocks[subBlockId] = {
8678
id: subBlockId,

‎apps/sim/stores/workflows/utils.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
22
import type { BlockState, SubBlockState } from '@/stores/workflows/workflow/types'
33

4-
const WEBHOOK_SUBBLOCK_FIELDS = ['webhookId', 'triggerPath']
5-
64
/**
75
* Normalizes a block name for comparison by converting to lowercase and removing spaces
86
* @param name - The block name to normalize
@@ -77,11 +75,10 @@ export function mergeSubblockState(
7775
const blockValues = workflowSubblockValues[id] || {}
7876

7977
// Create a deep copy of the block's subBlocks to maintain structure
80-
// Exclude webhook-specific fields that should not be persisted
8178
const mergedSubBlocks = Object.entries(blockSubBlocks).reduce(
8279
(subAcc, [subBlockId, subBlock]) => {
83-
// Skip if subBlock is undefined or is a webhook-specific field
84-
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
80+
// Skip if subBlock is undefined
81+
if (!subBlock) {
8582
return subAcc
8683
}
8784

@@ -119,12 +116,7 @@ export function mergeSubblockState(
119116
// Add any values that exist in the store but aren't in the block structure
120117
// This handles cases where block config has been updated but values still exist
121118
Object.entries(blockValues).forEach(([subBlockId, value]) => {
122-
if (
123-
!mergedSubBlocks[subBlockId] &&
124-
value !== null &&
125-
value !== undefined &&
126-
!WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)
127-
) {
119+
if (!mergedSubBlocks[subBlockId] && value !== null && value !== undefined) {
128120
// Create a minimal subblock structure
129121
mergedSubBlocks[subBlockId] = {
130122
id: subBlockId,
@@ -174,8 +166,8 @@ export async function mergeSubblockStateAsync(
174166
// Process all subblocks in parallel
175167
const subBlockEntries = await Promise.all(
176168
Object.entries(block.subBlocks).map(async ([subBlockId, subBlock]) => {
177-
// Skip if subBlock is undefined or webhook-specific
178-
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
169+
// Skip if subBlock is undefined
170+
if (!subBlock) {
179171
return null
180172
}
181173

0 commit comments

Comments
 (0)