Skip to content

Commit d09db7b

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(agent): keep tool configuration expansion local
1 parent 568a539 commit d09db7b

2 files changed

Lines changed: 469 additions & 116 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
/** @vitest-environment jsdom */
2+
import { act, type ComponentProps, type ReactNode, useState } from 'react'
3+
import { createRoot, type Root } from 'react-dom/client'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { create } from 'zustand'
6+
import type { StoredTool } from '@/lib/workflows/tool-input/types'
7+
import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
8+
9+
const fixture = vi.hoisted(() => ({
10+
tools: [] as StoredTool[],
11+
target: null as { subBlockId: string; valuePath: (string | number)[] } | null,
12+
write: vi.fn(),
13+
canonical: vi.fn(),
14+
blocks: [] as BlockConfig[],
15+
replace: (_tools: StoredTool[]) => {},
16+
}))
17+
18+
vi.mock('next/navigation', () => ({
19+
useParams: () => ({ workspaceId: 'workspace-1', workflowId: 'workflow-1' }),
20+
}))
21+
vi.mock('@/blocks', () => ({
22+
getAllBlocks: () => fixture.blocks,
23+
getBlock: (type: string) => fixture.blocks.find((block) => block.type === type),
24+
}))
25+
vi.mock('@/blocks/custom/client-overlay', () => ({ useCustomBlockOverlayVersion: () => 0 }))
26+
vi.mock('@/blocks/utils', () => ({ BUILT_IN_TOOL_TYPES: new Set() }))
27+
vi.mock('@/tools/metadata', () => ({ getToolMetadata: () => undefined }))
28+
vi.mock('@/providers/models', () => ({ supportsForcedToolUse: () => false }))
29+
vi.mock('@/providers/utils', () => ({
30+
getProviderFromModel: () => '',
31+
supportsToolUsageControl: () => false,
32+
}))
33+
vi.mock('@/hooks/use-collaborative-workflow', () => ({
34+
useCollaborativeWorkflow: () => ({
35+
collaborativeSetBlockCanonicalMode: fixture.canonical,
36+
collaborativeSetBlockCanonicalModes: fixture.canonical,
37+
}),
38+
}))
39+
vi.mock('@/hooks/use-permission-config', () => ({
40+
usePermissionConfig: () => ({
41+
filterBlocks: (blocks: BlockConfig[]) => blocks,
42+
config: {},
43+
isLoading: false,
44+
}),
45+
}))
46+
vi.mock('@/hooks/use-operation-access', () => ({
47+
useOperationAccess: () => ({ getDeniedOperations: () => new Set() }),
48+
}))
49+
vi.mock('@/hooks/queries/custom-tools', () => ({ useCustomTools: () => ({ data: [] }) }))
50+
vi.mock('@/hooks/queries/credentials', () => ({ useWorkspaceCredential: () => ({}) }))
51+
vi.mock('@/hooks/queries/workflows', () => ({ useWorkflows: () => ({ data: [] }) }))
52+
vi.mock('@/hooks/queries/deployments', () => ({
53+
useDeploymentInfo: () => ({ data: { isDeployed: true } }),
54+
useDeployWorkflow: () => ({}),
55+
}))
56+
vi.mock('@/hooks/mcp/use-mcp-tools', () => ({
57+
useMcpTools: () => ({ mcpTools: [], isLoading: false }),
58+
}))
59+
vi.mock('@/hooks/queries/mcp', () => ({
60+
useMcpToolServers: () => ({ data: [] }),
61+
useStoredMcpTools: () => ({ data: [] }),
62+
useAllowedMcpDomains: () => ({}),
63+
useCreateMcpServer: () => ({}),
64+
useForceRefreshMcpTools: () => ({ mutate: () => {} }),
65+
}))
66+
vi.mock('@/hooks/mcp/use-mcp-oauth-popup', () => ({ useMcpOauthPopup: () => ({}) }))
67+
vi.mock('@/hooks/use-available-env-vars', () => ({ useAvailableEnvVarKeys: () => [] }))
68+
vi.mock('@/hooks/use-settings-navigation', () => ({
69+
useSettingsNavigation: () => ({ navigateToSettings: () => {} }),
70+
}))
71+
vi.mock('@/app/workspace/[workspaceId]/providers/workspace-permissions-provider', () => ({
72+
useUserPermissionsContext: () => ({ canAdmin: false }),
73+
}))
74+
vi.mock(
75+
'@/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal',
76+
() => ({ McpServerFormModal: () => null })
77+
)
78+
vi.mock(
79+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/custom-tool-modal/custom-tool-modal',
80+
() => ({ CustomToolModal: () => null })
81+
)
82+
vi.mock(
83+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text',
84+
() => ({ formatDisplayText: (text: string) => text })
85+
)
86+
vi.mock(
87+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight',
88+
() => ({ getActiveWorkflowSearchHighlight: () => null })
89+
)
90+
vi.mock(
91+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider',
92+
() => ({
93+
useActiveSearchTarget: () => fixture.target,
94+
ActiveSearchTargetProvider: ({ children }: { children: ReactNode }) => children,
95+
})
96+
)
97+
vi.mock(
98+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-value',
99+
() => ({
100+
useSubBlockValue: () => {
101+
const [value, setValue] = useState(fixture.tools)
102+
fixture.replace = setValue
103+
return [
104+
value,
105+
(tools: StoredTool[]) => {
106+
fixture.write(tools)
107+
fixture.tools = structuredClone(tools)
108+
setValue(fixture.tools)
109+
},
110+
]
111+
},
112+
})
113+
)
114+
vi.mock('@/stores/workflows/workflow/store', () => ({
115+
useWorkflowStore: (selector: (state: unknown) => unknown) =>
116+
selector({ blocks: { 'block-1': { type: 'agent' } } }),
117+
}))
118+
vi.mock('@/stores/workflows/registry/store', () => ({
119+
useWorkflowRegistry: Object.assign(() => 'workflow-1', {
120+
getState: () => ({ activeWorkflowId: 'workflow-1' }),
121+
}),
122+
}))
123+
vi.mock('@/stores/workflows/subblock/store', () => ({
124+
useSubBlockStore: create<{
125+
workflowValues: Record<string, Record<string, Record<string, unknown>>>
126+
getValue: (block: string, field: string) => unknown
127+
setValue: (block: string, field: string, value: unknown) => void
128+
}>((set, get) => ({
129+
workflowValues: {},
130+
getValue: (block, field) => get().workflowValues['workflow-1']?.[block]?.[field],
131+
setValue: (block, field, value) =>
132+
set((state) => ({
133+
workflowValues: {
134+
'workflow-1': {
135+
...state.workflowValues['workflow-1'],
136+
[block]: { ...state.workflowValues['workflow-1']?.[block], [field]: value },
137+
},
138+
},
139+
})),
140+
})),
141+
}))
142+
143+
/** Keep the real tool-param bridge; substitute only the heavy leaf field renderer. */
144+
vi.mock(
145+
'@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/sub-block',
146+
() => ({
147+
SubBlock: ({
148+
blockId,
149+
config,
150+
disabled,
151+
}: {
152+
blockId: string
153+
config: SubBlockConfig
154+
disabled: boolean
155+
}) => {
156+
const value = useSubBlockStore((state) => state.getValue(blockId, config.id))
157+
return (
158+
<input
159+
aria-label={config.title}
160+
disabled={disabled}
161+
value={String(value ?? '')}
162+
onChange={(event) =>
163+
useSubBlockStore.getState().setValue(blockId, config.id, event.target.value)
164+
}
165+
/>
166+
)
167+
},
168+
})
169+
)
170+
171+
vi.mock('@sim/emcn', () => ({
172+
Button: ({ variant: _variant, ...props }: ComponentProps<'button'> & { variant?: string }) => (
173+
<button {...props} />
174+
),
175+
Badge: ({ children }: { children: ReactNode }) => <span>{children}</span>,
176+
cn: (...values: unknown[]) => values.filter(Boolean).join(' '),
177+
Combobox: ({
178+
groups,
179+
disabled,
180+
onOpenChange,
181+
}: {
182+
groups?: { items: { label: string; onSelect?: () => void; disabled?: boolean }[] }[]
183+
disabled?: boolean
184+
onOpenChange?: (open: boolean) => void
185+
}) => (
186+
<div>
187+
<button onClick={() => onOpenChange?.(true)}>Open tools</button>
188+
{groups
189+
?.flatMap((group) => group.items)
190+
.map((item) => (
191+
<button key={item.label} disabled={disabled || item.disabled} onClick={item.onSelect}>
192+
Add {item.label}
193+
</button>
194+
))}
195+
</div>
196+
),
197+
Tooltip: {
198+
Root: ({ children }: { children: ReactNode }) => children,
199+
Trigger: ({ children }: { children: ReactNode }) => children,
200+
Content: () => null,
201+
},
202+
Popover: ({ children }: { children: ReactNode }) => children,
203+
PopoverTrigger: ({ children }: { children: ReactNode }) => children,
204+
PopoverContent: () => null,
205+
PopoverItem: () => null,
206+
}))
207+
208+
import { ToolInput } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input'
209+
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
210+
211+
let container: HTMLDivElement
212+
let root: Root
213+
const tool = (value: string): StoredTool => ({
214+
type: 'mcp',
215+
toolId: `mcp-${value}`,
216+
title: value,
217+
isExpanded: true,
218+
params: { query: value, serverId: 'server-1', toolName: value },
219+
schema: { type: 'object', properties: { query: { type: 'string' } } },
220+
})
221+
const buttons = () => [...container.querySelectorAll<HTMLButtonElement>('button[aria-expanded]')]
222+
const render = (props: Partial<ComponentProps<typeof ToolInput>> = {}) =>
223+
act(() => root.render(<ToolInput blockId='block-1' subBlockId='tools' {...props} />))
224+
const click = (element: HTMLElement) => act(() => element.click())
225+
226+
beforeEach(() => {
227+
vi.clearAllMocks()
228+
fixture.tools = [tool('First'), tool('Second')]
229+
fixture.target = null
230+
fixture.blocks = []
231+
container = document.createElement('div')
232+
document.body.append(container)
233+
root = createRoot(container)
234+
globalThis.IS_REACT_ACT_ENVIRONMENT = true
235+
})
236+
afterEach(() => {
237+
act(() => root.unmount())
238+
container.remove()
239+
})
240+
241+
describe('ToolInput local expansion', () => {
242+
it('ignores persisted expansion and independently toggles without writing workflow data', () => {
243+
render()
244+
expect(buttons().map((button) => button.getAttribute('aria-expanded'))).toEqual([
245+
'false',
246+
'false',
247+
])
248+
click(buttons()[0])
249+
click(buttons()[1])
250+
click(buttons()[0])
251+
expect(buttons().map((button) => button.getAttribute('aria-expanded'))).toEqual([
252+
'false',
253+
'true',
254+
])
255+
expect(fixture.write).not.toHaveBeenCalled()
256+
expect(fixture.canonical).not.toHaveBeenCalled()
257+
})
258+
259+
it('preserves an immediate parameter edit through collapse and real bridge rehydration', () => {
260+
render()
261+
click(buttons()[0])
262+
const input = container.querySelector('input')!
263+
act(() => {
264+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(
265+
input,
266+
'Edited'
267+
)
268+
input.dispatchEvent(new Event('input', { bubbles: true }))
269+
})
270+
expect(fixture.tools[0].params?.query).toBe('Edited')
271+
click(buttons()[0])
272+
click(buttons()[0])
273+
expect(container.querySelector('input')?.value).toBe('Edited')
274+
expect(fixture.write).toHaveBeenCalledTimes(1)
275+
})
276+
277+
it('retains the correct duplicate instance when an earlier row is removed', () => {
278+
fixture.tools[1].toolId = fixture.tools[0].toolId
279+
render()
280+
click(buttons()[1])
281+
click(container.querySelector<HTMLButtonElement>('button[aria-label="Remove tool"]')!)
282+
expect(buttons()[0].getAttribute('aria-expanded')).toBe('true')
283+
expect(container.querySelector('input')?.value).toBe('Second')
284+
})
285+
286+
it('follows the dragged instance when rows reorder', () => {
287+
render()
288+
click(buttons()[0])
289+
const rows = container.querySelectorAll<HTMLElement>('[draggable="true"]')
290+
const transfer = { setData: () => {}, effectAllowed: '', dropEffect: '' }
291+
act(() => {
292+
const event = new Event('dragstart', { bubbles: true })
293+
Object.assign(event, { dataTransfer: transfer })
294+
rows[1].dispatchEvent(event)
295+
})
296+
act(() => {
297+
const event = new Event('drop', { bubbles: true, cancelable: true })
298+
Object.assign(event, { dataTransfer: transfer })
299+
rows[0].dispatchEvent(event)
300+
})
301+
expect(buttons().map((button) => button.getAttribute('aria-expanded'))).toEqual([
302+
'false',
303+
'true',
304+
])
305+
expect(container.querySelector('input')?.value).toBe('First')
306+
})
307+
308+
it('resets after an external replacement or a different editor scope', () => {
309+
render()
310+
click(buttons()[0])
311+
act(() => fixture.replace([tool('Replacement')]))
312+
expect(buttons()[0].getAttribute('aria-expanded')).toBe('false')
313+
click(buttons()[0])
314+
render({ blockId: 'block-2' })
315+
expect(buttons()[0].getAttribute('aria-expanded')).toBe('false')
316+
})
317+
318+
it('opens search matches without changing the local choice or stored tools', () => {
319+
fixture.target = { subBlockId: 'tools', valuePath: [1, 'params', 'query'] }
320+
render()
321+
expect(buttons().map((button) => button.getAttribute('aria-expanded'))).toEqual([
322+
'false',
323+
'true',
324+
])
325+
fixture.target = null
326+
render({ disabled: true })
327+
expect(buttons()[1].getAttribute('aria-expanded')).toBe('false')
328+
expect(fixture.write).not.toHaveBeenCalled()
329+
})
330+
331+
it('permits locked inspection and opt-in preview expansion without editable fields', () => {
332+
render({ disabled: true })
333+
click(buttons()[0])
334+
expect(container.querySelector('input')?.disabled).toBe(true)
335+
render({ isPreview: true, previewValue: fixture.tools })
336+
expect(buttons()[0].disabled).toBe(true)
337+
render({ isPreview: true, previewValue: fixture.tools, allowExpandInPreview: true })
338+
click(buttons()[0])
339+
expect(container.querySelector('input')?.disabled).toBe(true)
340+
expect(fixture.write).not.toHaveBeenCalled()
341+
})
342+
343+
it('opens only the newly added configurable tool without persisting expansion flags', () => {
344+
render()
345+
click(buttons()[0])
346+
click(
347+
[...container.querySelectorAll('button')].find(
348+
(button) => button.textContent === 'Open tools'
349+
)!
350+
)
351+
click(
352+
[...container.querySelectorAll('button')].find(
353+
(button) => button.textContent === 'Add MCP Server (Advanced)'
354+
)!
355+
)
356+
expect(buttons().map((button) => button.getAttribute('aria-expanded'))).toEqual([
357+
'true',
358+
'false',
359+
'true',
360+
])
361+
expect(fixture.tools[2]).not.toHaveProperty('isExpanded')
362+
expect(fixture.tools[0].isExpanded).toBe(true)
363+
})
364+
})

0 commit comments

Comments
 (0)