mirror of
https://github.com/simstudioai/sim.git
synced 2026-01-09 15:07:55 -05:00
improvement(copilot): add edge handle validation to copilot edit workflow (#2448)
* Add edge handle validation * Clean * Fix lint * Fix empty target handle
This commit is contained in:
Siddharth Ganesan
committed by
GitHub
GitHub
parent
811c736705
commit
83d813a7cc
@@ -50,6 +50,8 @@ type SkippedItemType =
|
||||
| 'invalid_block_type'
|
||||
| 'invalid_edge_target'
|
||||
| 'invalid_edge_source'
|
||||
| 'invalid_source_handle'
|
||||
| 'invalid_target_handle'
|
||||
| 'invalid_subblock_field'
|
||||
| 'missing_required_params'
|
||||
| 'invalid_subflow_parent'
|
||||
@@ -734,8 +736,279 @@ function normalizeResponseFormat(value: any): string {
|
||||
}
|
||||
}
|
||||
|
||||
interface EdgeHandleValidationResult {
|
||||
valid: boolean
|
||||
error?: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper to add connections as edges for a block
|
||||
* Validates source handle is valid for the block type
|
||||
*/
|
||||
function validateSourceHandleForBlock(
|
||||
sourceHandle: string,
|
||||
sourceBlockType: string,
|
||||
sourceBlock: any
|
||||
): EdgeHandleValidationResult {
|
||||
if (sourceHandle === 'error') {
|
||||
return { valid: true }
|
||||
}
|
||||
|
||||
switch (sourceBlockType) {
|
||||
case 'loop':
|
||||
if (sourceHandle === 'loop-start-source' || sourceHandle === 'loop-end-source') {
|
||||
return { valid: true }
|
||||
}
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid source handle "${sourceHandle}" for loop block. Valid handles: loop-start-source, loop-end-source, error`,
|
||||
}
|
||||
|
||||
case 'parallel':
|
||||
if (sourceHandle === 'parallel-start-source' || sourceHandle === 'parallel-end-source') {
|
||||
return { valid: true }
|
||||
}
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid source handle "${sourceHandle}" for parallel block. Valid handles: parallel-start-source, parallel-end-source, error`,
|
||||
}
|
||||
|
||||
case 'condition': {
|
||||
if (!sourceHandle.startsWith('condition-')) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid source handle "${sourceHandle}" for condition block. Must start with "condition-"`,
|
||||
}
|
||||
}
|
||||
|
||||
const conditionsValue = sourceBlock?.subBlocks?.conditions?.value
|
||||
if (!conditionsValue) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid condition handle "${sourceHandle}" - no conditions defined`,
|
||||
}
|
||||
}
|
||||
|
||||
return validateConditionHandle(sourceHandle, sourceBlock.id, conditionsValue)
|
||||
}
|
||||
|
||||
case 'router':
|
||||
if (sourceHandle === 'source' || sourceHandle.startsWith('router-')) {
|
||||
return { valid: true }
|
||||
}
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid source handle "${sourceHandle}" for router block. Valid handles: source, router-{targetId}, error`,
|
||||
}
|
||||
|
||||
default:
|
||||
if (sourceHandle === 'source') {
|
||||
return { valid: true }
|
||||
}
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid source handle "${sourceHandle}" for ${sourceBlockType} block. Valid handles: source, error`,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates condition handle references a valid condition in the block.
|
||||
* Accepts both internal IDs (condition-blockId-if) and semantic keys (condition-blockId-else-if)
|
||||
*/
|
||||
function validateConditionHandle(
|
||||
sourceHandle: string,
|
||||
blockId: string,
|
||||
conditionsValue: string | any[]
|
||||
): EdgeHandleValidationResult {
|
||||
let conditions: any[]
|
||||
if (typeof conditionsValue === 'string') {
|
||||
try {
|
||||
conditions = JSON.parse(conditionsValue)
|
||||
} catch {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Cannot validate condition handle "${sourceHandle}" - conditions is not valid JSON`,
|
||||
}
|
||||
}
|
||||
} else if (Array.isArray(conditionsValue)) {
|
||||
conditions = conditionsValue
|
||||
} else {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Cannot validate condition handle "${sourceHandle}" - conditions is not an array`,
|
||||
}
|
||||
}
|
||||
|
||||
if (!Array.isArray(conditions) || conditions.length === 0) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid condition handle "${sourceHandle}" - no conditions defined`,
|
||||
}
|
||||
}
|
||||
|
||||
const validHandles = new Set<string>()
|
||||
const semanticPrefix = `condition-${blockId}-`
|
||||
let elseIfCount = 0
|
||||
|
||||
for (const condition of conditions) {
|
||||
if (condition.id) {
|
||||
validHandles.add(`condition-${condition.id}`)
|
||||
}
|
||||
|
||||
const title = condition.title?.toLowerCase()
|
||||
if (title === 'if') {
|
||||
validHandles.add(`${semanticPrefix}if`)
|
||||
} else if (title === 'else if') {
|
||||
elseIfCount++
|
||||
validHandles.add(
|
||||
elseIfCount === 1 ? `${semanticPrefix}else-if` : `${semanticPrefix}else-if-${elseIfCount}`
|
||||
)
|
||||
} else if (title === 'else') {
|
||||
validHandles.add(`${semanticPrefix}else`)
|
||||
}
|
||||
}
|
||||
|
||||
if (validHandles.has(sourceHandle)) {
|
||||
return { valid: true }
|
||||
}
|
||||
|
||||
const validOptions = Array.from(validHandles).slice(0, 5)
|
||||
const moreCount = validHandles.size - validOptions.length
|
||||
let validOptionsStr = validOptions.join(', ')
|
||||
if (moreCount > 0) {
|
||||
validOptionsStr += `, ... and ${moreCount} more`
|
||||
}
|
||||
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid condition handle "${sourceHandle}". Valid handles: ${validOptionsStr}`,
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates target handle is valid (must be 'target')
|
||||
*/
|
||||
function validateTargetHandle(targetHandle: string): EdgeHandleValidationResult {
|
||||
if (targetHandle === 'target') {
|
||||
return { valid: true }
|
||||
}
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid target handle "${targetHandle}". Expected "target"`,
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a validated edge between two blocks.
|
||||
* Returns true if edge was created, false if skipped due to validation errors.
|
||||
*/
|
||||
function createValidatedEdge(
|
||||
modifiedState: any,
|
||||
sourceBlockId: string,
|
||||
targetBlockId: string,
|
||||
sourceHandle: string,
|
||||
targetHandle: string,
|
||||
operationType: string,
|
||||
logger: ReturnType<typeof createLogger>,
|
||||
skippedItems?: SkippedItem[]
|
||||
): boolean {
|
||||
if (!modifiedState.blocks[targetBlockId]) {
|
||||
logger.warn(`Target block "${targetBlockId}" not found. Edge skipped.`, {
|
||||
sourceBlockId,
|
||||
targetBlockId,
|
||||
sourceHandle,
|
||||
})
|
||||
skippedItems?.push({
|
||||
type: 'invalid_edge_target',
|
||||
operationType,
|
||||
blockId: sourceBlockId,
|
||||
reason: `Edge from "${sourceBlockId}" to "${targetBlockId}" skipped - target block does not exist`,
|
||||
details: { sourceHandle, targetHandle, targetId: targetBlockId },
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
const sourceBlock = modifiedState.blocks[sourceBlockId]
|
||||
if (!sourceBlock) {
|
||||
logger.warn(`Source block "${sourceBlockId}" not found. Edge skipped.`, {
|
||||
sourceBlockId,
|
||||
targetBlockId,
|
||||
})
|
||||
skippedItems?.push({
|
||||
type: 'invalid_edge_source',
|
||||
operationType,
|
||||
blockId: sourceBlockId,
|
||||
reason: `Edge from "${sourceBlockId}" to "${targetBlockId}" skipped - source block does not exist`,
|
||||
details: { sourceHandle, targetHandle, targetId: targetBlockId },
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
const sourceBlockType = sourceBlock.type
|
||||
if (!sourceBlockType) {
|
||||
logger.warn(`Source block "${sourceBlockId}" has no type. Edge skipped.`, {
|
||||
sourceBlockId,
|
||||
targetBlockId,
|
||||
})
|
||||
skippedItems?.push({
|
||||
type: 'invalid_edge_source',
|
||||
operationType,
|
||||
blockId: sourceBlockId,
|
||||
reason: `Edge from "${sourceBlockId}" to "${targetBlockId}" skipped - source block has no type`,
|
||||
details: { sourceHandle, targetHandle, targetId: targetBlockId },
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
const sourceValidation = validateSourceHandleForBlock(sourceHandle, sourceBlockType, sourceBlock)
|
||||
if (!sourceValidation.valid) {
|
||||
logger.warn(`Invalid source handle. Edge skipped.`, {
|
||||
sourceBlockId,
|
||||
targetBlockId,
|
||||
sourceHandle,
|
||||
error: sourceValidation.error,
|
||||
})
|
||||
skippedItems?.push({
|
||||
type: 'invalid_source_handle',
|
||||
operationType,
|
||||
blockId: sourceBlockId,
|
||||
reason: sourceValidation.error || `Invalid source handle "${sourceHandle}"`,
|
||||
details: { sourceHandle, targetHandle, targetId: targetBlockId },
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
const targetValidation = validateTargetHandle(targetHandle)
|
||||
if (!targetValidation.valid) {
|
||||
logger.warn(`Invalid target handle. Edge skipped.`, {
|
||||
sourceBlockId,
|
||||
targetBlockId,
|
||||
targetHandle,
|
||||
error: targetValidation.error,
|
||||
})
|
||||
skippedItems?.push({
|
||||
type: 'invalid_target_handle',
|
||||
operationType,
|
||||
blockId: sourceBlockId,
|
||||
reason: targetValidation.error || `Invalid target handle "${targetHandle}"`,
|
||||
details: { sourceHandle, targetHandle, targetId: targetBlockId },
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
modifiedState.edges.push({
|
||||
id: crypto.randomUUID(),
|
||||
source: sourceBlockId,
|
||||
sourceHandle,
|
||||
target: targetBlockId,
|
||||
targetHandle,
|
||||
type: 'default',
|
||||
})
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds connections as edges for a block
|
||||
*/
|
||||
function addConnectionsAsEdges(
|
||||
modifiedState: any,
|
||||
@@ -747,34 +1020,16 @@ function addConnectionsAsEdges(
|
||||
Object.entries(connections).forEach(([sourceHandle, targets]) => {
|
||||
const targetArray = Array.isArray(targets) ? targets : [targets]
|
||||
targetArray.forEach((targetId: string) => {
|
||||
// Validate target block exists - skip edge if target doesn't exist
|
||||
if (!modifiedState.blocks[targetId]) {
|
||||
logger.warn(
|
||||
`Target block "${targetId}" not found when creating connection from "${blockId}". ` +
|
||||
`Edge skipped.`,
|
||||
{
|
||||
sourceBlockId: blockId,
|
||||
targetBlockId: targetId,
|
||||
existingBlocks: Object.keys(modifiedState.blocks),
|
||||
}
|
||||
)
|
||||
skippedItems?.push({
|
||||
type: 'invalid_edge_target',
|
||||
operationType: 'add_edge',
|
||||
blockId: blockId,
|
||||
reason: `Edge from "${blockId}" to "${targetId}" skipped - target block does not exist`,
|
||||
details: { sourceHandle, targetId },
|
||||
})
|
||||
return
|
||||
}
|
||||
modifiedState.edges.push({
|
||||
id: crypto.randomUUID(),
|
||||
source: blockId,
|
||||
createValidatedEdge(
|
||||
modifiedState,
|
||||
blockId,
|
||||
targetId,
|
||||
sourceHandle,
|
||||
target: targetId,
|
||||
targetHandle: 'target',
|
||||
type: 'default',
|
||||
})
|
||||
'target',
|
||||
'add_edge',
|
||||
logger,
|
||||
skippedItems
|
||||
)
|
||||
})
|
||||
})
|
||||
}
|
||||
@@ -1257,67 +1512,44 @@ function applyOperationsToWorkflowState(
|
||||
|
||||
// Handle connections update (convert to edges)
|
||||
if (params?.connections) {
|
||||
// Remove existing edges from this block
|
||||
modifiedState.edges = modifiedState.edges.filter((edge: any) => edge.source !== block_id)
|
||||
|
||||
// Add new edges based on connections
|
||||
Object.entries(params.connections).forEach(([connectionType, targets]) => {
|
||||
if (targets === null) return
|
||||
|
||||
// Map semantic connection names to actual React Flow handle IDs
|
||||
// 'success' in YAML/connections maps to 'source' handle in React Flow
|
||||
const mapConnectionTypeToHandle = (type: string): string => {
|
||||
if (type === 'success') return 'source'
|
||||
if (type === 'error') return 'error'
|
||||
// Conditions and other types pass through as-is
|
||||
return type
|
||||
}
|
||||
|
||||
const actualSourceHandle = mapConnectionTypeToHandle(connectionType)
|
||||
const sourceHandle = mapConnectionTypeToHandle(connectionType)
|
||||
|
||||
const addEdge = (targetBlock: string, targetHandle?: string) => {
|
||||
// Validate target block exists - skip edge if target doesn't exist
|
||||
if (!modifiedState.blocks[targetBlock]) {
|
||||
logger.warn(
|
||||
`Target block "${targetBlock}" not found when creating connection from "${block_id}". ` +
|
||||
`Edge skipped.`,
|
||||
{
|
||||
sourceBlockId: block_id,
|
||||
targetBlockId: targetBlock,
|
||||
existingBlocks: Object.keys(modifiedState.blocks),
|
||||
}
|
||||
)
|
||||
logSkippedItem(skippedItems, {
|
||||
type: 'invalid_edge_target',
|
||||
operationType: 'edit',
|
||||
blockId: block_id,
|
||||
reason: `Edge from "${block_id}" to "${targetBlock}" skipped - target block does not exist`,
|
||||
details: { sourceHandle: actualSourceHandle, targetId: targetBlock },
|
||||
})
|
||||
return
|
||||
}
|
||||
modifiedState.edges.push({
|
||||
id: crypto.randomUUID(),
|
||||
source: block_id,
|
||||
sourceHandle: actualSourceHandle,
|
||||
target: targetBlock,
|
||||
targetHandle: targetHandle || 'target',
|
||||
type: 'default',
|
||||
})
|
||||
const addEdgeForTarget = (targetBlock: string, targetHandle?: string) => {
|
||||
createValidatedEdge(
|
||||
modifiedState,
|
||||
block_id,
|
||||
targetBlock,
|
||||
sourceHandle,
|
||||
targetHandle || 'target',
|
||||
'edit',
|
||||
logger,
|
||||
skippedItems
|
||||
)
|
||||
}
|
||||
|
||||
if (typeof targets === 'string') {
|
||||
addEdge(targets)
|
||||
addEdgeForTarget(targets)
|
||||
} else if (Array.isArray(targets)) {
|
||||
targets.forEach((target: any) => {
|
||||
if (typeof target === 'string') {
|
||||
addEdge(target)
|
||||
addEdgeForTarget(target)
|
||||
} else if (target?.block) {
|
||||
addEdge(target.block, target.handle)
|
||||
addEdgeForTarget(target.block, target.handle)
|
||||
}
|
||||
})
|
||||
} else if (typeof targets === 'object' && (targets as any)?.block) {
|
||||
addEdge((targets as any).block, (targets as any).handle)
|
||||
addEdgeForTarget((targets as any).block, (targets as any).handle)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user