Skip to content

Commit 16d4176

Browse files
fixed errored workfows not showing as stopped
1 parent fa28ff3 commit 16d4176

5 files changed

Lines changed: 251 additions & 39 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-sidebar/column-sidebar.tsx

Lines changed: 176 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,48 @@ export function ColumnSidebar({
288288
return allColumns.slice(0, idx)
289289
}, [configState, allColumns])
290290

291+
/**
292+
* Split `otherColumns` into the two dep buckets:
293+
* - `scalarDepColumns` — plain columns; tickable into `dependencies.columns`.
294+
* - `groupDepOptions` — producing workflow groups whose outputs land left of the
295+
* current column; tickable into `dependencies.workflowGroups`. A group only
296+
* shows up here when at least one of its output columns is left-of-current.
297+
* The current group itself is excluded so we never depend on ourselves.
298+
*/
299+
const scalarDepColumns = useMemo(
300+
() => otherColumns.filter((c) => !c.workflowGroupId),
301+
[otherColumns]
302+
)
303+
const groupDepOptions = useMemo<WorkflowGroup[]>(() => {
304+
const seen = new Set<string>()
305+
const result: WorkflowGroup[] = []
306+
for (const c of otherColumns) {
307+
if (!c.workflowGroupId) continue
308+
if (seen.has(c.workflowGroupId)) continue
309+
if (existingGroup && c.workflowGroupId === existingGroup.id) continue
310+
const g = workflowGroups.find((gg) => gg.id === c.workflowGroupId)
311+
if (!g) continue
312+
seen.add(c.workflowGroupId)
313+
result.push(g)
314+
}
315+
return result
316+
}, [otherColumns, workflowGroups, existingGroup])
317+
291318
const [uniqueInput, setUniqueInput] = useState<boolean>(false)
292319
const [selectedWorkflowId, setSelectedWorkflowId] = useState<string>('')
320+
/** Plain (non-workflow-output) column names this group waits on. */
293321
const [deps, setDeps] = useState<string[]>([])
322+
/** Producing workflow group ids this group waits on. Workflow-output columns are
323+
* represented by their parent group, since the schema validator forbids depending
324+
* on a workflow-output column directly (`workflow-columns.ts` enforces this). */
325+
const [groupDeps, setGroupDeps] = useState<string[]>([])
294326
/** Encoded `${blockId}::${path}` values — disambiguates duplicate paths in the picker. */
295327
const [selectedOutputs, setSelectedOutputs] = useState<string[]>([])
296328
/** Surfaces required-field errors only after a save attempt, matching the workflow editor's deploy flow. */
297329
const [showValidation, setShowValidation] = useState(false)
330+
/** Save-time error (network/validation thrown by the mutation). Rendered inline next to the footer
331+
* buttons so it isn't covered by the toaster, which sits over the bottom-right of the panel. */
332+
const [saveError, setSaveError] = useState<string | null>(null)
298333

299334
const existingColumnRef = useRef(existingColumn)
300335
existingColumnRef.current = existingColumn
@@ -304,6 +339,7 @@ export function ColumnSidebar({
304339
useEffect(() => {
305340
if (!open || !configState) return
306341
setShowValidation(false)
342+
setSaveError(null)
307343
const existing = existingColumnRef.current
308344
const cols = allColumnsRef.current
309345
const leftOfCurrent = (() => {
@@ -312,6 +348,16 @@ export function ColumnSidebar({
312348
if (idx === -1) return cols.filter((c) => c.name !== configState.columnName)
313349
return cols.slice(0, idx)
314350
})()
351+
// Default deps when there's no persisted group yet: tick every left-of-current
352+
// scalar column + every left-of-current producing group.
353+
const defaultScalarDeps = leftOfCurrent.filter((c) => !c.workflowGroupId).map((c) => c.name)
354+
const defaultGroupDeps = (() => {
355+
const seen = new Set<string>()
356+
for (const c of leftOfCurrent) {
357+
if (c.workflowGroupId) seen.add(c.workflowGroupId)
358+
}
359+
return Array.from(seen)
360+
})()
315361
if (configState.mode === 'edit') {
316362
const group = existing?.workflowGroupId
317363
? workflowGroups.find((g) => g.id === existing.workflowGroupId)
@@ -323,11 +369,31 @@ export function ColumnSidebar({
323369
setNameInput(existing?.name ?? configState.columnName)
324370
if (group) {
325371
setSelectedWorkflowId(group.workflowId)
326-
setDeps(group.dependencies?.columns ?? leftOfCurrent.map((c) => c.name))
372+
// Sanitize legacy persisted deps: any workflow-output column names that
373+
// sneaked into `dependencies.columns` (writes from before the schema
374+
// validator forbade them) are lifted into `workflowGroups` here so the
375+
// sidebar surfaces a re-saveable state.
376+
const persistedCols = group.dependencies?.columns
377+
const persistedGroups = group.dependencies?.workflowGroups
378+
if (persistedCols !== undefined || persistedGroups !== undefined) {
379+
const liftedGroupIds = new Set(persistedGroups ?? [])
380+
const cleanCols: string[] = []
381+
for (const colName of persistedCols ?? []) {
382+
const c = cols.find((cc) => cc.name === colName)
383+
if (c?.workflowGroupId) liftedGroupIds.add(c.workflowGroupId)
384+
else cleanCols.push(colName)
385+
}
386+
setDeps(cleanCols)
387+
setGroupDeps(Array.from(liftedGroupIds))
388+
} else {
389+
setDeps(defaultScalarDeps)
390+
setGroupDeps(defaultGroupDeps)
391+
}
327392
setSelectedOutputs([]) // re-encoded against current workflow blocks below
328393
} else {
329394
setSelectedWorkflowId('')
330395
setDeps([])
396+
setGroupDeps([])
331397
setSelectedOutputs([])
332398
}
333399
} else {
@@ -337,7 +403,8 @@ export function ColumnSidebar({
337403
setUniqueInput(false)
338404
setNameInput(configState.proposedName)
339405
setSelectedWorkflowId(workflowId)
340-
setDeps(leftOfCurrent.map((c) => c.name))
406+
setDeps(defaultScalarDeps)
407+
setGroupDeps(defaultGroupDeps)
341408
setSelectedOutputs([])
342409
}
343410
}, [open, configState, workflowGroups])
@@ -521,6 +588,12 @@ export function ColumnSidebar({
521588
setDeps((prev) => (prev.includes(name) ? prev.filter((d) => d !== name) : [...prev, name]))
522589
}
523590

591+
const toggleGroupDep = (groupId: string) => {
592+
setGroupDeps((prev) =>
593+
prev.includes(groupId) ? prev.filter((d) => d !== groupId) : [...prev, groupId]
594+
)
595+
}
596+
524597
const toggleOutput = (encoded: string) => {
525598
setSelectedOutputs((prev) =>
526599
prev.includes(encoded) ? prev.filter((v) => v !== encoded) : [...prev, encoded]
@@ -597,6 +670,7 @@ export function ColumnSidebar({
597670

598671
const handleSave = async () => {
599672
if (!configState) return
673+
setSaveError(null)
600674
const trimmedName = nameInput.trim()
601675
if (!trimmedName || (isWorkflow && (!selectedWorkflowId || selectedOutputs.length === 0))) {
602676
setShowValidation(true)
@@ -606,7 +680,10 @@ export function ColumnSidebar({
606680
try {
607681
if (isWorkflow) {
608682
const orderedOutputs = buildOrderedPickedOutputs()
609-
const dependencies: WorkflowGroupDependencies = { columns: deps }
683+
const dependencies: WorkflowGroupDependencies = {
684+
columns: deps,
685+
...(groupDeps.length > 0 ? { workflowGroups: groupDeps } : {}),
686+
}
610687

611688
if (existingGroup) {
612689
// Update path: diff outputs, derive new column names for added entries,
@@ -716,7 +793,7 @@ export function ColumnSidebar({
716793

717794
onClose()
718795
} catch (err) {
719-
toast.error(toError(err).message)
796+
setSaveError(toError(err).message)
720797
}
721798
}
722799

@@ -904,43 +981,94 @@ export function ColumnSidebar({
904981
<FieldDivider />
905982

906983
<div className='flex flex-col gap-[9.5px]'>
907-
<Label className='pl-0.5'>Trigger when these columns are filled</Label>
984+
<Label className='pl-0.5'>Trigger when these are ready</Label>
908985
<div className='flex max-h-[240px] min-w-0 flex-col overflow-y-auto rounded-md border border-[var(--border)]'>
909-
{otherColumns.length === 0 ? (
986+
{scalarDepColumns.length === 0 && groupDepOptions.length === 0 ? (
910987
<div className='px-2 py-3 text-[var(--text-tertiary)] text-small'>
911-
No other columns.
988+
No upstream columns or groups.
912989
</div>
913990
) : (
914-
otherColumns.map((c, idx) => {
915-
const checked = deps.includes(c.name)
916-
return (
917-
<div
918-
key={c.name}
919-
role='checkbox'
920-
aria-checked={checked}
921-
tabIndex={0}
922-
onClick={() => toggleDep(c.name)}
923-
onKeyDown={(e) => {
924-
if (e.key === ' ' || e.key === 'Enter') {
925-
e.preventDefault()
926-
toggleDep(c.name)
927-
}
928-
}}
929-
className={cn(
930-
'flex h-[36px] flex-shrink-0 cursor-pointer items-center gap-2.5 px-2.5 hover:bg-[var(--surface-2)]',
931-
idx < otherColumns.length - 1 && 'border-[var(--border)] border-b'
932-
)}
933-
>
934-
<Checkbox size='sm' checked={checked} className='pointer-events-none' />
935-
<span className='font-medium text-[var(--text-secondary)] text-small'>
936-
{c.name}
937-
</span>
938-
<span className='ml-auto text-[var(--text-tertiary)] text-caption'>
939-
{c.type}
940-
</span>
941-
</div>
942-
)
943-
})
991+
<>
992+
{scalarDepColumns.map((c, idx) => {
993+
const checked = deps.includes(c.name)
994+
const isLast =
995+
idx === scalarDepColumns.length - 1 && groupDepOptions.length === 0
996+
return (
997+
<div
998+
key={`col:${c.name}`}
999+
role='checkbox'
1000+
aria-checked={checked}
1001+
tabIndex={0}
1002+
onClick={() => toggleDep(c.name)}
1003+
onKeyDown={(e) => {
1004+
if (e.key === ' ' || e.key === 'Enter') {
1005+
e.preventDefault()
1006+
toggleDep(c.name)
1007+
}
1008+
}}
1009+
className={cn(
1010+
'flex h-[36px] flex-shrink-0 cursor-pointer items-center gap-2.5 px-2.5 hover:bg-[var(--surface-2)]',
1011+
!isLast && 'border-[var(--border)] border-b'
1012+
)}
1013+
>
1014+
<Checkbox
1015+
size='sm'
1016+
checked={checked}
1017+
className='pointer-events-none'
1018+
/>
1019+
<span className='font-medium text-[var(--text-secondary)] text-small'>
1020+
{c.name}
1021+
</span>
1022+
<span className='ml-auto text-[var(--text-tertiary)] text-caption'>
1023+
{c.type}
1024+
</span>
1025+
</div>
1026+
)
1027+
})}
1028+
{groupDepOptions.map((g, idx) => {
1029+
const checked = groupDeps.includes(g.id)
1030+
const isLast = idx === groupDepOptions.length - 1
1031+
const wf = workflows?.find((w) => w.id === g.workflowId)
1032+
const color = wf?.color ?? 'var(--text-muted)'
1033+
const label = g.name ?? wf?.name ?? 'Workflow'
1034+
return (
1035+
<div
1036+
key={`group:${g.id}`}
1037+
role='checkbox'
1038+
aria-checked={checked}
1039+
tabIndex={0}
1040+
onClick={() => toggleGroupDep(g.id)}
1041+
onKeyDown={(e) => {
1042+
if (e.key === ' ' || e.key === 'Enter') {
1043+
e.preventDefault()
1044+
toggleGroupDep(g.id)
1045+
}
1046+
}}
1047+
className={cn(
1048+
'flex h-[36px] flex-shrink-0 cursor-pointer items-center gap-2.5 px-2.5 hover:bg-[var(--surface-2)]',
1049+
!isLast && 'border-[var(--border)] border-b'
1050+
)}
1051+
>
1052+
<Checkbox
1053+
size='sm'
1054+
checked={checked}
1055+
className='pointer-events-none'
1056+
/>
1057+
<span
1058+
className='h-[10px] w-[10px] shrink-0 rounded-sm'
1059+
style={{ backgroundColor: color }}
1060+
aria-hidden='true'
1061+
/>
1062+
<span className='min-w-0 truncate font-medium text-[var(--text-secondary)] text-small'>
1063+
{label}
1064+
</span>
1065+
<span className='ml-auto text-[var(--text-tertiary)] text-caption'>
1066+
workflow
1067+
</span>
1068+
</div>
1069+
)
1070+
})}
1071+
</>
9441072
)}
9451073
</div>
9461074
</div>
@@ -1019,7 +1147,17 @@ export function ColumnSidebar({
10191147
)}
10201148
</div>
10211149

1022-
<div className='flex items-center justify-end gap-2 border-[var(--border)] border-t px-2 py-3'>
1150+
<div className='flex items-center gap-2 border-[var(--border)] border-t px-2 py-3'>
1151+
{saveError ? (
1152+
<p
1153+
role='alert'
1154+
className='min-w-0 flex-1 break-words pl-0.5 text-destructive text-caption'
1155+
>
1156+
{saveError}
1157+
</p>
1158+
) : (
1159+
<span className='flex-1' />
1160+
)}
10231161
<Button variant='default' size='sm' onClick={onClose}>
10241162
Cancel
10251163
</Button>

apps/sim/background/workflow-column-execution.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ export async function executeWorkflowGroupCellJob(
5050
// surface partial errors in the terminal-error state.
5151
const blockErrors: Record<string, string> = {}
5252
let writeChain: Promise<void> = Promise.resolve()
53+
// Set right before the terminal `completed`/`error` write fires. The
54+
// executor fires `onBlockComplete` callbacks fire-and-forget, so some can
55+
// still be in the microtask queue after `executeWorkflow` resolves; they
56+
// would otherwise enqueue a `running` partial-write that lands after the
57+
// terminal state and clobber it. Once this flag is set, `schedulePartialWrite`
58+
// becomes a no-op.
59+
let terminalWritten = false
5360

5461
try {
5562
const table = await getTableById(tableId)
@@ -147,12 +154,17 @@ export async function executeWorkflowGroupCellJob(
147154

148155
/** Snapshot the current state and append a partial write to the chain. */
149156
const schedulePartialWrite = () => {
157+
if (terminalWritten) return
150158
const dataSnapshot: RowData = { ...accumulatedData }
151159
const blockErrorsSnapshot = { ...blockErrors }
152160
const runningSnapshot = Array.from(runningBlockIds)
153161
writeChain = writeChain
154162
.then(async () => {
155163
if (signal?.aborted) return
164+
// Re-check inside the chain — a write enqueued before the
165+
// terminal flag flipped should still bail if the chain runs
166+
// after the terminal write.
167+
if (terminalWritten) return
156168
await writeState(
157169
{
158170
status: 'running',
@@ -245,7 +257,11 @@ export async function executeWorkflowGroupCellJob(
245257
)
246258

247259
// Drain queued partial writes before the terminal write so a late
248-
// `running` partial doesn't clobber it.
260+
// `running` partial doesn't clobber it. Setting `terminalWritten`
261+
// before draining means any onBlockComplete callbacks still in the
262+
// microtask queue (the executor fires them fire-and-forget) become
263+
// no-ops the moment they try to enqueue.
264+
terminalWritten = true
249265
await writeChain.catch(() => {})
250266

251267
await writeState(
@@ -270,6 +286,7 @@ export async function executeWorkflowGroupCellJob(
270286
// `running` partial doesn't clobber it — same reason as the success
271287
// path above. Reset `runningBlockIds`/`blockErrors` explicitly so the
272288
// renderer sees a clean terminal state (otherwise stale spinners stay).
289+
terminalWritten = true
273290
await writeChain.catch(() => {})
274291
try {
275292
await writeState({

0 commit comments

Comments
 (0)