Skip to content

Commit 09bf6fe

Browse files
j15zj15z
committed
fix(workflows): save reordered tools and canonical modes atomically
1 parent 1a7c827 commit 09bf6fe

5 files changed

Lines changed: 159 additions & 21 deletions

File tree

‎apps/realtime/src/database/operations.test.ts‎

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
/** @vitest-environment node */
2-
import { OPERATION_TARGETS, SUBBLOCK_OPERATIONS } from '@sim/realtime-protocol/constants'
2+
import {
3+
BLOCK_OPERATIONS,
4+
OPERATION_TARGETS,
5+
SUBBLOCK_OPERATIONS,
6+
} from '@sim/realtime-protocol/constants'
37
import { beforeEach, describe, expect, it, vi } from 'vitest'
48

59
const { mockTransaction, mockSelectWhere, mockSet } = vi.hoisted(() => ({
@@ -121,3 +125,89 @@ describe('search replacement persistence', () => {
121125
expect(mockSet).toHaveBeenCalledTimes(1)
122126
})
123127
})
128+
129+
describe('atomic tool reordering', () => {
130+
const block = {
131+
id: 'agent-1',
132+
type: 'agent',
133+
name: 'Agent',
134+
position: { x: 0, y: 0 },
135+
locked: false,
136+
subBlocks: {
137+
tools: {
138+
id: 'tools',
139+
type: 'tool-input',
140+
value: [{ type: 'jira', params: { projectId: 'project-1' } }],
141+
},
142+
},
143+
data: {},
144+
}
145+
146+
beforeEach(() => {
147+
vi.clearAllMocks()
148+
mockTransaction.mockImplementation(
149+
async (callback: (tx: typeof transaction) => Promise<void>) => callback(transaction)
150+
)
151+
mockSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) })
152+
mockSelectWhere.mockImplementation(() =>
153+
Object.assign(
154+
Promise.resolve([{ ...block, subBlocks: { tools: { value: [{ type: 'function' }] } } }]),
155+
{
156+
limit: async () => [
157+
{ ...block, subBlocks: { tools: { value: [{ type: 'function' }] } } },
158+
],
159+
}
160+
)
161+
)
162+
})
163+
164+
it('persists a reordered tool array and its mode map in one write', async () => {
165+
const first = {
166+
type: 'jira',
167+
params: { projectId: 'project-1', manualProjectId: '<Start.project>' },
168+
}
169+
const second = {
170+
type: 'jira',
171+
params: { projectId: 'project-2', manualProjectId: '<Start.project>' },
172+
}
173+
const original = {
174+
...block,
175+
subBlocks: { tools: { id: 'tools', type: 'tool-input', value: [first, second] } },
176+
data: { canonicalModes: { '1:projectId': 'advanced' } },
177+
}
178+
mockSelectWhere.mockResolvedValue([original])
179+
mockSet.mockReturnValue({
180+
where: () =>
181+
Object.assign(Promise.resolve(undefined), { returning: async () => [{ id: block.id }] }),
182+
})
183+
const subBlocks = { tools: { id: 'tools', type: 'tool-input', value: [second, first] } }
184+
const canonicalModes = { '0:projectId': 'advanced' }
185+
await expect(
186+
persistWorkflowOperation('workflow-1', {
187+
operation: BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES,
188+
target: OPERATION_TARGETS.BLOCK,
189+
timestamp: Date.now(),
190+
payload: { id: block.id, subBlocks, data: { canonicalModes } },
191+
})
192+
).resolves.toBeUndefined()
193+
expect(mockSet).toHaveBeenLastCalledWith(
194+
expect.objectContaining({ subBlocks, data: { canonicalModes } })
195+
)
196+
})
197+
198+
it('refuses an atomic tool update inside a locked container', async () => {
199+
mockSelectWhere.mockResolvedValue([
200+
{ ...block, data: { parentId: 'container' } },
201+
{ id: 'container', type: 'loop', locked: true, data: {} },
202+
])
203+
await expect(
204+
persistWorkflowOperation('workflow-1', {
205+
operation: BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES,
206+
target: OPERATION_TARGETS.BLOCK,
207+
timestamp: Date.now(),
208+
payload: { id: block.id, subBlocks: block.subBlocks, data: { canonicalModes: {} } },
209+
})
210+
).rejects.toThrow('locked')
211+
expect(mockSet).toHaveBeenCalledTimes(1)
212+
})
213+
})

‎apps/realtime/src/database/operations.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -820,17 +820,34 @@ async function handleBlockOperationTx(
820820
throw new Error('Missing required fields for replace canonical modes operation')
821821
}
822822

823-
const existingBlock = await tx
824-
.select({ data: workflowBlocks.data })
823+
const allBlocks = await tx
824+
.select({
825+
id: workflowBlocks.id,
826+
locked: workflowBlocks.locked,
827+
subBlocks: workflowBlocks.subBlocks,
828+
data: workflowBlocks.data,
829+
})
825830
.from(workflowBlocks)
826-
.where(and(eq(workflowBlocks.id, payload.id), eq(workflowBlocks.workflowId, workflowId)))
827-
.limit(1)
831+
.where(eq(workflowBlocks.workflowId, workflowId))
832+
const blocksById = Object.fromEntries(
833+
allBlocks.map((block: { id: string; locked: boolean; data: Record<string, unknown> }) => [
834+
block.id,
835+
block,
836+
])
837+
)
838+
if (isWorkflowBlockProtected(payload.id, blocksById)) {
839+
throw new Error(`Block ${payload.id} is locked or inside a locked container`)
840+
}
841+
const existingBlock = allBlocks.filter((block: { id: string }) => block.id === payload.id)
828842

829843
const currentData = (existingBlock?.[0]?.data as Record<string, unknown>) || {}
830844

845+
const subBlocks = { ...(existingBlock[0]?.subBlocks || {}), ...(payload.subBlocks || {}) }
846+
831847
const updateResult = await tx
832848
.update(workflowBlocks)
833849
.set({
850+
...(payload.subBlocks ? { subBlocks } : {}),
834851
data: {
835852
...currentData,
836853
canonicalModes: payload.data.canonicalModes,

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -395,11 +395,15 @@ export const ToolInput = memo(function ToolInput({
395395
const { collaborativeSetBlockCanonicalMode, collaborativeSetBlockCanonicalModes } =
396396
useCollaborativeWorkflow()
397397
const reindexCanonicalModesOnMutate = useCallback(
398-
(oldTools: StoredTool[], newTools: StoredTool[]) => {
398+
(oldTools: StoredTool[], newTools: StoredTool[], persistedTools = newTools) => {
399399
const next = reindexToolCanonicalModes(oldTools, newTools, canonicalModeOverrides)
400-
if (next) collaborativeSetBlockCanonicalModes(blockId, next)
400+
if (!next) return false
401+
collaborativeSetBlockCanonicalModes(blockId, next, {
402+
[subBlockId]: { id: subBlockId, type: 'tool-input', value: persistedTools },
403+
})
404+
return true
401405
},
402-
[canonicalModeOverrides, collaborativeSetBlockCanonicalModes, blockId]
406+
[canonicalModeOverrides, collaborativeSetBlockCanonicalModes, blockId, subBlockId]
403407
)
404408

405409
const value = isPreview ? previewValue : storeValue
@@ -857,8 +861,9 @@ export const ToolInput = memo(function ToolInput({
857861
(toolIndex: number) => {
858862
if (isPreview || disabled) return
859863
const updatedTools = selectedTools.filter((_, index) => index !== toolIndex)
860-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
861-
setStoreValue(updatedTools)
864+
if (!reindexCanonicalModesOnMutate(selectedTools, updatedTools)) {
865+
setStoreValue(updatedTools)
866+
}
862867
},
863868
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
864869
)
@@ -869,8 +874,9 @@ export const ToolInput = memo(function ToolInput({
869874
const updatedTools = selectedTools.filter(
870875
(t) => !(t.type === 'mcp' && t.params?.serverId === serverId)
871876
)
872-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
873-
setStoreValue(updatedTools)
877+
if (!reindexCanonicalModesOnMutate(selectedTools, updatedTools)) {
878+
setStoreValue(updatedTools)
879+
}
874880
},
875881
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
876882
)
@@ -900,8 +906,9 @@ export const ToolInput = memo(function ToolInput({
900906
})
901907

902908
if (updatedTools.length !== selectedTools.length) {
903-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
904-
setStoreValue(updatedTools)
909+
if (!reindexCanonicalModesOnMutate(selectedTools, updatedTools)) {
910+
setStoreValue(updatedTools)
911+
}
905912
}
906913
},
907914
[selectedTools, customTools, reindexCanonicalModesOnMutate, setStoreValue]
@@ -1077,8 +1084,9 @@ export const ToolInput = memo(function ToolInput({
10771084
newTools.splice(adjustedDropIndex, 0, draggedTool)
10781085
}
10791086

1080-
reindexCanonicalModesOnMutate(selectedTools, newTools)
1081-
setStoreValue(newTools)
1087+
if (!reindexCanonicalModesOnMutate(selectedTools, newTools)) {
1088+
setStoreValue(newTools)
1089+
}
10821090
setDraggedIndex(null)
10831091
setDragOverIndex(null)
10841092
}
@@ -1177,8 +1185,9 @@ export const ToolInput = memo(function ToolInput({
11771185
...filteredTools.map((tool) => ({ ...tool, isExpanded: false })),
11781186
serverBinding,
11791187
]
1180-
reindexCanonicalModesOnMutate(selectedTools, filteredTools)
1181-
setStoreValue(nextTools)
1188+
if (!reindexCanonicalModesOnMutate(selectedTools, filteredTools, nextTools)) {
1189+
setStoreValue(nextTools)
1190+
}
11821191
setMcpServerDrilldown(null)
11831192
setOpen(false)
11841193
},

‎apps/sim/hooks/use-collaborative-workflow.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
WORKFLOW_OPERATIONS,
1313
} from '@sim/realtime-protocol/constants'
1414
import { generateId } from '@sim/utils/id'
15-
import type { BlockRetryConfig } from '@sim/workflow-types/workflow'
15+
import type { BlockRetryConfig, SubBlockState } from '@sim/workflow-types/workflow'
1616
import { filterAcyclicEdges, getWorkflowBlockNameConflict } from '@sim/workflow-types/workflow'
1717
import { useQueryClient } from '@tanstack/react-query'
1818
import type { Edge } from '@xyflow/react'
@@ -59,6 +59,10 @@ import { findAllDescendantNodes, isBlockProtected } from '@/stores/workflows/wor
5959

6060
const logger = createLogger('CollaborativeWorkflow')
6161

62+
interface CanonicalModeSubBlockState extends Omit<SubBlockState, 'value'> {
63+
value: unknown
64+
}
65+
6266
export function useCollaborativeWorkflow() {
6367
const queryClient = useQueryClient()
6468
const undoRedo = useUndoRedo()
@@ -245,6 +249,13 @@ export function useCollaborativeWorkflow() {
245249
useWorkflowStore
246250
.getState()
247251
.setBlockCanonicalModes(payload.id, payload.data?.canonicalModes ?? {})
252+
if (payload.subBlocks) {
253+
for (const [subBlockId, subBlock] of Object.entries(
254+
payload.subBlocks as Record<string, CanonicalModeSubBlockState>
255+
)) {
256+
useSubBlockStore.getState().setValue(payload.id, subBlockId, subBlock.value)
257+
}
258+
}
248259
break
249260
}
250261
} else if (target === OPERATION_TARGETS.BLOCKS) {
@@ -1367,14 +1378,24 @@ export function useCollaborativeWorkflow() {
13671378
* {@link collaborativeSetBlockCanonicalMode}. Needed to reindex nested tool-input overrides on
13681379
* reorder/removal: a merge can't atomically drop a now-stale index key, and sequential
13691380
* per-key sets can clobber each other when two tools swap positions.
1381+
* Paired tool values travel in the same operation so their indexes stay aligned with the modes.
13701382
*/
13711383
const collaborativeSetBlockCanonicalModes = useCallback(
1372-
(id: string, canonicalModes: Record<string, 'basic' | 'advanced'>) => {
1384+
(
1385+
id: string,
1386+
canonicalModes: Record<string, 'basic' | 'advanced'>,
1387+
subBlocks?: Record<string, CanonicalModeSubBlockState>
1388+
) => {
13731389
if (isBaselineDiffView) {
13741390
return
13751391
}
13761392

13771393
useWorkflowStore.getState().setBlockCanonicalModes(id, canonicalModes)
1394+
if (subBlocks) {
1395+
for (const [subBlockId, subBlock] of Object.entries(subBlocks)) {
1396+
useSubBlockStore.getState().setValue(id, subBlockId, subBlock.value)
1397+
}
1398+
}
13781399

13791400
if (!activeWorkflowId) {
13801401
return
@@ -1386,7 +1407,7 @@ export function useCollaborativeWorkflow() {
13861407
operation: {
13871408
operation: BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES,
13881409
target: OPERATION_TARGETS.BLOCK,
1389-
payload: { id, data: { canonicalModes } },
1410+
payload: { id, data: { canonicalModes }, ...(subBlocks ? { subBlocks } : {}) },
13901411
},
13911412
workflowId: activeWorkflowId,
13921413
userId: session?.user?.id || 'unknown',

‎apps/sim/stores/workflows/subblock/store.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export const EMPTY_BLOCK_SUBBLOCK_VALUES: Record<string, SubBlockValue> = {}
5353
*
5454
* - remote-broadcast application — already persisted server-side
5555
* - undo/redo — persists via its own queued inverse operations
56+
* - canonical tool reindexing — queues paired tool values and modes in one operation
5657
* - synthetic tool subblock ids — excluded from both persistence and comparison
5758
* - whole-document replacement — the server's own state, re-seeded
5859
* - webhook management's runtime ids (webhookId/triggerPath/triggerConfig/

0 commit comments

Comments
 (0)