From 3d5617d1ff0bf8a98c4087ebc86fb58c18531f6b Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Mon, 14 Sep 2026 16:12:06 -0700 Subject: [PATCH] fix(agent): keep tool modes on their tools across API and concurrent edits - API and Chat edits that switch a basic/advanced pair by sending one side keep the side they left out, for Permission Mode and nested tool params such as Jira's project picker and manual ID. The side sent selects the mode, and a pair resent unchanged keeps its mode. - A tool-scoped mode toggle names its tool. The realtime server refuses it when that position no longer holds the tool, editors ignore a broadcast toggle for a tool they moved or removed, and removing or reordering tools always saves the list and its modes together, so a concurrent toggle can no longer land on another tool or an empty position. Co-Authored-By: Claude Opus 5 --- apps/realtime/src/database/operations.test.ts | 100 +++++++++++- apps/realtime/src/database/operations.ts | 44 +++++- apps/realtime/src/handlers/operations.test.ts | 56 +++++++ apps/realtime/src/handlers/operations.ts | 32 ++-- .../components/tool-input/tool-input.tsx | 38 +++-- apps/sim/hooks/use-collaborative-workflow.ts | 31 +++- apps/sim/lib/workflows/editing/builders.ts | 22 ++- apps/sim/lib/workflows/editing/engine.ts | 51 ++++-- .../lib/workflows/editing/operations.test.ts | 42 +++++ .../lib/workflows/editing/tool-modes.test.ts | 146 ++++++++++++++++++ apps/sim/lib/workflows/editing/tool-modes.ts | 139 +++++++++++++++++ .../sim/lib/workflows/subblocks/visibility.ts | 37 +++-- .../tool-input/usage-control.test.ts | 27 ---- .../lib/workflows/tool-input/usage-control.ts | 31 ---- packages/realtime-protocol/package.json | 4 + packages/realtime-protocol/src/schemas.ts | 15 ++ .../realtime-protocol/src/tool-input.test.ts | 83 ++++++++++ packages/realtime-protocol/src/tool-input.ts | 78 ++++++++++ 18 files changed, 842 insertions(+), 134 deletions(-) create mode 100644 apps/sim/lib/workflows/editing/tool-modes.test.ts create mode 100644 apps/sim/lib/workflows/editing/tool-modes.ts create mode 100644 packages/realtime-protocol/src/tool-input.test.ts create mode 100644 packages/realtime-protocol/src/tool-input.ts diff --git a/apps/realtime/src/database/operations.test.ts b/apps/realtime/src/database/operations.test.ts index fb81bbd304d..dbc68884957 100644 --- a/apps/realtime/src/database/operations.test.ts +++ b/apps/realtime/src/database/operations.test.ts @@ -1,5 +1,10 @@ /** @vitest-environment node */ -import { OPERATION_TARGETS, SUBBLOCK_OPERATIONS } from '@sim/realtime-protocol/constants' +import { + BLOCK_OPERATIONS, + OPERATION_TARGETS, + SUBBLOCK_OPERATIONS, +} from '@sim/realtime-protocol/constants' +import { getToolInputIdentity } from '@sim/realtime-protocol/tool-input' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockTransaction, mockSelectWhere, mockSet } = vi.hoisted(() => ({ @@ -98,7 +103,7 @@ describe('search replacement persistence', () => { }, ] - await expect(replaceTools(stored)).resolves.toBeUndefined() + await expect(replaceTools(stored)).resolves.toEqual({ applied: true }) expect(mockSet).toHaveBeenLastCalledWith( expect.objectContaining({ subBlocks: { tools: { id: 'tools', type: 'tool-input', value: replacement } }, @@ -153,7 +158,7 @@ describe('subblock update with canonical modes persistence', () => { } it('writes the subblock value and replaces canonical modes in one block update', async () => { - await expect(updateTools({})).resolves.toBeUndefined() + await expect(updateTools({})).resolves.toEqual({ applied: true }) expect(mockSet).toHaveBeenCalledTimes(2) expect(mockSet).toHaveBeenLastCalledWith( @@ -169,3 +174,92 @@ describe('subblock update with canonical modes persistence', () => { expect(mockSet).toHaveBeenCalledTimes(1) }) }) + +describe('tool-scoped canonical mode persistence', () => { + const jira = { type: 'jira', operation: 'read-bulk', params: { manualProjectId: 'MAN' } } + const wikipedia = { type: 'wikipedia', operation: 'wikipedia_search', params: { query: 'Sim' } } + const mockReturning = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + mockTransaction.mockImplementation( + async (callback: (tx: typeof transaction) => Promise) => callback(transaction) + ) + mockReturning.mockResolvedValue([{ id: 'agent-1' }]) + mockSet.mockReturnValue({ + where: vi.fn(() => Object.assign(Promise.resolve(undefined), { returning: mockReturning })), + }) + }) + + function toggle(storedTools: unknown[], payload: Record) { + mockSelectWhere.mockReturnValue({ + limit: vi.fn().mockResolvedValue([ + { + data: { canonicalModes: { '0:projectId': 'basic' } }, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: storedTools } }, + }, + ]), + }) + return persistWorkflowOperation('workflow-1', { + operation: BLOCK_OPERATIONS.UPDATE_CANONICAL_MODE, + target: OPERATION_TARGETS.BLOCK, + timestamp: Date.now(), + payload: { id: 'agent-1', canonicalMode: 'advanced', ...payload }, + }) + } + + function toolRef(toolIndex: number, tool: unknown) { + return { subblockId: 'tools', toolIndex, identity: getToolInputIdentity(tool) ?? {} } + } + + it('applies a mode while its tool still holds the toggled position', async () => { + await expect( + toggle([wikipedia, jira], { + canonicalId: '1:agentToolUsageControl', + toolRef: toolRef(1, jira), + }) + ).resolves.toEqual({ applied: true }) + + expect(mockSet).toHaveBeenLastCalledWith( + expect.objectContaining({ + data: { + canonicalModes: { '0:projectId': 'basic', '1:agentToolUsageControl': 'advanced' }, + }, + }) + ) + }) + + it('refuses a mode whose tool another editor moved first', async () => { + await expect( + toggle([jira, wikipedia], { + canonicalId: '1:agentToolUsageControl', + toolRef: toolRef(1, jira), + }) + ).resolves.toEqual({ applied: false }) + + expect(mockSet).toHaveBeenCalledTimes(1) + }) + + it('refuses a mode whose tool another editor removed first', async () => { + await expect( + toggle([wikipedia], { canonicalId: '1:agentToolUsageControl', toolRef: toolRef(1, jira) }) + ).resolves.toEqual({ applied: false }) + + expect(mockSet).toHaveBeenCalledTimes(1) + }) + + it('refuses a key that does not match the referenced position', async () => { + await expect( + toggle([wikipedia, jira], { + canonicalId: '0:agentToolUsageControl', + toolRef: toolRef(1, jira), + }) + ).resolves.toEqual({ applied: false }) + }) + + it('applies a mode sent without a tool reference as before', async () => { + await expect(toggle([], { canonicalId: 'files' })).resolves.toEqual({ applied: true }) + + expect(mockSet).toHaveBeenCalledTimes(2) + }) +}) diff --git a/apps/realtime/src/database/operations.ts b/apps/realtime/src/database/operations.ts index cd4316448bf..ee7f96450cf 100644 --- a/apps/realtime/src/database/operations.ts +++ b/apps/realtime/src/database/operations.ts @@ -23,6 +23,8 @@ import { VARIABLE_OPERATIONS, WORKFLOW_OPERATIONS, } from '@sim/realtime-protocol/constants' +import { isToolInputRefCurrent } from '@sim/realtime-protocol/tool-input' +import { isRecordLike } from '@sim/utils/object' import { randomFloat } from '@sim/utils/random' import { loadWorkflowFromNormalizedTablesRaw } from '@sim/workflow-persistence/load' import { mergeSubBlockValues } from '@sim/workflow-persistence/subblocks' @@ -374,7 +376,15 @@ export async function getWorkflowState(workflowId: string) { } } -export async function persistWorkflowOperation(workflowId: string, operation: any) { +export interface PersistWorkflowOperationResult { + /** False when the operation was a stale no-op that must not be broadcast to other editors. */ + applied: boolean +} + +export async function persistWorkflowOperation( + workflowId: string, + operation: any +): Promise { const startTime = Date.now() try { const { operation: op, target, payload, timestamp, userId } = operation @@ -392,6 +402,7 @@ export async function persistWorkflowOperation(workflowId: string, operation: an }) } + let applied = true await db.transaction(async (tx) => { // This UPDATE is also this workflow's write-serialization point, not // just a timestamp bump: it takes a row lock on `workflow` for the @@ -408,9 +419,11 @@ export async function persistWorkflowOperation(workflowId: string, operation: an .where(eq(workflow.id, workflowId)) switch (target) { - case OPERATION_TARGETS.BLOCK: - await handleBlockOperationTx(tx, workflowId, op, payload) + case OPERATION_TARGETS.BLOCK: { + const result = await handleBlockOperationTx(tx, workflowId, op, payload) + applied = result?.applied ?? true break + } case OPERATION_TARGETS.BLOCKS: await handleBlocksOperationTx(tx, workflowId, op, payload) break @@ -457,6 +470,8 @@ export async function persistWorkflowOperation(workflowId: string, operation: an workflowId: `${workflowId.substring(0, 8)}...`, }) } + + return { applied } } catch (error) { const duration = Date.now() - startTime logger.error( @@ -504,12 +519,17 @@ async function auditWorkflowLockToggle(workflowId: string, actorId: string): Pro }) } +function getSubBlockValue(subBlocks: unknown, subblockId: string): unknown { + const subBlock = isRecordLike(subBlocks) ? subBlocks[subblockId] : undefined + return isRecordLike(subBlock) ? subBlock.value : undefined +} + async function handleBlockOperationTx( tx: any, workflowId: string, operation: string, payload: any -) { +): Promise { switch (operation) { case BLOCK_OPERATIONS.UPDATE_POSITION: { if (!payload.id || !payload.position) { @@ -782,11 +802,25 @@ async function handleBlockOperationTx( } const existingBlock = await tx - .select({ data: workflowBlocks.data }) + .select({ data: workflowBlocks.data, subBlocks: workflowBlocks.subBlocks }) .from(workflowBlocks) .where(and(eq(workflowBlocks.id, payload.id), eq(workflowBlocks.workflowId, workflowId))) .limit(1) + if ( + payload.toolRef && + !isToolInputRefCurrent( + getSubBlockValue(existingBlock?.[0]?.subBlocks, payload.toolRef.subblockId), + payload.canonicalId, + payload.toolRef + ) + ) { + logger.debug( + `Skipped stale tool canonical mode: ${payload.id} -> ${payload.canonicalId}: ${payload.canonicalMode}` + ) + return { applied: false } + } + const currentData = (existingBlock?.[0]?.data as Record) || {} const currentCanonicalModes = (currentData.canonicalModes as Record) || {} const canonicalModes = { diff --git a/apps/realtime/src/handlers/operations.test.ts b/apps/realtime/src/handlers/operations.test.ts index a386a81b677..d8f51b23f4b 100644 --- a/apps/realtime/src/handlers/operations.test.ts +++ b/apps/realtime/src/handlers/operations.test.ts @@ -179,3 +179,59 @@ describe('workflow operation ACL', () => { }) }) }) + +describe('tool-scoped canonical mode broadcast', () => { + beforeEach(() => { + vi.clearAllMocks() + mockAssertMutable.mockResolvedValue(undefined) + mockAuthorizeWorkflow.mockResolvedValue({ allowed: true, workspacePermission: 'write' }) + }) + + function toolModeToggle(operationId: string) { + return { + operationId, + operation: 'update-canonical-mode', + target: 'block', + timestamp: Date.now(), + payload: { + id: BLOCK_ID, + canonicalId: '1:agentToolUsageControl', + canonicalMode: 'advanced', + toolRef: { subblockId: 'tools', toolIndex: 1, identity: { type: 'jira' } }, + }, + } + } + + it('confirms a stale toggle to its sender without broadcasting it', async () => { + mockPersist.mockResolvedValue({ applied: false }) + const { socket, handlers, toEmit } = setup('sock-tool-mode-1', 'write') + + await handlers['workflow-operation'](toolModeToggle('op-stale')) + + expect(mockPersist).toHaveBeenCalledWith( + WORKFLOW_ID, + expect.objectContaining({ + payload: expect.objectContaining({ + toolRef: { subblockId: 'tools', toolIndex: 1, identity: { type: 'jira' } }, + }), + }) + ) + expect(toEmit).not.toHaveBeenCalled() + expect(socket.emit).toHaveBeenCalledWith( + 'operation-confirmed', + expect.objectContaining({ operationId: 'op-stale' }) + ) + }) + + it('broadcasts a toggle that applied', async () => { + mockPersist.mockResolvedValue({ applied: true }) + const { handlers, toEmit } = setup('sock-tool-mode-2', 'write') + + await handlers['workflow-operation'](toolModeToggle('op-applied')) + + expect(toEmit).toHaveBeenCalledWith( + 'workflow-operation', + expect.objectContaining({ operation: 'update-canonical-mode' }) + ) + }) +}) diff --git a/apps/realtime/src/handlers/operations.ts b/apps/realtime/src/handlers/operations.ts index d3d603887db..84ff67befde 100644 --- a/apps/realtime/src/handlers/operations.ts +++ b/apps/realtime/src/handlers/operations.ts @@ -566,7 +566,7 @@ export function setupOperationsHandlers(socket: AuthenticatedSocket, roomManager } // For non-position operations, persist first then broadcast - await persistWorkflowOperation(workflowId, { + const persisted = await persistWorkflowOperation(workflowId, { operation, target, payload, @@ -576,21 +576,23 @@ export function setupOperationsHandlers(socket: AuthenticatedSocket, roomManager await roomManager.updateRoomLastModified(wf(workflowId)) - const broadcastData = { - operation, - target, - payload, - timestamp: operationTimestamp, - senderId: socket.id, - userId: session.userId, - userName: session.userName, - metadata: { - workflowId, - operationId: generateId(), - }, - } + if (persisted?.applied !== false) { + const broadcastData = { + operation, + target, + payload, + timestamp: operationTimestamp, + senderId: socket.id, + userId: session.userId, + userName: session.userName, + metadata: { + workflowId, + operationId: generateId(), + }, + } - socket.to(workflowId).emit('workflow-operation', broadcastData) + socket.to(workflowId).emit('workflow-operation', broadcastData) + } if (operationId) { socket.emit('operation-confirmed', { diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index 54b7d751123..3ba0c9a1f09 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -16,6 +16,9 @@ import { } from '@sim/emcn' import { ArrowLeft, ChevronRight, Pencil, Server, Wrench, X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' +import type { ToolInputRef } from '@sim/realtime-protocol/schemas' +import { getToolInputIdentity } from '@sim/realtime-protocol/tool-input' +import { filterUndefined } from '@sim/utils/object' import { useParams } from 'next/navigation' import { McpIcon, WorkflowIcon } from '@/components/icons' import { McpOperationPolicyEditor } from '@/components/mcp/operation-policy-editor' @@ -116,6 +119,16 @@ import { const logger = createLogger('ToolInput') +/** Names the tool a tool-scoped mode toggle targets, so a write to a stale position is refused. */ +function buildToolInputRef( + subblockId: string, + toolIndex: number, + tool: StoredTool +): ToolInputRef | undefined { + const identity = getToolInputIdentity(tool) + return identity ? { subblockId, toolIndex, identity } : undefined +} + const ADVANCED_MCP_SERVER_TOOL_SCHEMA: McpToolSchema = { type: 'object', properties: { @@ -413,21 +426,17 @@ export const ToolInput = memo(function ToolInput({ : [] /** - * Commits a tool list that moves or drops selected tools. Their canonical-mode overrides are - * keyed by position, so when any must move they persist in the same operation as the list. + * Commits a tool list that moves or drops selected tools together with its canonical-mode + * overrides, which are keyed by position. The overrides persist in the same operation even when + * none of this editor's own need to move, so a tool-scoped mode another editor saved a moment + * earlier is replaced instead of left on a position its tool no longer holds. * `positionedTools` is the list holding the kept tool references when `nextTools` clones them. */ const setToolsWithReindexedModes = useCallback( (nextTools: StoredTool[], positionedTools: StoredTool[] = nextTools) => { - const canonicalModes = reindexToolCanonicalModes( - selectedTools, - positionedTools, - canonicalModeOverrides - ) - if (!canonicalModes) { - setStoreValue(nextTools) - return - } + const canonicalModes = + reindexToolCanonicalModes(selectedTools, positionedTools, canonicalModeOverrides) ?? + (filterUndefined(canonicalModeOverrides ?? {}) as Record) collaborativeSetSubblockValueWithCanonicalModes( blockId, subBlockId, @@ -438,7 +447,6 @@ export const ToolInput = memo(function ToolInput({ [ selectedTools, canonicalModeOverrides, - setStoreValue, collaborativeSetSubblockValueWithCanonicalModes, blockId, subBlockId, @@ -1836,7 +1844,8 @@ export const ToolInput = memo(function ToolInput({ collaborativeSetBlockCanonicalMode( blockId, buildAgentToolUsageControlCanonicalKey(toolIndex), - nextMode + nextMode, + buildToolInputRef(subBlockId, toolIndex, tool) ) }} /> @@ -1917,7 +1926,8 @@ export const ToolInput = memo(function ToolInput({ collaborativeSetBlockCanonicalMode( blockId, `${toolIndex}:${canonicalId}`, - nextMode + nextMode, + buildToolInputRef(subBlockId, toolIndex, tool) ) }, } diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 85a63969fe1..50ee49a61d9 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -11,6 +11,8 @@ import { VARIABLE_OPERATIONS, WORKFLOW_OPERATIONS, } from '@sim/realtime-protocol/constants' +import type { ToolInputRef } from '@sim/realtime-protocol/schemas' +import { isToolInputRefCurrent } from '@sim/realtime-protocol/tool-input' import { generateId } from '@sim/utils/id' import type { BlockRetryConfig } from '@sim/workflow-types/workflow' import { filterAcyclicEdges, getWorkflowBlockNameConflict } from '@sim/workflow-types/workflow' @@ -249,6 +251,20 @@ export function useCollaborativeWorkflow() { useWorkflowStore.getState().setBlockRetry(payload.id, payload.retry) break case BLOCK_OPERATIONS.UPDATE_CANONICAL_MODE: + if ( + payload.toolRef && + !isToolInputRefCurrent( + useSubBlockStore.getState().getValue(payload.id, payload.toolRef.subblockId), + payload.canonicalId, + payload.toolRef + ) + ) { + logger.debug('Ignoring a tool mode whose tool this editor has moved or removed', { + blockId: payload.id, + canonicalId: payload.canonicalId, + }) + break + } useWorkflowStore .getState() .setBlockCanonicalMode(payload.id, payload.canonicalId, payload.canonicalMode) @@ -1352,8 +1368,17 @@ export function useCollaborativeWorkflow() { [executeQueuedOperation] ) + /** + * Sets one canonical mode. A tool-scoped key passes `toolRef` so the server can refuse the write + * when another editor has moved or removed that tool since this editor rendered it. + */ const collaborativeSetBlockCanonicalMode = useCallback( - (id: string, canonicalId: string, canonicalMode: 'basic' | 'advanced') => { + ( + id: string, + canonicalId: string, + canonicalMode: 'basic' | 'advanced', + toolRef?: ToolInputRef + ) => { if (isBaselineDiffView) { return } @@ -1370,7 +1395,9 @@ export function useCollaborativeWorkflow() { operation: { operation: BLOCK_OPERATIONS.UPDATE_CANONICAL_MODE, target: OPERATION_TARGETS.BLOCK, - payload: { id, canonicalId, canonicalMode }, + payload: toolRef + ? { id, canonicalId, canonicalMode, toolRef } + : { id, canonicalId, canonicalMode }, }, workflowId: activeWorkflowId, userId: session?.user?.id || 'unknown', diff --git a/apps/sim/lib/workflows/editing/builders.ts b/apps/sim/lib/workflows/editing/builders.ts index 4e9b7078705..b950584b0b2 100644 --- a/apps/sim/lib/workflows/editing/builders.ts +++ b/apps/sim/lib/workflows/editing/builders.ts @@ -19,12 +19,15 @@ import { } from '@/lib/permission-groups/operation-access' import { getEffectiveBlockOutputs } from '@/lib/workflows/blocks/block-outputs' import { isRetryEligibleBlock } from '@/lib/workflows/blocks/retry-eligibility' +import { + applySuppliedToolModes, + createToolCanonicalIndexResolver, +} from '@/lib/workflows/editing/tool-modes' import { buildCanonicalIndex, buildDefaultCanonicalModes, isCanonicalPair, } from '@/lib/workflows/subblocks/visibility' -import { applyAgentToolUsageControlModes } from '@/lib/workflows/tool-input/usage-control' import { hasTriggerCapability } from '@/lib/workflows/triggers/trigger-utils' import { getBlock } from '@/blocks/registry' import type { BlockConfig } from '@/blocks/types' @@ -252,12 +255,17 @@ export function createBlockFromParams( if (validatedInputs) { updateCanonicalModesForInputs(blockState, Object.keys(validatedInputs), blockConfig) - const tools = blockState.subBlocks.tools?.value - if (params.type === 'agent' && Array.isArray(tools)) { - blockState.data = { - ...blockState.data, - canonicalModes: applyAgentToolUsageControlModes(tools, blockState.data?.canonicalModes), - } + const getCanonicalIndex = createToolCanonicalIndexResolver() + for (const subBlock of blockConfig.subBlocks) { + const tools = blockState.subBlocks[subBlock.id]?.value + if (subBlock.type !== 'tool-input' || !Array.isArray(tools)) continue + const result = applySuppliedToolModes({ + tools, + canonicalModes: blockState.data?.canonicalModes, + getCanonicalIndex, + includePermissionMode: params.type === 'agent' && subBlock.id === 'tools', + }) + blockState.data = { ...blockState.data, canonicalModes: result.canonicalModes } } } } diff --git a/apps/sim/lib/workflows/editing/engine.ts b/apps/sim/lib/workflows/editing/engine.ts index 77b14809b35..1145e3936e8 100644 --- a/apps/sim/lib/workflows/editing/engine.ts +++ b/apps/sim/lib/workflows/editing/engine.ts @@ -2,10 +2,13 @@ import { createLogger } from '@sim/logger' import type { BlockState } from '@sim/workflow-types/workflow' import { isEqual } from 'es-toolkit' import type { PermissionGroupConfig } from '@/lib/permission-groups/fields' +import { + applySuppliedToolModes, + createToolCanonicalIndexResolver, +} from '@/lib/workflows/editing/tool-modes' import { coerceObjectArray } from '@/lib/workflows/persistence/remap-internal-ids' import { isValidKey } from '@/lib/workflows/sanitization/key-validation' import { reindexRewrittenToolCanonicalModes } from '@/lib/workflows/subblocks/visibility' -import { applyAgentToolUsageControlModes } from '@/lib/workflows/tool-input/usage-control' import { getBlock } from '@/blocks/registry' import { validateEdges } from '@/stores/workflows/workflow/edge-validation' import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' @@ -266,7 +269,7 @@ export function applyOperationsToWorkflowState( workflowState.blocks as Record | undefined, modifiedState.blocks as Record | undefined ) - applyAgentToolUsageControlModesAfterEdits( + applySuppliedToolModesAfterEdits( workflowState.blocks as Record | undefined, modifiedState.blocks as Record | undefined ) @@ -342,26 +345,40 @@ function reindexToolCanonicalModesAfterEdits( } /** - * An agent tool's Permission Mode follows the fields its rewritten entry supplies. Runs after - * {@link reindexToolCanonicalModesAfterEdits} so each choice lands on its tool's final position - * rather than being moved again as if it were keyed by the original list. + * Tool pair modes (nested basic/advanced params and an agent tool's Permission Mode) follow the + * side each rewritten tool entry supplies, and the side an entry left out is kept from the tool it + * replaced. Runs after {@link reindexToolCanonicalModesAfterEdits} so each choice lands on its + * tool's final position rather than being moved again as if it were keyed by the original list. */ -function applyAgentToolUsageControlModesAfterEdits( +function applySuppliedToolModesAfterEdits( originalBlocks: Record | undefined, blocks: Record | undefined ): void { + const getCanonicalIndex = createToolCanonicalIndexResolver() for (const [blockId, block] of Object.entries(blocks ?? {})) { - if (block.type !== 'agent') continue - const tools = coerceObjectArray(block.subBlocks?.tools?.value).array - if (!tools) continue - const originalTools = coerceObjectArray( - originalBlocks?.[blockId]?.subBlocks?.tools?.value - ).array - if (originalTools && isEqual(originalTools, tools)) continue - - block.data = { - ...block.data, - canonicalModes: applyAgentToolUsageControlModes(tools, block.data?.canonicalModes), + const originalBlock = originalBlocks?.[blockId] + for (const subBlock of getBlock(block.type)?.subBlocks ?? []) { + if (subBlock.type !== 'tool-input') continue + const tools = coerceObjectArray(block.subBlocks?.[subBlock.id]?.value).array + if (!tools) continue + const originalTools = + originalBlock?.type === block.type + ? coerceObjectArray(originalBlock.subBlocks?.[subBlock.id]?.value).array + : null + if (originalTools && isEqual(originalTools, tools)) continue + + const result = applySuppliedToolModes({ + tools, + originalTools: originalTools ?? undefined, + canonicalModes: block.data?.canonicalModes, + getCanonicalIndex, + includePermissionMode: block.type === 'agent' && subBlock.id === 'tools', + }) + if (!isEqual(result.tools, tools)) { + const field = block.subBlocks[subBlock.id] + block.subBlocks[subBlock.id] = { ...field, value: result.tools as typeof field.value } + } + block.data = { ...block.data, canonicalModes: result.canonicalModes } } } } diff --git a/apps/sim/lib/workflows/editing/operations.test.ts b/apps/sim/lib/workflows/editing/operations.test.ts index cffafc03b0c..c03249c995e 100644 --- a/apps/sim/lib/workflows/editing/operations.test.ts +++ b/apps/sim/lib/workflows/editing/operations.test.ts @@ -1382,4 +1382,46 @@ describe('tool canonical-mode reindexing', () => { '0:agentToolUsageControl': 'advanced', }) }) + + it('switches a nested pair to the side an edit supplies and keeps the side it leaves out', () => { + const bothSides = { ...selectorTool, params: { projectId: 'PROJ', manualProjectId: 'MANUAL' } } + const workflow = agentWithTools([bothSides], {}) + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'agent', + params: { inputs: { tools: [{ ...selectorTool, params: { manualProjectId: 'NEW' } }] } }, + }, + ]) + + expect(state.blocks.agent.data.canonicalModes).toEqual({ '0:projectId': 'advanced' }) + expect(state.blocks.agent.subBlocks.tools.value[0].params).toEqual({ + projectId: 'PROJ', + manualProjectId: 'NEW', + }) + }) + + it('keeps the fixed Permission Mode an edit leaves out when it switches to a variable', () => { + const workflow = agentWithTools([{ ...selectorTool, usageControl: 'none' }], {}) + const variableOnly = { + type: 'jira', + operation: 'jira_get_issue', + title: 'Selector', + params: { projectId: 'PROJ' }, + usageControlExpression: '', + } + + const { state } = applyOperationsToWorkflowState(workflow, [ + { operation_type: 'edit', block_id: 'agent', params: { inputs: { tools: [variableOnly] } } }, + ]) + + expect(state.blocks.agent.data.canonicalModes).toEqual({ + '0:agentToolUsageControl': 'advanced', + }) + expect(state.blocks.agent.subBlocks.tools.value[0]).toMatchObject({ + usageControl: 'none', + usageControlExpression: '', + }) + }) }) diff --git a/apps/sim/lib/workflows/editing/tool-modes.test.ts b/apps/sim/lib/workflows/editing/tool-modes.test.ts new file mode 100644 index 00000000000..963492089b8 --- /dev/null +++ b/apps/sim/lib/workflows/editing/tool-modes.test.ts @@ -0,0 +1,146 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { applySuppliedToolModes } from '@/lib/workflows/editing/tool-modes' +import type { CanonicalIndex } from '@/lib/workflows/subblocks/visibility' + +const JIRA_INDEX: CanonicalIndex = { + groupsById: { + projectId: { canonicalId: 'projectId', basicId: 'projectId', advancedIds: ['manualProjectId'] }, + }, + canonicalIdBySubBlockId: { projectId: 'projectId', manualProjectId: 'projectId' }, +} + +const getCanonicalIndex = (toolType: string) => (toolType === 'jira' ? JIRA_INDEX : null) + +function apply( + tools: unknown[], + originalTools: unknown[] | undefined, + canonicalModes: Record = {} +) { + return applySuppliedToolModes({ + tools, + originalTools, + canonicalModes, + getCanonicalIndex, + includePermissionMode: true, + }) +} + +describe('applySuppliedToolModes', () => { + const jiraBoth = { + type: 'jira', + operation: 'read-bulk', + params: { domain: 'example.atlassian.net', projectId: 'SEL', manualProjectId: 'MAN-OLD' }, + usageControl: 'none', + usageControlExpression: '', + } + + it('switches a nested pair to the side supplied and keeps the side left out', () => { + const manualOnly = { + type: 'jira', + operation: 'read-bulk', + params: { domain: 'example.atlassian.net', manualProjectId: 'MAN-NEW' }, + usageControl: 'none', + usageControlExpression: '', + } + + const result = apply([manualOnly], [jiraBoth]) + + expect(result.canonicalModes).toEqual({ '0:projectId': 'advanced' }) + expect(result.tools[0]).toMatchObject({ + params: { domain: 'example.atlassian.net', projectId: 'SEL', manualProjectId: 'MAN-NEW' }, + }) + + const selectorOnly = { ...manualOnly, params: { projectId: 'SEL-NEW' } } + const back = apply([selectorOnly], [result.tools[0]], result.canonicalModes) + + expect(back.canonicalModes).toEqual({ '0:projectId': 'basic' }) + expect(back.tools[0]).toMatchObject({ + params: { projectId: 'SEL-NEW', manualProjectId: 'MAN-NEW' }, + }) + }) + + it('switches Permission Mode to the value supplied and keeps the value left out', () => { + const expressionOnly = { type: 'wikipedia', usageControlExpression: '' } + const fixed = { type: 'wikipedia', usageControl: 'none' } + + const variable = apply([expressionOnly], [fixed]) + expect(variable.canonicalModes).toEqual({ '0:agentToolUsageControl': 'advanced' }) + expect(variable.tools[0]).toEqual({ + type: 'wikipedia', + usageControl: 'none', + usageControlExpression: '', + }) + + const selector = apply( + [{ type: 'wikipedia', usageControl: 'force' }], + variable.tools, + variable.canonicalModes + ) + expect(selector.canonicalModes).toEqual({}) + expect(selector.tools[0]).toEqual({ + type: 'wikipedia', + usageControl: 'force', + usageControlExpression: '', + }) + }) + + it('keeps the current modes when both sides are supplied', () => { + const modes = { '0:projectId': 'advanced', '0:agentToolUsageControl': 'advanced' } as const + const edited = { ...jiraBoth, params: { ...jiraBoth.params, manualProjectId: 'MAN-NEW' } } + + const result = apply([edited], [jiraBoth], modes) + + expect(result.canonicalModes).toEqual(modes) + expect(result.tools[0]).toBe(edited) + }) + + it('returns Permission Mode to the Selector default when neither value is supplied', () => { + const result = apply([{ type: 'wikipedia' }], [{ type: 'wikipedia', usageControl: 'none' }], { + '0:agentToolUsageControl': 'advanced', + }) + + expect(result.canonicalModes).toEqual({}) + expect(result.tools[0]).toEqual({ type: 'wikipedia' }) + }) + + it('leaves a tool identical to its previous entry untouched', () => { + const pendingToggle = { type: 'jira', params: { manualProjectId: 'MAN' }, usageControl: 'auto' } + const modes = { '0:projectId': 'basic', '0:agentToolUsageControl': 'advanced' } as const + + const result = apply([{ ...pendingToggle, isExpanded: true }], [pendingToggle], modes) + + expect(result.canonicalModes).toEqual(modes) + }) + + it('keeps the mode of a pair an edit resends unchanged', () => { + const stored = { type: 'jira', params: { manualProjectId: 'MAN' }, usageControl: 'auto' } + + const result = apply([{ ...stored, usageControl: 'none' }], [stored], { + '0:projectId': 'basic', + }) + + expect(result.canonicalModes).toEqual({ '0:projectId': 'basic' }) + }) + + it('selects modes for new tools without carrying values from other tools', () => { + const result = apply( + [ + jiraBoth, + { + type: 'jira', + operation: 'write', + params: { manualProjectId: 'NEW' }, + usageControl: 'auto', + }, + ], + [jiraBoth] + ) + + expect(result.canonicalModes).toEqual({ '1:projectId': 'advanced' }) + expect(result.tools[1]).toMatchObject({ params: { manualProjectId: 'NEW' } }) + expect(result.tools[1]).not.toHaveProperty('params.projectId') + }) +}) diff --git a/apps/sim/lib/workflows/editing/tool-modes.ts b/apps/sim/lib/workflows/editing/tool-modes.ts new file mode 100644 index 00000000000..db9d315e793 --- /dev/null +++ b/apps/sim/lib/workflows/editing/tool-modes.ts @@ -0,0 +1,139 @@ +import { isRecordLike, omit } from '@sim/utils/object' +import { isEqual } from 'es-toolkit' +import { + buildCanonicalIndex, + type CanonicalIndex, + type CanonicalMode, + type CanonicalModeOverrides, + isCanonicalPair, + matchRewrittenTools, +} from '@/lib/workflows/subblocks/visibility' +import { buildAgentToolUsageControlCanonicalKey } from '@/lib/workflows/tool-input/usage-control' +import { getBlock } from '@/blocks/registry' + +export type ToolCanonicalIndexResolver = (toolType: string) => CanonicalIndex | null + +interface ApplySuppliedToolModesOptions { + tools: readonly unknown[] + /** The list this edit replaced, or `undefined` when every tool is new. */ + originalTools?: readonly unknown[] + /** Overrides already keyed by the tools' final positions. */ + canonicalModes: CanonicalModeOverrides | undefined + getCanonicalIndex: ToolCanonicalIndexResolver + /** Agent tools also carry the Permission Mode pair. */ + includePermissionMode: boolean +} + +interface SuppliedToolModes { + tools: unknown[] + canonicalModes: Record +} + +/** Resolves each tool type's canonical basic/advanced groups once per resolver. */ +export function createToolCanonicalIndexResolver(): ToolCanonicalIndexResolver { + const indexByType = new Map() + return (toolType) => { + if (!indexByType.has(toolType)) { + const subBlocks = getBlock(toolType)?.subBlocks + indexByType.set(toolType, subBlocks ? buildCanonicalIndex(subBlocks) : null) + } + return indexByType.get(toolType) ?? null + } +} + +function withoutExpandedState(tool: Record): Record { + return omit(tool, ['isExpanded']) +} + +/** + * Selects each tool pair's basic/advanced mode from the side a serialized tool list supplies, and + * keeps the inactive side the caller left out. An edit replaces the whole list, so a tool switched + * by sending one side would otherwise lose the other side's stored value. Supplying both sides + * keeps the current mode, because a read-and-resend round trip includes both. Supplying neither + * leaves a nested pair as sent and returns Permission Mode to its documented `auto` Selector + * default. A pair resent with unchanged values keeps its mode, and tools identical to their + * previous entry are left untouched. + */ +export function applySuppliedToolModes({ + tools, + originalTools, + canonicalModes: overrides, + getCanonicalIndex, + includePermissionMode, +}: ApplySuppliedToolModesOptions): SuppliedToolModes { + const canonicalModes: Record = {} + for (const [key, mode] of Object.entries(overrides ?? {})) { + if (mode) canonicalModes[key] = mode + } + + const previousIndexByIndex = new Map() + if (originalTools) { + for (const [previousIndex, index] of matchRewrittenTools(originalTools, tools)) { + previousIndexByIndex.set(index, previousIndex) + } + } + + const nextTools = tools.map((tool, toolIndex) => { + if (!isRecordLike(tool)) return tool + + const previousIndex = previousIndexByIndex.get(toolIndex) + const matched = previousIndex === undefined ? undefined : originalTools?.[previousIndex] + const previous = isRecordLike(matched) ? matched : undefined + if (previous && isEqual(withoutExpandedState(previous), withoutExpandedState(tool))) { + return tool + } + + let next: Record = tool + const canonicalIndex = typeof tool.type === 'string' ? getCanonicalIndex(tool.type) : null + if (canonicalIndex) { + const params = isRecordLike(tool.params) ? tool.params : {} + const previousParams = isRecordLike(previous?.params) ? previous.params : {} + const carriedParams: Record = {} + + for (const group of Object.values(canonicalIndex.groupsById)) { + if (!isCanonicalPair(group)) continue + const basicIds = group.basicId ? [group.basicId] : [] + const basicSupplied = basicIds.some((id) => params[id] !== undefined) + const advancedSupplied = group.advancedIds.some((id) => params[id] !== undefined) + if (basicSupplied === advancedSupplied) continue + const pairIds = [...basicIds, ...group.advancedIds] + if (previous && pairIds.every((id) => isEqual(params[id], previousParams[id]))) continue + + canonicalModes[`${toolIndex}:${group.canonicalId}`] = advancedSupplied + ? 'advanced' + : 'basic' + for (const id of advancedSupplied ? basicIds : group.advancedIds) { + if (params[id] === undefined && previousParams[id] !== undefined) { + carriedParams[id] = previousParams[id] + } + } + } + + if (Object.keys(carriedParams).length > 0) { + next = { ...next, params: { ...params, ...carriedParams } } + } + } + + if (includePermissionMode) { + const key = buildAgentToolUsageControlCanonicalKey(toolIndex) + const hasFixedValue = tool.usageControl !== undefined + const hasExpression = tool.usageControlExpression !== undefined + + if (hasExpression && !hasFixedValue) { + canonicalModes[key] = 'advanced' + if (previous?.usageControl !== undefined) { + next = { ...next, usageControl: previous.usageControl } + } + } else if (!hasExpression) { + delete canonicalModes[key] + if (hasFixedValue && previous?.usageControlExpression !== undefined) { + next = { ...next, usageControlExpression: previous.usageControlExpression } + } + } + } + + return next + }) + + return { tools: nextTools, canonicalModes } +} diff --git a/apps/sim/lib/workflows/subblocks/visibility.ts b/apps/sim/lib/workflows/subblocks/visibility.ts index 15b797fe72f..4f4bce35a1a 100644 --- a/apps/sim/lib/workflows/subblocks/visibility.ts +++ b/apps/sim/lib/workflows/subblocks/visibility.ts @@ -451,20 +451,18 @@ function withoutExpandedState(tool: unknown): unknown { } /** - * {@link reindexCanonicalModesByPosition} for a tool array rewritten from serialized input (the - * workflow edit API and Chat), where object identity is gone. Each rewritten tool claims an - * unclaimed old tool with the same content, then any tool still unmatched claims an unclaimed - * old tool of the same `type`, so a tool whose params were edited in place keeps its modes. Each - * pass tries the same position before any other, so unmoved tools and identical duplicates keep - * their own overrides. A tool left unmatched is new, and an old tool left unmatched was removed. + * Matches a tool array rewritten from serialized input (the workflow edit API and Chat) to the + * list it replaced, where object identity is gone. Each rewritten tool claims an unclaimed old + * tool with the same content, then any tool still unmatched claims an unclaimed old tool of the + * same `type`, so a tool whose params were edited in place still matches. Each pass tries the same + * position before any other, so unmoved tools and identical duplicates keep their own match. A + * tool left unmatched is new, and an old tool left unmatched was removed. Returns each matched old + * index's new index. */ -export function reindexRewrittenToolCanonicalModes( +export function matchRewrittenTools( oldTools: readonly unknown[], - newTools: readonly unknown[], - overrides: CanonicalModeOverrides | undefined -): Record | undefined { - if (!overrides) return undefined - + newTools: readonly unknown[] +): Map { const oldContents = oldTools.map(withoutExpandedState) const newContents = newTools.map(withoutExpandedState) const newIndexByOldIndex = new Map() @@ -490,7 +488,20 @@ export function reindexRewrittenToolCanonicalModes( }) } } - return reindexCanonicalModesByPosition(newIndexByOldIndex, overrides) + return newIndexByOldIndex +} + +/** + * {@link reindexCanonicalModesByPosition} for a tool array rewritten from serialized input, using + * {@link matchRewrittenTools} to find where each tool now sits. + */ +export function reindexRewrittenToolCanonicalModes( + oldTools: readonly unknown[], + newTools: readonly unknown[], + overrides: CanonicalModeOverrides | undefined +): Record | undefined { + if (!overrides) return undefined + return reindexCanonicalModesByPosition(matchRewrittenTools(oldTools, newTools), overrides) } /** diff --git a/apps/sim/lib/workflows/tool-input/usage-control.test.ts b/apps/sim/lib/workflows/tool-input/usage-control.test.ts index e276a0b1cee..13897b563ed 100644 --- a/apps/sim/lib/workflows/tool-input/usage-control.test.ts +++ b/apps/sim/lib/workflows/tool-input/usage-control.test.ts @@ -4,38 +4,11 @@ import { describe, expect, it } from 'vitest' import { parseStoredToolInputValue } from '@/lib/workflows/tool-input/types' import { - applyAgentToolUsageControlModes, buildAgentToolUsageControlCanonicalKey, getAgentToolUsageControlMode, resolveAgentToolUsageControl, } from '@/lib/workflows/tool-input/usage-control' -describe('applyAgentToolUsageControlModes', () => { - it('selects Variable for an expression alone and Selector for a fixed value alone', () => { - expect( - applyAgentToolUsageControlModes( - [{ usageControl: 'force' }, { usageControlExpression: '' }], - { '0:agentToolUsageControl': 'advanced', model: 'advanced' } - ) - ).toEqual({ '1:agentToolUsageControl': 'advanced', model: 'advanced' }) - }) - - it('keeps the current mode of a tool that carries both values', () => { - const modes = { '1:agentToolUsageControl': 'advanced', model: 'advanced' } as const - const repeated = { usageControl: 'force', usageControlExpression: 'none' } - - expect(applyAgentToolUsageControlModes([repeated, repeated], modes)).toEqual(modes) - }) - - it('returns to Selector when a tool omits both values', () => { - expect( - applyAgentToolUsageControlModes([{ type: 'custom-tool' }], { - '0:agentToolUsageControl': 'advanced', - }) - ).toEqual({}) - }) -}) - describe('agent tool usage control', () => { it('defaults legacy tools to Auto in basic mode', () => { expect(resolveAgentToolUsageControl({}, 0)).toBe('auto') diff --git a/apps/sim/lib/workflows/tool-input/usage-control.ts b/apps/sim/lib/workflows/tool-input/usage-control.ts index abb58f3273b..4a7dc8493d4 100644 --- a/apps/sim/lib/workflows/tool-input/usage-control.ts +++ b/apps/sim/lib/workflows/tool-input/usage-control.ts @@ -1,4 +1,3 @@ -import { isRecordLike } from '@sim/utils/object' import type { CanonicalMode, CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility' import type { ToolUsageControl } from '@/providers/types' @@ -23,36 +22,6 @@ export function getAgentToolUsageControlMode( : 'basic' } -/** - * Selects each agent tool's Permission Mode from the fields a serialized tool array supplies: an - * expression alone selects Variable, and a fixed value alone (or neither) selects Selector. A tool - * carrying both keeps its current mode, since a round trip includes the dormant alternative. - * `canonicalModes` must already be keyed by the tools' final positions. - */ -export function applyAgentToolUsageControlModes( - tools: readonly unknown[], - canonicalModes: CanonicalModeOverrides | undefined -): Record { - const result: Record = {} - for (const [key, mode] of Object.entries(canonicalModes ?? {})) { - if (mode) result[key] = mode - } - tools.forEach((tool, index) => { - if (!isRecordLike(tool)) return - const hasFixedValue = tool.usageControl !== undefined - const hasExpression = tool.usageControlExpression !== undefined - if (hasFixedValue && hasExpression) return - - const key = buildAgentToolUsageControlCanonicalKey(index) - if (hasExpression) { - result[key] = 'advanced' - } else { - delete result[key] - } - }) - return result -} - export function resolveAgentToolUsageControl( tool: AgentToolUsageControlInput, toolIndex: number, diff --git a/packages/realtime-protocol/package.json b/packages/realtime-protocol/package.json index 6a230d0adef..3ff05592c0f 100644 --- a/packages/realtime-protocol/package.json +++ b/packages/realtime-protocol/package.json @@ -33,6 +33,10 @@ "./table-presence": { "types": "./src/table-presence.ts", "default": "./src/table-presence.ts" + }, + "./tool-input": { + "types": "./src/tool-input.ts", + "default": "./src/tool-input.ts" } }, "scripts": { diff --git a/packages/realtime-protocol/src/schemas.ts b/packages/realtime-protocol/src/schemas.ts index 5de6d60397a..194a0e9024e 100644 --- a/packages/realtime-protocol/src/schemas.ts +++ b/packages/realtime-protocol/src/schemas.ts @@ -38,6 +38,20 @@ const AutoConnectEdgeSchema = z.object({ const CanonicalModeSchema = z.enum(['basic', 'advanced']) +/** + * Names the `tool-input` entry a tool-scoped canonical mode was set for. Those mode keys are + * positional (`${toolIndex}:${canonicalId}`), so the server applies the write only while that + * position still holds the same tool, and drops it when another editor's reorder or removal + * persisted first. + */ +export const ToolInputRefSchema = z.object({ + subblockId: z.string().min(1), + toolIndex: z.number().int().nonnegative(), + identity: z.record(z.string(), z.string()), +}) + +export type ToolInputRef = z.infer + export const BlockOperationSchema = z.object({ operation: z.enum([ BLOCK_OPERATIONS.UPDATE_POSITION, @@ -70,6 +84,7 @@ export const BlockOperationSchema = z.object({ horizontalHandles: z.boolean().optional(), canonicalId: z.string().optional(), canonicalMode: CanonicalModeSchema.optional(), + toolRef: ToolInputRefSchema.optional(), triggerMode: z.boolean().optional(), height: z.number().optional(), }), diff --git a/packages/realtime-protocol/src/tool-input.test.ts b/packages/realtime-protocol/src/tool-input.test.ts new file mode 100644 index 00000000000..970ee807809 --- /dev/null +++ b/packages/realtime-protocol/src/tool-input.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest' +import { getToolInputIdentity, isSameToolInputIdentity, isToolInputRefCurrent } from './tool-input' + +describe('isToolInputRefCurrent', () => { + const jira = { type: 'jira', operation: 'read-bulk', params: { manualProjectId: 'MAN' } } + const wikipedia = { type: 'wikipedia', operation: 'wikipedia_search' } + const toolRef = { subblockId: 'tools', toolIndex: 1, identity: getToolInputIdentity(jira) ?? {} } + + it('accepts a key while its position holds the same tool', () => { + expect(isToolInputRefCurrent([wikipedia, jira], '1:agentToolUsageControl', toolRef)).toBe(true) + expect(isToolInputRefCurrent(JSON.stringify([wikipedia, jira]), '1:projectId', toolRef)).toBe( + true + ) + }) + + it('rejects a key whose tool moved or was removed', () => { + expect(isToolInputRefCurrent([jira, wikipedia], '1:agentToolUsageControl', toolRef)).toBe(false) + expect(isToolInputRefCurrent([wikipedia], '1:agentToolUsageControl', toolRef)).toBe(false) + expect(isToolInputRefCurrent(undefined, '1:agentToolUsageControl', toolRef)).toBe(false) + }) + + it('rejects a key that points at a different position', () => { + expect(isToolInputRefCurrent([wikipedia, jira], '0:agentToolUsageControl', toolRef)).toBe(false) + }) +}) + +describe('tool-input identity', () => { + const jira = { + type: 'jira', + operation: 'read-bulk', + params: { projectId: 'PROJ', manualProjectId: 'MANUAL' }, + usageControl: 'auto', + isExpanded: true, + } + + it('ignores param values, expansion, and Permission Mode', () => { + const identity = getToolInputIdentity(jira) + + expect(identity).toEqual({ type: 'jira', operation: 'read-bulk' }) + expect( + isSameToolInputIdentity( + { ...jira, params: { manualProjectId: 'OTHER' }, usageControl: 'none', isExpanded: false }, + identity ?? {} + ) + ).toBe(true) + }) + + it('distinguishes tools by operation and referenced tool', () => { + const identity = getToolInputIdentity(jira) ?? {} + + expect(isSameToolInputIdentity({ ...jira, operation: 'write' }, identity)).toBe(false) + expect(isSameToolInputIdentity({ type: 'wikipedia' }, identity)).toBe(false) + expect( + isSameToolInputIdentity( + { type: 'custom-tool', customToolId: 'b' }, + getToolInputIdentity({ type: 'custom-tool', customToolId: 'a' }) ?? {} + ) + ).toBe(false) + }) + + it('distinguishes MCP tools by server and tool name', () => { + const tool = { type: 'mcp', toolId: 'mcp-1', params: { serverId: 's1', toolName: 'search' } } + const identity = getToolInputIdentity(tool) ?? {} + + expect(identity).toEqual({ + type: 'mcp', + toolId: 'mcp-1', + 'params.serverId': 's1', + 'params.toolName': 'search', + }) + expect( + isSameToolInputIdentity({ ...tool, params: { serverId: 's2', toolName: 'search' } }, identity) + ).toBe(false) + }) + + it('treats a missing or malformed entry as a different tool', () => { + const identity = getToolInputIdentity(jira) ?? {} + + expect(getToolInputIdentity(undefined)).toBeNull() + expect(getToolInputIdentity({ operation: 'read-bulk' })).toBeNull() + expect(isSameToolInputIdentity(undefined, identity)).toBe(false) + }) +}) diff --git a/packages/realtime-protocol/src/tool-input.ts b/packages/realtime-protocol/src/tool-input.ts new file mode 100644 index 00000000000..07c25c86d70 --- /dev/null +++ b/packages/realtime-protocol/src/tool-input.ts @@ -0,0 +1,78 @@ +import type { ToolInputRef } from './schemas' + +/** + * Identity of a `tool-input` entry, independent of its param values. Tool-scoped canonical-mode + * keys are positional (`${toolIndex}:${canonicalId}`), so a mode write names the tool it was made + * for and the server compares identities to refuse a write whose tool has since moved or been + * removed by another editor. + */ +export type ToolInputIdentity = Record + +const TOOL_IDENTITY_FIELDS = ['type', 'operation', 'toolId', 'customToolId'] as const + +const TOOL_IDENTITY_PARAM_FIELDS = ['serverId', 'toolName', 'workflowId'] as const + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +/** + * Reads the fields that say which tool an entry is: its type and operation, the custom tool or MCP + * tool it references, and the server or workflow its params bind to. Param values a user edits are + * excluded, so a pending param change never makes a mode write look stale. + */ +export function getToolInputIdentity(tool: unknown): ToolInputIdentity | null { + if (!isRecord(tool) || typeof tool.type !== 'string') return null + + const identity: ToolInputIdentity = {} + for (const field of TOOL_IDENTITY_FIELDS) { + const value = tool[field] + if (typeof value === 'string') identity[field] = value + } + + const params = isRecord(tool.params) ? tool.params : {} + for (const field of TOOL_IDENTITY_PARAM_FIELDS) { + const value = params[field] + if (typeof value === 'string') identity[`params.${field}`] = value + } + + return identity +} + +function readToolInputList(value: unknown): unknown[] | null { + if (Array.isArray(value)) return value + if (typeof value !== 'string') return null + try { + const parsed: unknown = JSON.parse(value) + return Array.isArray(parsed) ? parsed : null + } catch { + return null + } +} + +/** + * Whether a tool-scoped canonical-mode write still names the tool it was made for: its key must + * point at `toolRef.toolIndex`, and that position must still hold a tool with the same identity. + * The server checks the persisted list before applying the write, and each editor checks its own + * list before applying a broadcast one. + */ +export function isToolInputRefCurrent( + tools: unknown, + canonicalId: string, + toolRef: ToolInputRef +): boolean { + if (!canonicalId.startsWith(`${toolRef.toolIndex}:`)) return false + const list = readToolInputList(tools) + return list !== null && isSameToolInputIdentity(list[toolRef.toolIndex], toolRef.identity) +} + +export function isSameToolInputIdentity(tool: unknown, identity: ToolInputIdentity): boolean { + const current = getToolInputIdentity(tool) + if (!current) return false + + const fields = new Set([...Object.keys(current), ...Object.keys(identity)]) + for (const field of fields) { + if (current[field] !== identity[field]) return false + } + return true +}