Skip to content

Commit 14c8f36

Browse files
waleedlatif1waleedlatif1
authored
fix(agent): keep nested tool basic/advanced modes attached to their tools (#7804)
* fix(agent): keep nested tool basic/advanced modes attached to their tools Tool canonical-mode overrides are keyed by array position, so a reorder or removal must move them with the tools. - Workflow edit engine (v2 operations API, Chat) reindexes modes when a batch rewrites a block's tool list, matching tools by content, then by type - Editor persists the tool list and reindexed modes in one realtime operation instead of two independent writes that could partially persist * fix(realtime): reject locked-block tool updates and tighten new types * test(agent): drop redundant cast in tool mode reindex tests * improvement(realtime): share the writable-block check across subblock writes
1 parent ae32a4d commit 14c8f36

14 files changed

Lines changed: 591 additions & 89 deletions

File tree

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,51 @@ describe('search replacement persistence', () => {
121121
expect(mockSet).toHaveBeenCalledTimes(1)
122122
})
123123
})
124+
125+
describe('subblock update with canonical modes persistence', () => {
126+
const tools = [{ type: 'jira', params: { manualProjectId: '{{PROJECT}}' } }]
127+
const canonicalModes = { '0:projectId': 'advanced' as const, model: 'basic' as const }
128+
129+
beforeEach(() => {
130+
vi.clearAllMocks()
131+
mockTransaction.mockImplementation(
132+
async (callback: (tx: typeof transaction) => Promise<void>) => callback(transaction)
133+
)
134+
mockSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) })
135+
})
136+
137+
function updateTools(block: Record<string, unknown>) {
138+
mockSelectWhere.mockResolvedValue([
139+
{
140+
id: 'agent-1',
141+
locked: false,
142+
data: { width: 350, canonicalModes: { '1:projectId': 'advanced' } },
143+
subBlocks: { tools: { id: 'tools', type: 'tool-input', value: [] } },
144+
...block,
145+
},
146+
])
147+
return persistWorkflowOperation('workflow-1', {
148+
operation: SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES,
149+
target: OPERATION_TARGETS.SUBBLOCK,
150+
timestamp: Date.now(),
151+
payload: { blockId: 'agent-1', subblockId: 'tools', value: tools, canonicalModes },
152+
})
153+
}
154+
155+
it('writes the subblock value and replaces canonical modes in one block update', async () => {
156+
await expect(updateTools({})).resolves.toBeUndefined()
157+
158+
expect(mockSet).toHaveBeenCalledTimes(2)
159+
expect(mockSet).toHaveBeenLastCalledWith(
160+
expect.objectContaining({
161+
subBlocks: { tools: { id: 'tools', type: 'tool-input', value: tools } },
162+
data: { width: 350, canonicalModes },
163+
})
164+
)
165+
})
166+
167+
it('rejects a locked block without writing either field', async () => {
168+
await expect(updateTools({ locked: true })).rejects.toThrow('is locked')
169+
expect(mockSet).toHaveBeenCalledTimes(1)
170+
})
171+
})

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

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import { randomFloat } from '@sim/utils/random'
2727
import { loadWorkflowFromNormalizedTablesRaw } from '@sim/workflow-persistence/load'
2828
import { mergeSubBlockValues } from '@sim/workflow-persistence/subblocks'
29+
import type { DbOrTx } from '@sim/workflow-persistence/types'
2930
import {
3031
filterAcyclicEdges,
3132
filterUniqueWorkflowEdges,
@@ -1989,6 +1990,38 @@ async function handleSubflowOperationTx(
19891990
}
19901991
}
19911992

1993+
/** Every block in the workflow by id, for the locked-container check subblock writes need. */
1994+
async function loadSubblockUpdateBlocks(tx: DbOrTx, workflowId: string) {
1995+
const allBlocks = await tx
1996+
.select({
1997+
id: workflowBlocks.id,
1998+
subBlocks: workflowBlocks.subBlocks,
1999+
locked: workflowBlocks.locked,
2000+
data: workflowBlocks.data,
2001+
})
2002+
.from(workflowBlocks)
2003+
.where(eq(workflowBlocks.workflowId, workflowId))
2004+
return Object.fromEntries(allBlocks.map((block) => [block.id, block]))
2005+
}
2006+
2007+
/**
2008+
* The block a subblock write targets, rejecting one that is missing, locked, or in a locked
2009+
* container.
2010+
*/
2011+
function getWritableSubblockUpdateBlock(
2012+
blocksById: Awaited<ReturnType<typeof loadSubblockUpdateBlocks>>,
2013+
blockId: string
2014+
) {
2015+
const block = blocksById[blockId]
2016+
if (!block) {
2017+
throw new Error(`Block ${blockId} not found`)
2018+
}
2019+
if (isWorkflowBlockProtected(blockId, blocksById)) {
2020+
throw new Error(`Block ${blockId} is locked or inside a locked container`)
2021+
}
2022+
return block
2023+
}
2024+
19922025
// Subblock operations - targeted value updates without replacing workflow state
19932026
async function handleSubblockOperationTx(
19942027
tx: any,
@@ -2003,35 +2036,15 @@ async function handleSubblockOperationTx(
20032036
return
20042037
}
20052038

2006-
const allBlocks = await tx
2007-
.select({
2008-
id: workflowBlocks.id,
2009-
subBlocks: workflowBlocks.subBlocks,
2010-
locked: workflowBlocks.locked,
2011-
data: workflowBlocks.data,
2012-
})
2013-
.from(workflowBlocks)
2014-
.where(eq(workflowBlocks.workflowId, workflowId))
2015-
2016-
type SubblockUpdateBlockRecord = (typeof allBlocks)[number]
2017-
const blocksById: Record<string, SubblockUpdateBlockRecord> = Object.fromEntries(
2018-
allBlocks.map((block: SubblockUpdateBlockRecord) => [block.id, block])
2019-
)
2039+
const blocksById = await loadSubblockUpdateBlocks(tx, workflowId)
20202040

20212041
for (const update of updates) {
20222042
const { blockId, subblockId, value, expectedValue } = update
20232043
if (!blockId || !subblockId) {
20242044
throw new Error('Missing required fields for subblock batch update')
20252045
}
20262046

2027-
const block = blocksById[blockId]
2028-
if (!block) {
2029-
throw new Error(`Block ${blockId} not found`)
2030-
}
2031-
2032-
if (isWorkflowBlockProtected(blockId, blocksById)) {
2033-
throw new Error(`Block ${blockId} is locked or inside a locked container`)
2034-
}
2047+
const block = getWritableSubblockUpdateBlock(blocksById, blockId)
20352048

20362049
const subBlocks = { ...((block.subBlocks as Record<string, any>) || {}) }
20372050
const currentSubBlock = subBlocks[subblockId]
@@ -2060,6 +2073,36 @@ async function handleSubblockOperationTx(
20602073
break
20612074
}
20622075

2076+
case SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES: {
2077+
const { blockId, subblockId, value, canonicalModes } = payload
2078+
if (!blockId || !subblockId || !canonicalModes) {
2079+
throw new Error('Missing required fields for subblock update with canonical modes')
2080+
}
2081+
2082+
const blocksById = await loadSubblockUpdateBlocks(tx, workflowId)
2083+
const block = getWritableSubblockUpdateBlock(blocksById, blockId)
2084+
2085+
const subBlocks = {
2086+
...((block.subBlocks as Record<string, Record<string, unknown>> | null) || {}),
2087+
}
2088+
const currentSubBlock = subBlocks[subblockId]
2089+
subBlocks[subblockId] = currentSubBlock
2090+
? { ...currentSubBlock, value }
2091+
: { id: subblockId, type: 'unknown', value }
2092+
2093+
await tx
2094+
.update(workflowBlocks)
2095+
.set({
2096+
subBlocks,
2097+
data: { ...((block.data as Record<string, unknown>) || {}), canonicalModes },
2098+
updatedAt: new Date(),
2099+
})
2100+
.where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
2101+
2102+
logger.debug(`Updated subblock ${blockId}.${subblockId} with canonical modes`)
2103+
break
2104+
}
2105+
20632106
default:
20642107
logger.warn(`Unknown subblock operation: ${operation}`)
20652108
throw new Error(`Unsupported subblock operation: ${operation}`)

‎apps/realtime/src/middleware/permissions.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ describe('checkRolePermission', () => {
114114
const result = checkRolePermission('write', 'subblock-batch-update')
115115
expectPermissionAllowed(result)
116116
})
117+
118+
it('should allow subblock-update-with-canonical-modes operation', () => {
119+
const result = checkRolePermission('write', 'subblock-update-with-canonical-modes')
120+
expectPermissionAllowed(result)
121+
})
117122
})
118123

119124
describe('read role', () => {
@@ -155,6 +160,11 @@ describe('checkRolePermission', () => {
155160
expectPermissionDenied(result, 'read')
156161
})
157162

163+
it('should deny subblock-update-with-canonical-modes operation for read role', () => {
164+
const result = checkRolePermission('read', 'subblock-update-with-canonical-modes')
165+
expectPermissionDenied(result, 'read')
166+
})
167+
158168
it('should deny toggle-enabled operation for read role', () => {
159169
const result = checkRolePermission('read', 'toggle-enabled')
160170
expectPermissionDenied(result, 'read')

‎apps/realtime/src/middleware/permissions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const WRITE_OPERATIONS: string[] = [
5252
// Subblock operations
5353
SUBBLOCK_OPERATIONS.UPDATE,
5454
SUBBLOCK_OPERATIONS.BATCH_UPDATE,
55+
SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES,
5556
// Variable operations
5657
VARIABLE_OPERATIONS.UPDATE,
5758
// Workflow operations

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

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -392,15 +392,8 @@ export const ToolInput = memo(function ToolInput({
392392
[blockId]
393393
)
394394
)
395-
const { collaborativeSetBlockCanonicalMode, collaborativeSetBlockCanonicalModes } =
395+
const { collaborativeSetBlockCanonicalMode, collaborativeSetSubblockValueWithCanonicalModes } =
396396
useCollaborativeWorkflow()
397-
const reindexCanonicalModesOnMutate = useCallback(
398-
(oldTools: StoredTool[], newTools: StoredTool[]) => {
399-
const next = reindexToolCanonicalModes(oldTools, newTools, canonicalModeOverrides)
400-
if (next) collaborativeSetBlockCanonicalModes(blockId, next)
401-
},
402-
[canonicalModeOverrides, collaborativeSetBlockCanonicalModes, blockId]
403-
)
404397

405398
const value = isPreview ? previewValue : storeValue
406399

@@ -412,6 +405,39 @@ export const ToolInput = memo(function ToolInput({
412405
? (value as StoredTool[])
413406
: []
414407

408+
/**
409+
* Commits a tool list that moves or drops selected tools. Their canonical-mode overrides are
410+
* keyed by position, so when any must move they persist in the same operation as the list.
411+
* `positionedTools` is the list holding the kept tool references when `nextTools` clones them.
412+
*/
413+
const setToolsWithReindexedModes = useCallback(
414+
(nextTools: StoredTool[], positionedTools: StoredTool[] = nextTools) => {
415+
const canonicalModes = reindexToolCanonicalModes(
416+
selectedTools,
417+
positionedTools,
418+
canonicalModeOverrides
419+
)
420+
if (!canonicalModes) {
421+
setStoreValue(nextTools)
422+
return
423+
}
424+
collaborativeSetSubblockValueWithCanonicalModes(
425+
blockId,
426+
subBlockId,
427+
structuredClone(nextTools),
428+
canonicalModes
429+
)
430+
},
431+
[
432+
selectedTools,
433+
canonicalModeOverrides,
434+
setStoreValue,
435+
collaborativeSetSubblockValueWithCanonicalModes,
436+
blockId,
437+
subBlockId,
438+
]
439+
)
440+
415441
// Tool categories the consuming block can't run (declared on its tool-input
416442
// subBlock): shown in the picker but greyed out with a tooltip instead of added.
417443
const blockType = useWorkflowStore(useCallback((state) => state.blocks[blockId]?.type, [blockId]))
@@ -857,10 +883,9 @@ export const ToolInput = memo(function ToolInput({
857883
(toolIndex: number) => {
858884
if (isPreview || disabled) return
859885
const updatedTools = selectedTools.filter((_, index) => index !== toolIndex)
860-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
861-
setStoreValue(updatedTools)
886+
setToolsWithReindexedModes(updatedTools)
862887
},
863-
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
888+
[isPreview, disabled, selectedTools, setToolsWithReindexedModes]
864889
)
865890

866891
const handleRemoveAllFromServer = useCallback(
@@ -869,10 +894,9 @@ export const ToolInput = memo(function ToolInput({
869894
const updatedTools = selectedTools.filter(
870895
(t) => !(t.type === 'mcp' && t.params?.serverId === serverId)
871896
)
872-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
873-
setStoreValue(updatedTools)
897+
setToolsWithReindexedModes(updatedTools)
874898
},
875-
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
899+
[isPreview, disabled, selectedTools, setToolsWithReindexedModes]
876900
)
877901

878902
const handleDeleteTool = useCallback(
@@ -900,11 +924,10 @@ export const ToolInput = memo(function ToolInput({
900924
})
901925

902926
if (updatedTools.length !== selectedTools.length) {
903-
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
904-
setStoreValue(updatedTools)
927+
setToolsWithReindexedModes(updatedTools)
905928
}
906929
},
907-
[selectedTools, customTools, reindexCanonicalModesOnMutate, setStoreValue]
930+
[selectedTools, customTools, setToolsWithReindexedModes]
908931
)
909932

910933
const handleParamChange = useCallback(
@@ -1077,8 +1100,7 @@ export const ToolInput = memo(function ToolInput({
10771100
newTools.splice(adjustedDropIndex, 0, draggedTool)
10781101
}
10791102

1080-
reindexCanonicalModesOnMutate(selectedTools, newTools)
1081-
setStoreValue(newTools)
1103+
setToolsWithReindexedModes(newTools)
10821104
setDraggedIndex(null)
10831105
setDragOverIndex(null)
10841106
}
@@ -1177,8 +1199,7 @@ export const ToolInput = memo(function ToolInput({
11771199
...filteredTools.map((tool) => ({ ...tool, isExpanded: false })),
11781200
serverBinding,
11791201
]
1180-
reindexCanonicalModesOnMutate(selectedTools, filteredTools)
1181-
setStoreValue(nextTools)
1202+
setToolsWithReindexedModes(nextTools, filteredTools)
11821203
setMcpServerDrilldown(null)
11831204
setOpen(false)
11841205
},
@@ -1445,7 +1466,7 @@ export const ToolInput = memo(function ToolInput({
14451466
supportsAdvancedMcpServer,
14461467
availableWorkflows,
14471468
isToolAlreadySelected,
1448-
reindexCanonicalModesOnMutate,
1469+
setToolsWithReindexedModes,
14491470
])
14501471

14511472
return (

0 commit comments

Comments
 (0)