fix(copilot): fix triggers unsave on edit (#1874)

* Fix copilot trigger unsave

* Fix flushing

* Lint

* Fix test

* Fix some tests

* Fix lint
This commit is contained in:
Siddharth Ganesan
2025-11-10 18:24:33 -08:00
committed by GitHub
parent 2f9224c166
commit 118c477d97
10 changed files with 185 additions and 27 deletions

View File

@@ -442,7 +442,7 @@ export function TriggerSave({
/>
) : (
<p className='text-muted-foreground text-xs'>
Generate a temporary URL that executes this webhook against the live (un-deployed)
Generate a temporary URL that executes this webhook against the live (undeployed)
workflow state.
</p>
)}

View File

@@ -897,7 +897,7 @@ export function WebhookModal({
) : (
<p className='text-muted-foreground text-xs'>
Generate a temporary URL that executes this webhook against the live
(un-deployed) workflow state.
(undeployed) workflow state.
</p>
)}
{testUrlExpiresAt && (

View File

@@ -10,6 +10,7 @@ import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/db-helpers'
import { validateWorkflowState } from '@/lib/workflows/validation'
import { getAllBlocks } from '@/blocks/registry'
import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils'
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/consts'
interface EditWorkflowOperation {
operation_type: 'add' | 'edit' | 'delete' | 'insert_into_subflow' | 'extract_from_subflow'
@@ -307,6 +308,38 @@ function addConnectionsAsEdges(
})
}
function applyTriggerConfigToBlockSubblocks(block: any, triggerConfig: Record<string, any>) {
if (!block?.subBlocks || !triggerConfig || typeof triggerConfig !== 'object') {
return
}
Object.entries(triggerConfig).forEach(([configKey, configValue]) => {
const existingSubblock = block.subBlocks[configKey]
if (existingSubblock) {
const existingValue = existingSubblock.value
const valuesEqual =
typeof existingValue === 'object' || typeof configValue === 'object'
? JSON.stringify(existingValue) === JSON.stringify(configValue)
: existingValue === configValue
if (valuesEqual) {
return
}
block.subBlocks[configKey] = {
...existingSubblock,
value: configValue,
}
} else {
block.subBlocks[configKey] = {
id: configKey,
type: 'short-input',
value: configValue,
}
}
})
}
/**
* Apply operations directly to the workflow JSON state
*/
@@ -405,6 +438,9 @@ function applyOperationsToWorkflowState(
if (params?.inputs) {
if (!block.subBlocks) block.subBlocks = {}
Object.entries(params.inputs).forEach(([key, value]) => {
if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) {
return
}
let sanitizedValue = value
// Special handling for inputFormat - ensure it's an array
@@ -432,10 +468,26 @@ function applyOperationsToWorkflowState(
value: sanitizedValue,
}
} else {
block.subBlocks[key].value = sanitizedValue
const existingValue = block.subBlocks[key].value
const valuesEqual =
typeof existingValue === 'object' || typeof sanitizedValue === 'object'
? JSON.stringify(existingValue) === JSON.stringify(sanitizedValue)
: existingValue === sanitizedValue
if (!valuesEqual) {
block.subBlocks[key].value = sanitizedValue
}
}
})
if (
Object.hasOwn(params.inputs, 'triggerConfig') &&
block.subBlocks.triggerConfig &&
typeof block.subBlocks.triggerConfig.value === 'object'
) {
applyTriggerConfigToBlockSubblocks(block, block.subBlocks.triggerConfig.value)
}
// Update loop/parallel configuration in block.data
if (block.type === 'loop') {
block.data = block.data || {}

View File

@@ -434,6 +434,11 @@ describe('Database Helpers', () => {
it('should successfully save workflow data to normalized tables', async () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -470,6 +475,11 @@ describe('Database Helpers', () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -526,6 +536,11 @@ describe('Database Helpers', () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -644,6 +659,11 @@ describe('Database Helpers', () => {
it('should successfully migrate workflow from JSON to normalized tables', async () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -687,6 +707,11 @@ describe('Database Helpers', () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -751,6 +776,11 @@ describe('Database Helpers', () => {
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
@@ -980,6 +1010,11 @@ describe('Database Helpers', () => {
// Mock the transaction for save operation
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue(undefined),
}),
@@ -1111,6 +1146,11 @@ describe('Database Helpers', () => {
// Mock successful save
const mockTransaction = vi.fn().mockImplementation(async (callback) => {
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue([]),
}),
}),
delete: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue(undefined),
}),

View File

@@ -1,6 +1,7 @@
import crypto from 'crypto'
import {
db,
webhook,
workflow,
workflowBlocks,
workflowDeploymentVersion,
@@ -249,6 +250,17 @@ export async function saveWorkflowToNormalizedTables(
try {
// Start a transaction
await db.transaction(async (tx) => {
// Snapshot existing webhooks before deletion to preserve them through the cycle
let existingWebhooks: any[] = []
try {
existingWebhooks = await tx.select().from(webhook).where(eq(webhook.workflowId, workflowId))
} catch (webhookError) {
// Webhook table might not be available in test environments
logger.debug('Could not load webhooks before save, skipping preservation', {
error: webhookError instanceof Error ? webhookError.message : String(webhookError),
})
}
// Clear existing data for this workflow
await Promise.all([
tx.delete(workflowBlocks).where(eq(workflowBlocks.workflowId, workflowId)),
@@ -320,6 +332,41 @@ export async function saveWorkflowToNormalizedTables(
if (subflowInserts.length > 0) {
await tx.insert(workflowSubflows).values(subflowInserts)
}
// Re-insert preserved webhooks if any exist and their blocks still exist
if (existingWebhooks.length > 0) {
try {
const webhookInserts = existingWebhooks
.filter((wh) => !!state.blocks?.[wh.blockId ?? ''])
.map((wh) => ({
id: wh.id,
workflowId: wh.workflowId,
blockId: wh.blockId,
path: wh.path,
provider: wh.provider,
providerConfig: wh.providerConfig,
isActive: wh.isActive,
createdAt: wh.createdAt,
updatedAt: new Date(),
}))
if (webhookInserts.length > 0) {
await tx.insert(webhook).values(webhookInserts)
logger.debug(`Preserved ${webhookInserts.length} webhook(s) through workflow save`, {
workflowId,
})
}
} catch (webhookInsertError) {
// Webhook preservation is optional - don't fail the entire save if it errors
logger.warn('Could not preserve webhooks during save', {
error:
webhookInsertError instanceof Error
? webhookInsertError.message
: String(webhookInsertError),
workflowId,
})
}
}
})
return { success: true }

View File

@@ -1,6 +1,7 @@
import type { Edge } from 'reactflow'
import { sanitizeWorkflowForSharing } from '@/lib/workflows/credential-extractor'
import type { BlockState, Loop, Parallel, WorkflowState } from '@/stores/workflows/workflow/types'
import { TRIGGER_PERSISTED_SUBBLOCK_IDS } from '@/triggers/consts'
/**
* Sanitized workflow state for copilot (removes all UI-specific data)
@@ -68,6 +69,10 @@ export interface ExportWorkflowState {
* Check if a subblock contains sensitive/secret data
*/
function isSensitiveSubBlock(key: string, subBlock: BlockState['subBlocks'][string]): boolean {
if (TRIGGER_PERSISTED_SUBBLOCK_IDS.includes(key)) {
return false
}
// Check if it's an OAuth input type
if (subBlock.type === 'oauth-input') {
return true

View File

@@ -1,4 +1,5 @@
import type { CopilotWorkflowState } from '@/lib/workflows/json-sanitizer'
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/consts'
export interface EditOperation {
operation_type: 'add' | 'edit' | 'delete' | 'insert_into_subflow' | 'extract_from_subflow'
@@ -502,6 +503,9 @@ function computeInputDelta(
const delta: Record<string, any> = {}
for (const key in endInputs) {
if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) {
continue
}
if (
!(key in startInputs) ||
JSON.stringify(startInputs[key]) !== JSON.stringify(endInputs[key])

View File

@@ -10,8 +10,6 @@
import type { BlockState, SubBlockState } from '@/stores/workflows/workflow/types'
const WEBHOOK_SUBBLOCK_FIELDS = ['webhookId', 'triggerPath']
/**
* Server-safe version of mergeSubblockState for API routes
*
@@ -44,11 +42,10 @@ export function mergeSubblockState(
const blockValues = subBlockValues[id] || {}
// Create a deep copy of the block's subBlocks to maintain structure
// Exclude webhook-specific fields that should not be persisted
const mergedSubBlocks = Object.entries(blockSubBlocks).reduce(
(subAcc, [subBlockId, subBlock]) => {
// Skip if subBlock is undefined or is a webhook-specific field
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
// Skip if subBlock is undefined
if (!subBlock) {
return subAcc
}
@@ -75,12 +72,7 @@ export function mergeSubblockState(
// Add any values that exist in the provided values but aren't in the block structure
// This handles cases where block config has been updated but values still exist
Object.entries(blockValues).forEach(([subBlockId, value]) => {
if (
!mergedSubBlocks[subBlockId] &&
value !== null &&
value !== undefined &&
!WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)
) {
if (!mergedSubBlocks[subBlockId] && value !== null && value !== undefined) {
// Create a minimal subblock structure
mergedSubBlocks[subBlockId] = {
id: subBlockId,

View File

@@ -1,8 +1,6 @@
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import type { BlockState, SubBlockState } from '@/stores/workflows/workflow/types'
const WEBHOOK_SUBBLOCK_FIELDS = ['webhookId', 'triggerPath']
/**
* Normalizes a block name for comparison by converting to lowercase and removing spaces
* @param name - The block name to normalize
@@ -77,11 +75,10 @@ export function mergeSubblockState(
const blockValues = workflowSubblockValues[id] || {}
// Create a deep copy of the block's subBlocks to maintain structure
// Exclude webhook-specific fields that should not be persisted
const mergedSubBlocks = Object.entries(blockSubBlocks).reduce(
(subAcc, [subBlockId, subBlock]) => {
// Skip if subBlock is undefined or is a webhook-specific field
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
// Skip if subBlock is undefined
if (!subBlock) {
return subAcc
}
@@ -119,12 +116,7 @@ export function mergeSubblockState(
// Add any values that exist in the store but aren't in the block structure
// This handles cases where block config has been updated but values still exist
Object.entries(blockValues).forEach(([subBlockId, value]) => {
if (
!mergedSubBlocks[subBlockId] &&
value !== null &&
value !== undefined &&
!WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)
) {
if (!mergedSubBlocks[subBlockId] && value !== null && value !== undefined) {
// Create a minimal subblock structure
mergedSubBlocks[subBlockId] = {
id: subBlockId,
@@ -174,8 +166,8 @@ export async function mergeSubblockStateAsync(
// Process all subblocks in parallel
const subBlockEntries = await Promise.all(
Object.entries(block.subBlocks).map(async ([subBlockId, subBlock]) => {
// Skip if subBlock is undefined or webhook-specific
if (!subBlock || WEBHOOK_SUBBLOCK_FIELDS.includes(subBlockId)) {
// Skip if subBlock is undefined
if (!subBlock) {
return null
}

View File

@@ -14,3 +14,29 @@ export const SYSTEM_SUBBLOCK_IDS: string[] = [
'triggerId', // Stored trigger ID
'selectedTriggerId', // Selected trigger from dropdown (multi-trigger blocks)
]
/**
* Trigger-related subblock IDs whose values should be persisted and
* propagated when workflows are edited programmatically.
*/
export const TRIGGER_PERSISTED_SUBBLOCK_IDS: string[] = [
'triggerConfig',
'triggerCredentials',
'triggerId',
'selectedTriggerId',
'webhookId',
'triggerPath',
'testUrl',
'testUrlExpiresAt',
]
/**
* Trigger-related subblock IDs that represent runtime metadata. They should remain
* in the workflow state but must not be modified or cleared by diff operations.
*/
export const TRIGGER_RUNTIME_SUBBLOCK_IDS: string[] = [
'webhookId',
'triggerPath',
'testUrl',
'testUrlExpiresAt',
]