Skip to content

Commit d2306ad

Browse files
committed
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
1 parent 2799994 commit d2306ad

14 files changed

Lines changed: 583 additions & 81 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('skips a locked block without writing either field', async () => {
168+
await expect(updateTools({ locked: true })).resolves.toBeUndefined()
169+
expect(mockSet).toHaveBeenCalledTimes(1)
170+
})
171+
})

apps/realtime/src/database/operations.ts

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,30 @@ async function handleSubflowOperationTx(
19891989
}
19901990
}
19911991

1992+
interface SubblockUpdateBlockRecord {
1993+
id: string
1994+
subBlocks: unknown
1995+
locked: boolean
1996+
data: unknown
1997+
}
1998+
1999+
/** Every block in the workflow by id, for the locked-container check subblock writes need. */
2000+
async function loadSubblockUpdateBlocks(
2001+
tx: any,
2002+
workflowId: string
2003+
): Promise<Record<string, SubblockUpdateBlockRecord>> {
2004+
const allBlocks: SubblockUpdateBlockRecord[] = await tx
2005+
.select({
2006+
id: workflowBlocks.id,
2007+
subBlocks: workflowBlocks.subBlocks,
2008+
locked: workflowBlocks.locked,
2009+
data: workflowBlocks.data,
2010+
})
2011+
.from(workflowBlocks)
2012+
.where(eq(workflowBlocks.workflowId, workflowId))
2013+
return Object.fromEntries(allBlocks.map((block) => [block.id, block]))
2014+
}
2015+
19922016
// Subblock operations - targeted value updates without replacing workflow state
19932017
async function handleSubblockOperationTx(
19942018
tx: any,
@@ -2003,20 +2027,7 @@ async function handleSubblockOperationTx(
20032027
return
20042028
}
20052029

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-
)
2030+
const blocksById = await loadSubblockUpdateBlocks(tx, workflowId)
20202031

20212032
for (const update of updates) {
20222033
const { blockId, subblockId, value, expectedValue } = update
@@ -2060,6 +2071,41 @@ async function handleSubblockOperationTx(
20602071
break
20612072
}
20622073

2074+
case SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES: {
2075+
const { blockId, subblockId, value, canonicalModes } = payload
2076+
if (!blockId || !subblockId || !canonicalModes) {
2077+
throw new Error('Missing required fields for subblock update with canonical modes')
2078+
}
2079+
2080+
const blocksById = await loadSubblockUpdateBlocks(tx, workflowId)
2081+
const block = blocksById[blockId]
2082+
if (!block) {
2083+
throw new Error(`Block ${blockId} not found`)
2084+
}
2085+
if (isWorkflowBlockProtected(blockId, blocksById)) {
2086+
logger.info(`Skipping subblock update of locked block ${blockId}`)
2087+
break
2088+
}
2089+
2090+
const subBlocks = { ...((block.subBlocks as Record<string, any>) || {}) }
2091+
const currentSubBlock = subBlocks[subblockId]
2092+
subBlocks[subblockId] = currentSubBlock
2093+
? { ...currentSubBlock, value }
2094+
: { id: subblockId, type: 'unknown', value }
2095+
2096+
await tx
2097+
.update(workflowBlocks)
2098+
.set({
2099+
subBlocks,
2100+
data: { ...((block.data as Record<string, unknown>) || {}), canonicalModes },
2101+
updatedAt: new Date(),
2102+
})
2103+
.where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
2104+
2105+
logger.debug(`Updated subblock ${blockId}.${subblockId} with canonical modes`)
2106+
break
2107+
}
2108+
20632109
default:
20642110
logger.warn(`Unknown subblock operation: ${operation}`)
20652111
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)