Skip to content

Commit c5724b7

Browse files
PlaneInABottlePlaneInABottle
committed
fix: finalize socket parity confirmations
1 parent ec0a3b3 commit c5724b7

7 files changed

Lines changed: 450 additions & 34 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function parseUndoRedoStackKey(key: string): { workflowId: string; userId: strin
4545
}
4646
}
4747

48-
function pruneUndoRedoStacksForWorkflow(
48+
export function pruneUndoRedoStacksForWorkflow(
4949
workflowId: string,
5050
graph?: { blocksById: Record<string, BlockState>; edgesById: Record<string, Edge> }
5151
) {
@@ -1087,6 +1087,11 @@ export function useCollaborativeWorkflow() {
10871087
parentId: u.newParentId || '',
10881088
position: u.newPosition,
10891089
})),
1090+
revertUpdates: batchUpdates.map((u) => ({
1091+
id: u.blockId,
1092+
parentId: u.oldParentId || '',
1093+
position: u.oldPosition,
1094+
})),
10901095
autoConnect: options?.autoConnect,
10911096
},
10921097
},

‎apps/sim/socket/database/operations.test.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,4 +294,104 @@ describe('persistWorkflowOperation', () => {
294294
},
295295
})
296296
})
297+
298+
it('returns only applied batch parent updates when protected targets are skipped', async () => {
299+
const updatedBlocks = new Map<string, Record<string, unknown>>([
300+
['block-1', { id: 'block-1', data: {}, positionX: 0, positionY: 0 }],
301+
['block-2', { id: 'block-2', data: {}, positionX: 5, positionY: 5 }],
302+
['loop-1', { id: 'loop-1', data: {}, positionX: 0, positionY: 0 }],
303+
['locked-parent', { id: 'locked-parent', locked: true, data: {} }],
304+
])
305+
306+
const tx = {
307+
update: vi.fn((table) => {
308+
if (table === mockWorkflowTable) {
309+
return {
310+
set: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })),
311+
}
312+
}
313+
314+
return {
315+
set: vi.fn((values) => ({
316+
where: vi.fn().mockImplementation(() => {
317+
if ('data' in values) {
318+
const blockId = (values.data as Record<string, unknown>).parentId
319+
? 'block-1'
320+
: 'block-2'
321+
updatedBlocks.set(blockId, {
322+
...updatedBlocks.get(blockId),
323+
...values,
324+
})
325+
}
326+
return Promise.resolve(undefined)
327+
}),
328+
returning: vi.fn().mockResolvedValue([{ id: 'block-1' }]),
329+
})),
330+
}
331+
}),
332+
select: vi.fn((selection) => ({
333+
from: vi.fn((table) => ({
334+
where: vi.fn(() => {
335+
if (table === mockWorkflowBlocksTable) {
336+
if ('positionX' in selection) {
337+
return {
338+
limit: vi.fn().mockResolvedValue([
339+
updatedBlocks.get('block-1') ?? {
340+
id: 'block-1',
341+
data: {},
342+
positionX: 0,
343+
positionY: 0,
344+
},
345+
]),
346+
}
347+
}
348+
349+
return Promise.resolve([
350+
{ id: 'block-1', locked: false, data: {} },
351+
{ id: 'block-2', locked: false, data: {} },
352+
{ id: 'loop-1', locked: false, data: {} },
353+
{ id: 'locked-parent', locked: true, data: {} },
354+
])
355+
}
356+
357+
if (table === mockWorkflowEdgesTable) {
358+
return Promise.resolve([])
359+
}
360+
361+
return Promise.resolve([])
362+
}),
363+
})),
364+
})),
365+
insert: vi.fn(() => ({
366+
values: vi.fn(() => ({
367+
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
368+
})),
369+
})),
370+
delete: vi.fn(() => ({
371+
where: vi.fn().mockResolvedValue(undefined),
372+
})),
373+
}
374+
375+
mockTransaction.mockImplementation(async (callback) => callback(tx))
376+
377+
const result = await persistWorkflowOperation('workflow-1', {
378+
operation: 'batch-update-parent',
379+
target: 'blocks',
380+
payload: {
381+
updates: [
382+
{ id: 'block-1', parentId: 'loop-1', position: { x: 10, y: 20 } },
383+
{ id: 'block-2', parentId: 'locked-parent', position: { x: 30, y: 40 } },
384+
],
385+
autoConnect: false,
386+
},
387+
timestamp: Date.now(),
388+
userId: 'user-1',
389+
})
390+
391+
expect(result.appliedPayload).toEqual({
392+
updates: [{ id: 'block-1', parentId: 'loop-1', position: { x: 10, y: 20 } }],
393+
autoConnect: false,
394+
})
395+
expect(result.addedEdges).toEqual([])
396+
})
297397
})

‎apps/sim/socket/handlers/operations.test.ts‎

Lines changed: 83 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,88 @@ describe('setupOperationsHandlers', () => {
615615
)
616616
})
617617

618-
it('broadcasts generic side-effect edge syncs to the initiating client', async () => {
618+
it('returns the authoritative batch parent payload to the initiator', async () => {
619+
mockPersistWorkflowOperation.mockResolvedValue({
620+
appliedPayload: {
621+
updates: [{ id: 'block-1', parentId: 'loop-1', position: { x: 10, y: 20 } }],
622+
},
623+
})
624+
625+
const socketEmit = vi.fn()
626+
const socketHandlers = new Map<string, (data: unknown) => Promise<void>>()
627+
628+
const socket = {
629+
id: 'socket-1',
630+
on: vi.fn((event: string, handler: (data: unknown) => Promise<void>) => {
631+
socketHandlers.set(event, handler)
632+
}),
633+
emit: socketEmit,
634+
to: vi.fn(() => ({
635+
emit: vi.fn(),
636+
})),
637+
}
638+
639+
const roomManager = {
640+
io: {} as never,
641+
initialize: vi.fn(),
642+
isReady: vi.fn(() => true),
643+
shutdown: vi.fn(),
644+
addUserToRoom: vi.fn(),
645+
removeUserFromRoom: vi.fn(),
646+
getWorkflowIdForSocket: vi.fn().mockResolvedValue('workflow-1'),
647+
getUserSession: vi.fn().mockResolvedValue({ userId: 'user-1', userName: 'Test User' }),
648+
getWorkflowUsers: vi.fn().mockResolvedValue([
649+
{
650+
socketId: 'socket-1',
651+
userId: 'user-1',
652+
workflowId: 'workflow-1',
653+
userName: 'Test User',
654+
joinedAt: Date.now(),
655+
lastActivity: Date.now(),
656+
role: 'admin',
657+
},
658+
]),
659+
hasWorkflowRoom: vi.fn().mockResolvedValue(true),
660+
updateUserActivity: vi.fn(),
661+
updateRoomLastModified: vi.fn(),
662+
broadcastPresenceUpdate: vi.fn(),
663+
emitToWorkflow: vi.fn(),
664+
getUniqueUserCount: vi.fn(),
665+
getTotalActiveConnections: vi.fn(),
666+
handleWorkflowDeletion: vi.fn(),
667+
handleWorkflowRevert: vi.fn(),
668+
handleWorkflowUpdate: vi.fn(),
669+
}
670+
671+
setupOperationsHandlers(socket as never, roomManager)
672+
673+
const workflowOperationHandler = socketHandlers.get('workflow-operation')
674+
675+
await workflowOperationHandler?.({
676+
operationId: 'op-parent-authoritative',
677+
operation: BLOCKS_OPERATIONS.BATCH_UPDATE_PARENT,
678+
target: OPERATION_TARGETS.BLOCKS,
679+
payload: {
680+
updates: [
681+
{ id: 'block-1', parentId: 'loop-1', position: { x: 10, y: 20 } },
682+
{ id: 'block-2', parentId: 'locked-parent', position: { x: 30, y: 40 } },
683+
],
684+
},
685+
timestamp: 123,
686+
})
687+
688+
expect(socketEmit).toHaveBeenCalledWith(
689+
'operation-confirmed',
690+
expect.objectContaining({
691+
operationId: 'op-parent-authoritative',
692+
appliedPayload: {
693+
updates: [{ id: 'block-1', parentId: 'loop-1', position: { x: 10, y: 20 } }],
694+
},
695+
})
696+
)
697+
})
698+
699+
it('does not emit edge side-effect syncs when the operation has no handler support', async () => {
619700
mockPersistWorkflowOperation.mockResolvedValue({
620701
removedEdgeIds: ['edge-removed'],
621702
addedEdges: [
@@ -703,34 +784,7 @@ describe('setupOperationsHandlers', () => {
703784
payload: { ids: ['block-1'], locked: true },
704785
})
705786
)
706-
expect(emitToWorkflow).toHaveBeenNthCalledWith(
707-
1,
708-
'workflow-1',
709-
'workflow-operation',
710-
expect.objectContaining({
711-
operation: EDGES_OPERATIONS.BATCH_REMOVE_EDGES,
712-
target: OPERATION_TARGETS.EDGES,
713-
payload: { ids: ['edge-removed'] },
714-
})
715-
)
716-
expect(emitToWorkflow).toHaveBeenNthCalledWith(
717-
2,
718-
'workflow-1',
719-
'workflow-operation',
720-
expect.objectContaining({
721-
operation: EDGES_OPERATIONS.BATCH_ADD_EDGES,
722-
target: OPERATION_TARGETS.EDGES,
723-
payload: {
724-
edges: [
725-
expect.objectContaining({
726-
id: 'edge-added',
727-
source: 'container-1',
728-
target: 'block-1',
729-
}),
730-
],
731-
},
732-
})
733-
)
787+
expect(emitToWorkflow).not.toHaveBeenCalled()
734788
expect(socketEmit).toHaveBeenCalledWith(
735789
'operation-confirmed',
736790
expect.objectContaining({ operationId: 'op-2', serverTimestamp: expect.any(Number) })

‎apps/sim/socket/validation/schemas.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,15 @@ export const BatchUpdateParentSchema = z.object({
328328
position: PositionSchema,
329329
})
330330
),
331+
revertUpdates: z
332+
.array(
333+
z.object({
334+
id: z.string(),
335+
parentId: z.string().nullable().optional(),
336+
position: PositionSchema,
337+
})
338+
)
339+
.optional(),
331340
autoConnect: z.boolean().optional(),
332341
}),
333342
timestamp: z.number(),

0 commit comments

Comments
 (0)