Skip to content

Commit 4d24abd

Browse files
Sg312Sg312
committed
Fix lint
1 parent f7d315a commit 4d24abd

2 files changed

Lines changed: 146 additions & 89 deletions

File tree

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

Lines changed: 92 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function baseBlock(id: string, type: string, name: string, subBlocks: Record<str
1414
}
1515

1616
describe('lintEditedWorkflowState', () => {
17-
it('reports orphan blocks and empty condition/router ports', () => {
17+
it('reports orphan blocks but allows unconnected condition/router branches', () => {
1818
const workflowState = {
1919
blocks: {
2020
start: baseBlock('start', 'starter', 'Start'),
@@ -68,11 +68,7 @@ describe('lintEditedWorkflowState', () => {
6868
expect(lint.orphanBlocks).toEqual([
6969
{ blockId: 'function', blockName: 'Orphan Function', blockType: 'function' },
7070
])
71-
expect(lint.emptyOutgoingPorts.map((port) => `${port.blockName}.${port.label}`)).toEqual([
72-
'Condition.else',
73-
'Router.route-0',
74-
'Router.route-1',
75-
])
71+
expect(lint.emptyOutgoingPorts).toEqual([])
7672
expect(lint.invalidBranchPorts).toEqual([])
7773
expect(hasWorkflowLintIssues(lint)).toBe(true)
7874
})
@@ -173,6 +169,42 @@ describe('lintEditedWorkflowState', () => {
173169
expect(hasWorkflowLintIssues(lint)).toBe(false)
174170
})
175171

172+
it('objectively reports multiple sources without turning disconnected islands into an issue', () => {
173+
const workflowState = {
174+
blocks: {
175+
start: baseBlock('start', 'starter', 'Start'),
176+
fetch: baseBlock('fetch', 'function', 'FetchCurrentFiles'),
177+
normalize: baseBlock('normalize', 'function', 'NormalizeFiles'),
178+
slack: { ...baseBlock('slack', 'slack', 'SlackTrigger'), triggerMode: true },
179+
filter: baseBlock('filter', 'condition', 'MessageFilter'),
180+
},
181+
edges: [
182+
{ id: 'e1', source: 'start', sourceHandle: 'source', target: 'fetch', targetHandle: 'target' },
183+
{
184+
id: 'e2',
185+
source: 'fetch',
186+
sourceHandle: 'source',
187+
target: 'normalize',
188+
targetHandle: 'target',
189+
},
190+
{
191+
id: 'e3',
192+
source: 'slack',
193+
sourceHandle: 'source',
194+
target: 'filter',
195+
targetHandle: 'target',
196+
},
197+
],
198+
}
199+
200+
const lint = lintEditedWorkflowState(workflowState as any)
201+
202+
expect(lint.sources.map((b) => b.blockId).sort()).toEqual(['slack', 'start'])
203+
expect(lint.orphanBlocks).toEqual([])
204+
expect(lint.emptyOutgoingPorts).toEqual([])
205+
expect(hasWorkflowLintIssues(lint)).toBe(false)
206+
})
207+
176208
it('reports sources and sinks (triggers are sources, terminals are sinks, notes excluded)', () => {
177209
const workflowState = {
178210
blocks: {
@@ -213,4 +245,58 @@ describe('lintEditedWorkflowState', () => {
213245
expect(lint.sources.map((b) => b.blockId)).not.toContain('note')
214246
expect(lint.sinks.map((b) => b.blockId)).not.toContain('note')
215247
})
248+
249+
it('warns when loop/parallel start ports are empty', () => {
250+
const workflowState = {
251+
blocks: {
252+
start: baseBlock('start', 'starter', 'Start'),
253+
loop: baseBlock('loop', 'loop', 'Loop'),
254+
parallel: baseBlock('parallel', 'parallel', 'Parallel'),
255+
},
256+
edges: [
257+
{ id: 'e1', source: 'start', sourceHandle: 'source', target: 'loop', targetHandle: 'target' },
258+
{
259+
id: 'e2',
260+
source: 'loop',
261+
sourceHandle: 'loop-end-source',
262+
target: 'parallel',
263+
targetHandle: 'target',
264+
},
265+
],
266+
}
267+
268+
const lint = lintEditedWorkflowState(workflowState as any)
269+
270+
expect(lint.emptyOutgoingPorts.map((port) => `${port.blockName}.${port.handle}`)).toEqual([
271+
'Loop.loop-start-source',
272+
'Parallel.parallel-start-source',
273+
])
274+
expect(hasWorkflowLintIssues(lint)).toBe(true)
275+
})
276+
277+
it('treats loop/parallel start edges as internal so containers can still be sinks', () => {
278+
const workflowState = {
279+
blocks: {
280+
start: baseBlock('start', 'starter', 'Start'),
281+
loop: baseBlock('loop', 'loop', 'Loop'),
282+
child: baseBlock('child', 'function', 'Loop Child'),
283+
},
284+
edges: [
285+
{ id: 'e1', source: 'start', sourceHandle: 'source', target: 'loop', targetHandle: 'target' },
286+
{
287+
id: 'e2',
288+
source: 'loop',
289+
sourceHandle: 'loop-start-source',
290+
target: 'child',
291+
targetHandle: 'target',
292+
},
293+
],
294+
}
295+
296+
const lint = lintEditedWorkflowState(workflowState as any)
297+
298+
expect(lint.emptyOutgoingPorts).toEqual([])
299+
expect(lint.sinks.map((block) => block.blockId).sort()).toEqual(['child', 'loop'])
300+
expect(hasWorkflowLintIssues(lint)).toBe(false)
301+
})
216302
})

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

Lines changed: 54 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type BlockState = {
1212
id?: string
1313
type?: string
1414
name?: string
15+
triggerMode?: boolean
1516
subBlocks?: Record<string, { value?: unknown } | undefined>
1617
}
1718

@@ -90,61 +91,28 @@ function blockRef(blockId: string, block: BlockState): WorkflowLintBlockRef {
9091
}
9192
}
9293

93-
function parseArrayValue(value: unknown): any[] {
94-
if (Array.isArray(value)) return value
95-
if (typeof value === 'string') {
96-
try {
97-
const parsed = JSON.parse(value)
98-
return Array.isArray(parsed) ? parsed : []
99-
} catch {
100-
return []
101-
}
102-
}
103-
return []
104-
}
105-
106-
function conditionPortLabel(title: string, elseIfIndex: number): string {
107-
if (title === 'if') return 'if'
108-
if (title === 'else') return 'else'
109-
if (title === 'else if') return `else-if-${elseIfIndex}`
110-
return title || `branch-${elseIfIndex}`
94+
function isWorkflowEntryBlock(block: BlockState) {
95+
return Boolean(block.triggerMode) || isTriggerBlockType(block.type)
11196
}
11297

113-
function conditionPorts(block: BlockState) {
114-
const conditions = parseArrayValue(block.subBlocks?.conditions?.value)
115-
let elseIfIndex = 0
116-
117-
return conditions
118-
.map((condition, index) => {
119-
const title = String(condition?.title ?? '').toLowerCase()
120-
const label = conditionPortLabel(title, elseIfIndex)
121-
if (title === 'else if') elseIfIndex++
122-
123-
if (!condition?.id) return null
124-
return {
125-
handle: `condition-${condition.id}`,
126-
label: label || `branch-${index}`,
127-
value: block.subBlocks?.conditions?.value,
128-
}
129-
})
130-
.filter((port): port is { handle: string; label: string; value: unknown } => Boolean(port))
131-
}
132-
133-
function routerPorts(block: BlockState) {
134-
return parseArrayValue(block.subBlocks?.routes?.value)
135-
.map((route, index) => {
136-
if (!route?.id) return null
137-
return {
138-
handle: `router-${route.id}`,
139-
label: `route-${index}`,
140-
value: block.subBlocks?.routes?.value,
141-
}
142-
})
143-
.filter((port): port is { handle: string; label: string; value: unknown } => Boolean(port))
98+
function requiredSubflowStartPort(block: BlockState) {
99+
if (block.type === 'loop') {
100+
return { handle: 'loop-start-source', label: 'loop-start-source' }
101+
}
102+
if (block.type === 'parallel') {
103+
return { handle: 'parallel-start-source', label: 'parallel-start-source' }
104+
}
105+
return null
144106
}
145107

146-
function shouldLintDynamicPorts(block: BlockState) {
147-
return block.type === 'condition' || block.type === 'router_v2'
108+
function countsAsExternalOutgoing(block: BlockState, sourceHandle?: string | null) {
109+
if (block.type === 'loop') {
110+
return sourceHandle !== 'loop-start-source'
111+
}
112+
if (block.type === 'parallel') {
113+
return sourceHandle !== 'parallel-start-source'
114+
}
115+
return true
148116
}
149117

150118
export function lintEditedWorkflowState(workflowState: Pick<WorkflowState, 'blocks' | 'edges'>) {
@@ -179,43 +147,50 @@ export function lintEditedWorkflowState(workflowState: Pick<WorkflowState, 'bloc
179147
}
180148

181149
incomingEdgesByTarget.set(targetBlockId, (incomingEdgesByTarget.get(targetBlockId) || 0) + 1)
182-
outgoingEdgesBySource.add(sourceBlockId)
183-
184-
if (!shouldLintDynamicPorts(sourceBlock)) continue
150+
if (countsAsExternalOutgoing(sourceBlock, edge?.sourceHandle)) {
151+
outgoingEdgesBySource.add(sourceBlockId)
152+
}
185153

186154
const sourceHandle = edge?.sourceHandle
187155
if (!sourceHandle || sourceHandle === 'error') continue
188156

189-
const validation =
190-
sourceBlock.type === 'condition'
191-
? validateConditionHandle(
192-
sourceHandle,
193-
sourceBlockId,
194-
sourceBlock.subBlocks?.conditions?.value as string | any[]
195-
)
196-
: validateRouterHandle(
197-
sourceHandle,
198-
sourceBlockId,
199-
sourceBlock.subBlocks?.routes?.value as string | any[]
200-
)
201-
202-
if (!validation.valid) {
203-
invalidBranchPorts.push({
204-
...blockRef(sourceBlockId, sourceBlock),
205-
sourceHandle,
206-
reason: validation.error || `Invalid branch handle "${sourceHandle}"`,
207-
})
157+
if (sourceBlock.type === 'condition' || sourceBlock.type === 'router_v2') {
158+
const validation =
159+
sourceBlock.type === 'condition'
160+
? validateConditionHandle(
161+
sourceHandle,
162+
sourceBlockId,
163+
sourceBlock.subBlocks?.conditions?.value as string | any[]
164+
)
165+
: validateRouterHandle(
166+
sourceHandle,
167+
sourceBlockId,
168+
sourceBlock.subBlocks?.routes?.value as string | any[]
169+
)
170+
171+
if (!validation.valid) {
172+
invalidBranchPorts.push({
173+
...blockRef(sourceBlockId, sourceBlock),
174+
sourceHandle,
175+
reason: validation.error || `Invalid branch handle "${sourceHandle}"`,
176+
})
177+
continue
178+
}
179+
180+
const normalizedHandle = validation.normalizedHandle || sourceHandle
181+
const handles = connectedDynamicHandles.get(sourceBlockId) || new Set<string>()
182+
handles.add(normalizedHandle)
183+
connectedDynamicHandles.set(sourceBlockId, handles)
208184
continue
209185
}
210186

211-
const normalizedHandle = validation.normalizedHandle || sourceHandle
212187
const handles = connectedDynamicHandles.get(sourceBlockId) || new Set<string>()
213-
handles.add(normalizedHandle)
188+
handles.add(sourceHandle)
214189
connectedDynamicHandles.set(sourceBlockId, handles)
215190
}
216191

217192
const orphanBlocks = Object.entries(blocks)
218-
.filter(([, block]) => block.type !== 'note' && !isTriggerBlockType(block.type))
193+
.filter(([, block]) => block.type !== 'note' && !isWorkflowEntryBlock(block))
219194
.filter(([blockId]) => !incomingEdgesByTarget.has(blockId))
220195
.map(([blockId, block]) => blockRef(blockId, block))
221196

@@ -233,12 +208,8 @@ export function lintEditedWorkflowState(workflowState: Pick<WorkflowState, 'bloc
233208

234209
const emptyOutgoingPorts = Object.entries(blocks).flatMap(([blockId, block]) => {
235210
const handles = connectedDynamicHandles.get(blockId) || new Set<string>()
236-
const ports =
237-
block.type === 'condition'
238-
? conditionPorts(block)
239-
: block.type === 'router_v2'
240-
? routerPorts(block)
241-
: []
211+
const requiredPort = requiredSubflowStartPort(block)
212+
const ports = requiredPort ? [requiredPort] : []
242213

243214
return ports
244215
.filter((port) => !handles.has(port.handle))
@@ -327,7 +298,7 @@ export function formatWorkflowLintMessage(lint: WorkflowLintIssueView) {
327298

328299
if (lint.emptyOutgoingPorts.length > 0) {
329300
parts.push(
330-
`Unconnected condition/router ports: ${lint.emptyOutgoingPorts
301+
`Unconnected required subflow start ports: ${lint.emptyOutgoingPorts
331302
.map((port) => `"${port.blockName || port.blockId}".${port.label}`)
332303
.join(', ')}`
333304
)

0 commit comments

Comments
 (0)