From 4cb98aea9fbcede8ae414c3f817c7b0490d401f8 Mon Sep 17 00:00:00 2001 From: Robin Braumann <50590409+bjorger@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:02:23 +0000 Subject: [PATCH] feat: Configure workflow tool inputs for Agents (#40285) --- .../agent-json-config.schema.test.ts | 30 +- .../src/agents/agent-json-config.schema.ts | 11 +- .../src/runtime/__tests__/lazy-proxy.test.ts | 39 +- .../src/runtime/lazy-proxy.ts | 7 +- .../workflow-execute-additional-data.test.ts | 10 +- .../__tests__/agent-publish.service.test.ts | 12 +- .../__tests__/workflow-tool-factory.test.ts | 81 +++++ .../agents/agent-definition.service.ts | 3 +- .../agent-runtime-reconstruction.service.ts | 1 + .../__tests__/infer-input-schema.test.ts | 27 +- .../__tests__/workflow-tool-factory.test.ts | 2 + ...kflow-tool-workflow-loader.service.test.ts | 13 +- .../agents/tools/workflow-tool-factory.ts | 84 +++-- .../workflow-tool-workflow-loader.service.ts | 6 +- .../__tests__/ephemeral-node-executor.test.ts | 34 +- .../node-execution/ephemeral-node-executor.ts | 36 ++ .../src/workflow-execute-additional-data.ts | 2 + .../frontend/@n8n/i18n/src/locales/en.json | 20 +- .../src/app/constants/injectionKeys.ts | 5 + .../agents/__tests__/AgentBuilderView.test.ts | 16 +- .../agents/__tests__/AgentModal.test.ts | 57 +++ .../WorkflowToolConfigContent.test.ts | 200 +++++++++- .../agentSectionEditor.utils.test.ts | 18 +- .../features/agents/agents-design-language.md | 15 +- .../agents/components/AgentToolConfigForm.vue | 7 +- .../AgentToolConfigWorkflowContent.vue | 2 + .../components/WorkflowToolConfigContent.vue | 341 +++++++----------- .../agents/components/WorkflowToolInputs.vue | 256 +++++++++++++ .../agents/components/modals/AgentModal.vue | 25 +- .../agents/composables/useAgentToolCatalog.ts | 4 +- .../agents/utils/agentSectionEditor.utils.ts | 10 +- .../agents/views/AgentBuilderView.vue | 10 +- .../components/ExpressionEditModal.vue | 7 +- .../components/ParameterInput.test.ts | 30 +- .../parameters/components/ParameterInput.vue | 31 +- .../components/ParameterInputFull.test.ts | 54 ++- .../components/ParameterInputFull.vue | 99 +++-- .../FromAiOverrideButton.vue | 14 +- .../FromAiOverrideField.vue | 127 ++++--- .../components/ParameterOptions.vue | 6 +- .../ndv/parameters/components/TextEdit.vue | 7 +- .../workflow/test/workflow-expression.test.ts | 16 +- 42 files changed, 1333 insertions(+), 442 deletions(-) create mode 100644 packages/frontend/editor-ui/src/features/agents/components/WorkflowToolInputs.vue diff --git a/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts b/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts index 6c259bd210c..c6eee32461d 100644 --- a/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts +++ b/packages/@n8n/api-types/src/agents/__tests__/agent-json-config.schema.test.ts @@ -659,18 +659,21 @@ describe('formatAgentConfigZodError', () => { }); describe('WorkflowToolJsonConfigSchema — inputs', () => { - it('accepts AI and fixed input bindings on a workflow tool', () => { + it('round-trips workflow input bindings', () => { + const inputs = { + chatId: { mode: 'ai' }, + query: { mode: 'ai', description: 'Input guidance' }, + shoppingListId: { mode: 'fixed', value: 'OySx3QNU0BcCs8yz' }, + label: { mode: 'fixed', value: '={{ literal }}' }, + count: { mode: 'expression', value: '={{ 1 + 2 }}' }, + }; const result = AgentJsonConfigSchema.safeParse({ ...minimalConfig, tools: [ { type: 'workflow', workflow: 'Show Shopping List', - inputs: { - chatId: { mode: 'ai' }, - shoppingListId: { mode: 'fixed', value: 'OySx3QNU0BcCs8yz' }, - botName: { mode: 'fixed', value: 'Jarvis' }, - }, + inputs, }, ], }); @@ -679,16 +682,17 @@ describe('WorkflowToolJsonConfigSchema — inputs', () => { if (result.success) { expect(result.data.tools?.[0]).toMatchObject({ type: 'workflow', - inputs: { - chatId: { mode: 'ai' }, - shoppingListId: { mode: 'fixed', value: 'OySx3QNU0BcCs8yz' }, - botName: { mode: 'fixed', value: 'Jarvis' }, - }, + inputs, }); } }); - it('rejects a fixed binding without a value', () => { + it.each([ + { mode: 'fixed' }, + { mode: 'expression' }, + { mode: 'expression', value: '{{ 1 }}' }, + { mode: 'expression', value: '= ' }, + ])('rejects an invalid binding: %j', (binding) => { const result = AgentJsonConfigSchema.safeParse({ ...minimalConfig, tools: [ @@ -696,7 +700,7 @@ describe('WorkflowToolJsonConfigSchema — inputs', () => { type: 'workflow', workflow: 'Show Shopping List', inputs: { - botName: { mode: 'fixed' }, + botName: binding, }, }, ], diff --git a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts index 2ea545883eb..ccac8d90dec 100644 --- a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts +++ b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts @@ -378,11 +378,12 @@ const CustomToolJsonConfigSchema = z.object({ /** * Per-field binding for a workflow tool's Execute Workflow Trigger inputs. - * - `ai`: field is advertised to the LLM and must be supplied at call time. + * - `ai`: field is advertised to the LLM with optional input guidance. * - `fixed`: field is omitted from the LLM schema and injected at invoke time. + * - `expression`: field is omitted from the LLM schema and resolved at invoke time. */ export const WorkflowToolInputFieldSchema = z.discriminatedUnion('mode', [ - z.object({ mode: z.literal('ai') }).strict(), + z.object({ mode: z.literal('ai'), description: z.string().optional() }).strict(), z .object({ mode: z.literal('fixed'), @@ -390,6 +391,12 @@ export const WorkflowToolInputFieldSchema = z.discriminatedUnion('mode', [ value: jsonValueSchema, }) .strict(), + z + .object({ + mode: z.literal('expression'), + value: z.string().regex(/^=\s*\S/, 'Enter an expression that starts with ='), + }) + .strict(), ]); export const WorkflowToolJsonConfigSchema = z diff --git a/packages/@n8n/expression-runtime/src/runtime/__tests__/lazy-proxy.test.ts b/packages/@n8n/expression-runtime/src/runtime/__tests__/lazy-proxy.test.ts index cb701dcd8b3..ba45e5ef2df 100644 --- a/packages/@n8n/expression-runtime/src/runtime/__tests__/lazy-proxy.test.ts +++ b/packages/@n8n/expression-runtime/src/runtime/__tests__/lazy-proxy.test.ts @@ -68,13 +68,13 @@ describe('createDeepLazyProxy', () => { }); it('toString() returns "[object Object]"', () => { - const p = proxy(); + const p = proxy(undefined, []); expect(p.toString()).toBe('[object Object]'); expect(mocks.getValueAtPath).not.toHaveBeenCalled(); }); it('valueOf() returns the proxy target', () => { - const p = proxy(); + const p = proxy(undefined, []); const val = p.valueOf(); expect(typeof val).toBe('object'); expect(val).not.toBeNull(); @@ -614,20 +614,27 @@ describe('createDeepLazyProxy', () => { expect(Object.keys(p)).toEqual(['name']); }); - it('host data with an own __proto__ key is cached as an own property, not a prototype', () => { - const protoValue = { polluted: true }; - mocks.getValueAtPath.mockImplementation((path: string[]) => { - const key = path[path.length - 1]; - if (key === 'name') return 'Alice'; - if (key === '__proto__') return protoValue; - return undefined; - }); - const p = proxy(['user'], ['name', '__proto__']); - p.name = 'Zed'; // triggers materialization over all keys - expect(Object.keys(p)).toEqual(['name', '__proto__']); - expect(p.polluted).toBeUndefined(); - expect(p['__proto__']).toBe(protoValue); - }); + it.each(['__proto__', 'constructor', 'toString', 'hasOwnProperty', 'valueOf'])( + 'preserves the own field %s before and after a write', + (field) => { + const fieldValue = { label: 'Field value' }; + mocks.getValueAtPath.mockImplementation((path: string[]) => { + const key = path[path.length - 1]; + if (key === 'name') return 'Alice'; + if (key === field) return fieldValue; + return undefined; + }); + const p = proxy(['user'], ['name', field]); + expect(Object.fromEntries(Object.entries(p))).toEqual({ + name: 'Alice', + [field]: fieldValue, + }); + p.name = 'Zed'; // triggers materialization over all keys + expect(Object.keys(p)).toEqual(['name', field]); + expect(p.label).toBeUndefined(); + expect(p[field]).toBe(fieldValue); + }, + ); it('write fetches each uncached key exactly once; later access stays local', () => { const p = writableObjectProxy(); diff --git a/packages/@n8n/expression-runtime/src/runtime/lazy-proxy.ts b/packages/@n8n/expression-runtime/src/runtime/lazy-proxy.ts index 970b1021480..d6a6e4fe160 100644 --- a/packages/@n8n/expression-runtime/src/runtime/lazy-proxy.ts +++ b/packages/@n8n/expression-runtime/src/runtime/lazy-proxy.ts @@ -1,3 +1,5 @@ +import { isKeyOf } from './utils'; + // ============================================================================ // Deep Lazy Proxy System // ============================================================================ @@ -312,7 +314,10 @@ export function createDeepLazyProxy( return arrayLength; } - if (prop in targetObj) { + if ( + isKeyOf(targetObj, prop) || + (prop in targetObj && (isArray || !resolveObjectKeys().includes(prop))) + ) { return targetObj[prop]; } diff --git a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts index 3b2333940b9..d907a8bfaf4 100644 --- a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts +++ b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts @@ -1231,6 +1231,7 @@ describe('WorkflowExecuteAdditionalData', () => { name: 'Test Workflow', active: true, activeVersionId: 'version-456', + versionId: 'draft-version', nodes: currentNodes, connections: currentConnections, activeVersion: mock({ @@ -1249,6 +1250,7 @@ describe('WorkflowExecuteAdditionalData', () => { expect(result.nodes).toEqual(activeVersionNodes); expect(result.connections).toEqual(activeVersionConnections); + expect(result.versionId).toBe('version-456'); expect(workflowRepository.get).toHaveBeenCalledWith( { id: 'workflow-123' }, { relations: ['activeVersion', 'tags'] }, @@ -1381,8 +1383,13 @@ describe('WorkflowExecuteAdditionalData', () => { ]; const mappingConnections = { 'Mapping Node': {} }; workflowPublishedDataService.getPublishedWorkflowData.mockResolvedValue({ - workflow: mock({ id: 'workflow-123', name: 'Test Workflow' }), + workflow: mock({ + id: 'workflow-123', + name: 'Test Workflow', + versionId: 'draft-version', + }), publishedVersion: mock({ + versionId: 'published-version', nodes: mappingNodes, connections: mappingConnections, }), @@ -1397,6 +1404,7 @@ describe('WorkflowExecuteAdditionalData', () => { expect(workflowRepository.get).not.toHaveBeenCalled(); expect(result.nodes).toEqual(mappingNodes); expect(result.connections).toEqual(mappingConnections); + expect(result.versionId).toBe('published-version'); }); it('throws when there is no published version (flag on)', async () => { diff --git a/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts index 9ce1e1bd9a8..80ec176ddf5 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-publish.service.test.ts @@ -682,7 +682,17 @@ describe('AgentPublishService', () => { versionId: 'published-v1', schema: { ...schema, - tools: [{ type: 'custom', id: 'tool', enabled: false, requireApproval: true }], + tools: [ + { type: 'custom', id: 'tool', enabled: false, requireApproval: true }, + { + type: 'workflow', + workflow: 'Test workflow', + inputs: { + constructor: { mode: 'ai' as const, description: 'Use the label.' }, + hasOwnProperty: { mode: 'fixed' as const, value: '=literal' }, + }, + }, + ], skills: [{ type: 'skill', id: 'skill', enabled: false }], subAgents: { agents: [{ agentId: 'agent-2', enabled: false, useWhen: 'Review notes' }] }, tasks: [{ type: 'task', id: 'task-1', enabled: false }], diff --git a/packages/cli/src/modules/agents/__tests__/workflow-tool-factory.test.ts b/packages/cli/src/modules/agents/__tests__/workflow-tool-factory.test.ts index 222d0e0669d..342a4138471 100644 --- a/packages/cli/src/modules/agents/__tests__/workflow-tool-factory.test.ts +++ b/packages/cli/src/modules/agents/__tests__/workflow-tool-factory.test.ts @@ -6,6 +6,7 @@ import { mock } from 'vitest-mock-extended'; import type { ActiveExecutions } from '@/active-executions'; import { ExecutionPersistence } from '@/executions/execution-persistence'; import type { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks'; +import type { EphemeralNodeExecutor } from '@/node-execution/ephemeral-node-executor'; import type { WorkflowRunner } from '@/workflow-runner'; import { @@ -85,6 +86,7 @@ function makeContext(foundWorkflow: WorkflowEntity | null): WorkflowToolContext return { workflowLoader, + executor: mock(), workflowRunner, subworkflowPolicyChecker: mock(), activeExecutions, @@ -316,6 +318,85 @@ describe('resolveWorkflowTool() — metadata attachment', () => { expect.anything(), ); }); + + it('resolves configured inputs for new calls and preserves them on continuation', async () => { + const trigger = makeExecuteWorkflowTriggerNode({ + parameters: { + inputSource: 'jsonExample', + jsonExample: JSON.stringify({ sides: 6, count: 1, label: '' }), + }, + }); + const context = makeContext(makeWorkflow({}, trigger)); + const executor = mock(); + executor.evaluateExpressions + .mockResolvedValueOnce({ count: 2 }) + .mockResolvedValueOnce({ count: 3 }) + .mockResolvedValueOnce({ count: 'invalid' }) + .mockRejectedValueOnce(new Error('Cannot resolve input "count"')); + context.executor = executor; + const run = vi.fn().mockResolvedValue('exec-1'); + context.workflowRunner.run = run; + context.activeExecutions.has = vi.fn().mockReturnValue(false); + Container.set(ExecutionPersistence, { + findSingleExecution: vi.fn().mockResolvedValue({ + status: 'success', + data: { resultData: { runData: {} } }, + }), + } as unknown as ExecutionPersistence); + + const tool = await resolveWorkflowTool( + { + type: 'workflow', + workflow: 'My Test Workflow', + inputs: { + count: { mode: 'expression', value: '={{ $json.sides / 3 }}' }, + hasOwnProperty: { mode: 'expression' as const, value: '={{ $json.removed }}' }, + label: { mode: 'fixed', value: '={{ literal }}' }, + }, + }, + context, + ); + + for (const [sides, count] of [ + [6, 2], + [9, 3], + ]) { + await tool.handler?.({ sides, count: 99, label: 'model value' }, {}); + expect(run).toHaveBeenLastCalledWith( + expect.objectContaining({ + pinData: { [TRIGGER_NAME]: [{ json: { sides, count, label: '={{ literal }}' } }] }, + }), + undefined, + undefined, + undefined, + expect.anything(), + ); + } + await expect(tool.handler?.({ sides: 6 }, {})).rejects.toThrow('count'); + await expect(tool.handler?.({ sides: 6 }, {})).rejects.toThrow('Cannot resolve input "count"'); + expect(run).toHaveBeenCalledTimes(2); + expect(executor.evaluateExpressions).toHaveBeenLastCalledWith( + expect.objectContaining({ projectId: 'project-1' }), + { count: '={{ $json.sides / 3 }}' }, + [{ json: { sides: 6 } }], + ); + + await expect( + tool.handler?.( + {}, + { + continuation: { executionId: 'exec-1' }, + suspend: vi.fn(), + resumeData: undefined, + }, + ), + ).resolves.toMatchObject({ + executionId: 'exec-1', + status: 'success', + }); + expect(executor.evaluateExpressions).toHaveBeenCalledTimes(4); + expect(run).toHaveBeenCalledTimes(2); + }); }); describe('workflow tool compatibility', () => { diff --git a/packages/cli/src/modules/agents/agent-definition.service.ts b/packages/cli/src/modules/agents/agent-definition.service.ts index da3e1a94db2..64e3f3304b9 100644 --- a/packages/cli/src/modules/agents/agent-definition.service.ts +++ b/packages/cli/src/modules/agents/agent-definition.service.ts @@ -53,7 +53,8 @@ export class AgentDefinitionService { ctx: OperationContext = {}, ): Promise { return await this.transactionRunner.run(ctx, async (txCtx) => { - agent.schema = deepCopy(definition.schema); + // Preserve workflow input names in the JSON configuration. + agent.schema = structuredClone(definition.schema); agent.tools = deepCopy(definition.tools); agent.skills = deepCopy(definition.skills); if (agent.schema) agent.name = agent.schema.name; diff --git a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts index 80e29f62253..825a83f79fd 100644 --- a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts +++ b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts @@ -836,6 +836,7 @@ export class AgentRuntimeReconstructionService { ); const context = { workflowLoader: Container.get(WorkflowToolWorkflowLoader), + executor: this.ephemeralNodeExecutor, workflowRunner: await getWorkflowRunner(), subworkflowPolicyChecker: Container.get(SubworkflowPolicyChecker), activeExecutions: this.activeExecutions, diff --git a/packages/cli/src/modules/agents/tools/__tests__/infer-input-schema.test.ts b/packages/cli/src/modules/agents/tools/__tests__/infer-input-schema.test.ts index e2b64f78a65..9cd179e2388 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/infer-input-schema.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/infer-input-schema.test.ts @@ -1,12 +1,13 @@ import type { INode } from 'n8n-workflow'; import { z } from 'zod'; +import { zodSchemaToJsonSchema } from '@n8n/ai-utilities/json-schema'; import { getFixedWorkflowToolInputs, inferInputSchema, listWorkflowInputFields, mergeWorkflowToolInput, - omitFixedFieldsFromSchema, + buildWorkflowToolInputSchema, } from '../workflow-tool-factory'; function makeExecuteWorkflowTrigger(parameters: INode['parameters']): INode { @@ -142,43 +143,55 @@ describe('inferInputSchema — executeWorkflow', () => { }); }); -describe('fixed workflow tool inputs', () => { - it('extracts fixed values and omits them from the LLM schema', () => { +describe('configured workflow tool inputs', () => { + it('advertises AI inputs and merges configured values', () => { const schema = z.object({ chatId: z.string().optional(), shoppingListId: z.string().optional(), botName: z.string().optional(), + count: z.number().optional(), }); const inputs = { shoppingListId: { mode: 'fixed' as const, value: 'OySx3QNU0BcCs8yz' }, botName: { mode: 'fixed' as const, value: 'Jarvis' }, - chatId: { mode: 'ai' as const }, + ['__proto__']: { mode: 'fixed' as const, value: { source: 'configured' } }, + chatId: { mode: 'ai' as const, description: 'Input guidance' }, + count: { mode: 'expression' as const, value: '={{ 2 }}' }, + constructor: { mode: 'ai' as const, description: 'Removed input' }, }; - expect(getFixedWorkflowToolInputs(inputs)).toEqual({ + const fixedInputs = getFixedWorkflowToolInputs(inputs); + expect(fixedInputs).toEqual({ shoppingListId: 'OySx3QNU0BcCs8yz', botName: 'Jarvis', + ['__proto__']: { source: 'configured' }, }); + expect(Object.getPrototypeOf(fixedInputs)).toBe(Object.prototype); - const llmSchema = omitFixedFieldsFromSchema(schema, inputs); + const llmSchema = buildWorkflowToolInputSchema(schema, inputs); expect(Object.keys(llmSchema.shape)).toEqual(['chatId']); + expect(zodSchemaToJsonSchema(llmSchema)).toMatchObject({ + properties: { chatId: { description: inputs.chatId.description } }, + }); expect( mergeWorkflowToolInput( llmSchema.parse({ chatId: '42' }) as Record, inputs, schema, + { count: 2 }, ), ).toEqual({ chatId: '42', shoppingListId: 'OySx3QNU0BcCs8yz', botName: 'Jarvis', + count: 2, }); }); it('preserves catchall when omitting fixed keys from an open schema', () => { const schema = z.object({}).catchall(z.unknown()); const inputs = { pinned: { mode: 'fixed' as const, value: 'yes' } }; - const llmSchema = omitFixedFieldsFromSchema(schema, inputs); + const llmSchema = buildWorkflowToolInputSchema(schema, inputs); expect(llmSchema.parse({ anything: 1 })).toEqual({ anything: 1 }); expect(mergeWorkflowToolInput({ anything: 1 }, inputs, schema)).toEqual({ anything: 1, diff --git a/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-factory.test.ts b/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-factory.test.ts index b51c9869912..f435c3fa899 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-factory.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-factory.test.ts @@ -17,6 +17,7 @@ import { mock } from 'vitest-mock-extended'; import type { ActiveExecutions } from '@/active-executions'; import { ExecutionPersistence } from '@/executions/execution-persistence'; import type { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks'; +import type { EphemeralNodeExecutor } from '@/node-execution/ephemeral-node-executor'; import { WebhookResponseRelay } from '@/scaling/webhook-response-relay'; import type { WorkflowRunner } from '@/workflow-runner'; @@ -55,6 +56,7 @@ const workflow = { function buildContext(run: ReturnType, extras: Partial = {}) { return { workflowLoader: {} as never, + executor: mock(), workflowRunner: { run } as unknown as WorkflowRunner, subworkflowPolicyChecker: mock(), activeExecutions: { has: vi.fn().mockReturnValue(false) } as unknown as ActiveExecutions, diff --git a/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-workflow-loader.service.test.ts b/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-workflow-loader.service.test.ts index 91f0f7dd4b4..d1f5262df7c 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-workflow-loader.service.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/workflow-tool-workflow-loader.service.test.ts @@ -68,7 +68,9 @@ describe('WorkflowToolWorkflowLoader', () => { const { service, workflowRepository } = makeService(); workflowRepository.findOneByAgentToolReference.mockResolvedValue( makeWorkflow({ + versionId: 'draft-version', activeVersion: { + versionId: 'published-version', nodes: [{ id: 'published-node' }], connections: { Published: {} }, }, @@ -85,6 +87,7 @@ describe('WorkflowToolWorkflowLoader', () => { { withActiveVersion: true }, ); expect(workflow).toMatchObject({ + versionId: 'published-version', nodes: [{ id: 'published-node' }], connections: { Published: {} }, }); @@ -113,10 +116,13 @@ describe('WorkflowToolWorkflowLoader', () => { const { service, workflowRepository, workflowPublishedDataService } = makeService({ useWorkflowPublicationService: true, }); - workflowRepository.findOneByAgentToolReference.mockResolvedValue(makeWorkflow()); + workflowRepository.findOneByAgentToolReference.mockResolvedValue( + makeWorkflow({ versionId: 'draft-version' }), + ); workflowPublishedDataService.getPublishedWorkflowData.mockResolvedValue({ workflow: makeWorkflow(), publishedVersion: { + versionId: 'published-version', nodes: [{ id: 'published-node' }], connections: { Published: {} }, }, @@ -129,6 +135,9 @@ describe('WorkflowToolWorkflowLoader', () => { expect(workflowPublishedDataService.getPublishedWorkflowData).toHaveBeenCalledWith( 'workflow-1', ); - expect(workflow).toMatchObject({ nodes: [{ id: 'published-node' }] }); + expect(workflow).toMatchObject({ + versionId: 'published-version', + nodes: [{ id: 'published-node' }], + }); }); }); diff --git a/packages/cli/src/modules/agents/tools/workflow-tool-factory.ts b/packages/cli/src/modules/agents/tools/workflow-tool-factory.ts index c726e8bf1a6..80e26174214 100644 --- a/packages/cli/src/modules/agents/tools/workflow-tool-factory.ts +++ b/packages/cli/src/modules/agents/tools/workflow-tool-factory.ts @@ -31,6 +31,7 @@ import { createRunExecutionData, isTerminalExecutionStatus, EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, + WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE, TimeoutExecutionCancelledError, isIndefiniteWait, } from 'n8n-workflow'; @@ -40,6 +41,7 @@ import { z } from 'zod'; import type { ActiveExecutions } from '@/active-executions'; import { ExecutionPersistence } from '@/executions/execution-persistence'; import type { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks'; +import type { EphemeralNodeExecutor } from '@/node-execution/ephemeral-node-executor'; import { WebhookResponseRelay } from '@/scaling/webhook-response-relay'; import type { WorkflowRunner } from '@/workflow-runner'; @@ -151,6 +153,7 @@ export type WorkflowToolExecutionMode = Extract { if (!inputs) return {}; - const fixed: Record = {}; + const fixed: Array<[string, unknown]> = []; for (const [name, binding] of Object.entries(inputs)) { if (binding.mode === 'fixed') { - fixed[name] = binding.value; + fixed.push([name, binding.value]); } } - return fixed; + return Object.fromEntries(fixed); } -/** - * Drop fixed-bound keys from the LLM-facing schema so the model is not asked - * for values the user already configured. - */ -export function omitFixedFieldsFromSchema( +/** Advertise AI inputs and their guidance. Configured values stay out of the model schema. */ +export function buildWorkflowToolInputSchema( schema: z.ZodObject, inputs: WorkflowToolInputsConfig | undefined, ): z.ZodObject { - const fixedKeys = Object.keys(getFixedWorkflowToolInputs(inputs)); - if (fixedKeys.length === 0) return schema; + if (!inputs) return schema; const shape = { ...schema.shape }; - for (const key of fixedKeys) { - delete shape[key]; + for (const [name, binding] of Object.entries(inputs)) { + if (binding.mode !== 'ai') { + delete shape[name]; + } else if (Object.hasOwn(shape, name) && binding.description?.trim()) { + shape[name] = shape[name].describe(binding.description.trim()); + } } const catchall = schema._def.catchall as z.ZodTypeAny | undefined; @@ -394,25 +397,57 @@ export function omitFixedFieldsFromSchema( return z.object(shape); } -/** - * Merge LLM-supplied args with fixed tool-config values (fixed wins), then - * parse the result against the full declared schema so fixed values are - * coerced to their field's declared type (e.g. numeric IDs stored as strings - * are converted to numbers for number fields). Without this, fixed bindings - * bypass the schema and reach the sub-workflow with the wrong runtime type. - */ +/** Merge configured values over model arguments. Validate and coerce them to the trigger types. */ export function mergeWorkflowToolInput( llmInput: Record, inputs: WorkflowToolInputsConfig | undefined, fullSchema: z.ZodObject, + expressionValues: Record = {}, ): Record { const merged: Record = { ...llmInput, ...getFixedWorkflowToolInputs(inputs), + ...expressionValues, }; return fullSchema.parse(merged); } +async function resolveWorkflowToolInput( + input: Record, + inputs: WorkflowToolInputsConfig | undefined, + fullSchema: z.ZodObject, + context: WorkflowToolContext, + toolName: string, +): Promise> { + const modelInput: IDataObject = buildWorkflowToolInputSchema(fullSchema, inputs).parse(input); + const expressions: Record = {}; + const acceptsExtraInputs = !(fullSchema._def.catchall instanceof z.ZodNever); + for (const [name, binding] of Object.entries(inputs ?? {})) { + if ( + binding.mode === 'expression' && + (Object.hasOwn(fullSchema.shape, name) || acceptsExtraInputs) + ) { + expressions[name] = binding.value; + } + } + if (Object.keys(expressions).length === 0) { + return mergeWorkflowToolInput(modelInput, inputs, fullSchema); + } + + const resolved = await context.executor.evaluateExpressions( + { + projectId: context.projectId, + nodeType: WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE, + nodeTypeVersion: 2.2, + nodeParameters: {}, + nodeName: toolName, + }, + expressions, + [{ json: modelInput }], + ); + return mergeWorkflowToolInput(modelInput, inputs, fullSchema, resolved); +} + // --------------------------------------------------------------------------- // 4. executeWorkflow // --------------------------------------------------------------------------- @@ -1006,7 +1041,7 @@ async function buildWorkflowTool( return assembleWorkflowTool(descriptor, context, { reference: { workflowId: workflow.id, workflowName: workflow.name }, - inputSchema: omitFixedFieldsFromSchema(fullInputSchema, descriptor.inputs), + inputSchema: buildWorkflowToolInputSchema(fullInputSchema, descriptor.inputs), triggerType, }); } @@ -1055,7 +1090,7 @@ function assembleWorkflowTool( // A continuation means this workflow already ran on without us, so re-running // it would be wrong. Skip the reload too: a settled run's output should still // come back even if the workflow was archived since. The input is not parsed - // again either — fixed tool inputs were merged in on the original call. + // again either — configured inputs were resolved on the original call. const pending = WAIT_CONTINUATION_SCHEMA.safeParse(ctx.continuation); let current: Awaited> | undefined; let result: WorkflowToolExecutionResult; @@ -1065,11 +1100,12 @@ function assembleWorkflowTool( } else { current = await loadCurrentWorkflow(context, reference); const currentFullSchema = inferInputSchema(current.triggerNode, current.triggerType); - const currentSchema = omitFixedFieldsFromSchema(currentFullSchema, toolInputs); - const parsedInput = mergeWorkflowToolInput( - currentSchema.parse(input), + const parsedInput = await resolveWorkflowToolInput( + input, toolInputs, currentFullSchema, + context, + toolName, ); result = await executeWorkflow( current.workflow, diff --git a/packages/cli/src/modules/agents/tools/workflow-tool-workflow-loader.service.ts b/packages/cli/src/modules/agents/tools/workflow-tool-workflow-loader.service.ts index ccd858b3e87..46782533ca0 100644 --- a/packages/cli/src/modules/agents/tools/workflow-tool-workflow-loader.service.ts +++ b/packages/cli/src/modules/agents/tools/workflow-tool-workflow-loader.service.ts @@ -49,7 +49,11 @@ export class WorkflowToolWorkflowLoader { `Workflow "${workflow.name}" is not published. Publish it so the published agent can use it.`, ); } - Object.assign(workflow, { nodes: published.nodes, connections: published.connections }); + Object.assign(workflow, { + versionId: published.versionId, + nodes: published.nodes, + connections: published.connections, + }); } return Object.assign(workflow, { pinData: undefined }); diff --git a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts index 1fe4503d59b..49007e45bc8 100644 --- a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts +++ b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts @@ -947,7 +947,7 @@ describe('EphemeralNodeExecutor', () => { }); }); - it('resolves expressions when invoking a supplyData tool with the VM engine', async () => { + it('resolves expressions in standalone tool contexts with the VM engine', async () => { await Expression.initExpressionEngine({ engine: 'vm', bridgeTimeout: 1000, @@ -993,6 +993,38 @@ describe('EphemeralNodeExecutor', () => { status: 'success', data: [{ json: { response: 2 } }], }); + + mockGetBase.mockResolvedValue({ variables: { increment: '3' } }); + const tool = { + nodeType: '@n8n/n8n-nodes-langchain.toolWorkflow', + nodeTypeVersion: 2.2, + nodeParameters: {}, + projectId: 'p-1', + }; + await expect( + executor.evaluateExpressions( + tool, + { + count: '={{ $json.count + Number($vars.increment) }}', + options: '={{ { enabled: true, items: [1, 2] } }}', + date: '={{ $now.toISODate() }}', + }, + [{ json: { count: 4 } }], + ), + ).resolves.toEqual({ + count: 7, + options: { enabled: true, items: [1, 2] }, + date: expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/), + }); + await expect( + executor.evaluateExpressions(tool, { broken: '={{ 1 + }}' }, []), + ).rejects.toThrow('Cannot resolve input "broken"'); + // A failed expression must release the single isolate for the next call. + await expect( + executor.evaluateExpressions(tool, { count: '={{ $json.count }}' }, [ + { json: { count: 9 } }, + ]), + ).resolves.toEqual({ count: 9 }); } finally { await Expression.disposeExpressionEngine(); } diff --git a/packages/cli/src/node-execution/ephemeral-node-executor.ts b/packages/cli/src/node-execution/ephemeral-node-executor.ts index f61c7380f33..654bb63fa56 100644 --- a/packages/cli/src/node-execution/ephemeral-node-executor.ts +++ b/packages/cli/src/node-execution/ephemeral-node-executor.ts @@ -384,6 +384,42 @@ export class EphemeralNodeExecutor { } } + /** Resolve input expressions in the same context as a standalone node tool. */ + async evaluateExpressions( + tool: EphemeralWorkflowToolLike, + expressions: Record, + inputItems: INodeExecutionData[], + ): Promise> { + const parts = await this.buildEphemeralContextParts(tool, inputItems); + const context = new ExecuteContext( + parts.workflow, + parts.node, + parts.additionalData, + parts.mode, + parts.runExecutionData, + 0, + inputItems, + parts.inputData, + parts.executeData, + [], + ); + + return await withExpressionIsolate(parts.workflow, async () => { + const resolved: Record = {}; + for (const [name, expression] of Object.entries(expressions)) { + try { + // evaluateExpression adds the leading '=' itself. + resolved[name] = context.evaluateExpression(expression.slice(1)); + } catch (error) { + throw new UserError(`Cannot resolve input "${name}": ${getErrorMessage(error)}`, { + cause: error, + }); + } + } + return resolved; + }); + } + async executeInline(request: InlineNodeExecutionRequest): Promise { // Validation failures (unknown node type, trigger nodes, blacklisted // operations like send-and-wait) need to surface to the agent as a diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index ba0cd307c77..3c9578206dd 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -212,6 +212,7 @@ export async function getPublishedWorkflowData( } return { ...publishedData.workflow, + versionId: publishedData.publishedVersion.versionId, nodes: publishedData.publishedVersion.nodes, connections: publishedData.publishedVersion.connections, }; @@ -232,6 +233,7 @@ export async function getPublishedWorkflowData( if (workflowData && 'activeVersion' in workflowData && workflowData.activeVersion) { return { ...workflowData, + versionId: workflowData.activeVersion.versionId, nodes: workflowData.activeVersion.nodes, connections: workflowData.activeVersion.connections, }; diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index d060877f44b..96a3ba95fba 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -2797,7 +2797,8 @@ "parameterInputList.callout.dismiss.confirm.confirmButtonText": "Confirm", "parameterInputList.callout.dismiss.confirm.cancelButtonText": "Cancel", "parameterInputList.autoRequiresChatTriggerDescription": "Available when connected to a Chat Trigger node.", - "parameterOverride.overridePanelText": "Defined automatically by the model", + "parameterOverride.overridePanelText": "Defined by agent", + "parameterOverride.editValue": "Edit value", "parameterOverride.applyOverrideButtonTooltip": "Let the model define this parameter", "parameterOverride.descriptionTooltip": "Explain to the LLM how it should generate this value, a good, specific description would allow LLMs to produce expected results much more often", "personalizationModal.businessOwner": "Business Owner", @@ -8303,15 +8304,14 @@ "agents.toolConfig.workflow.target.idNotFound": "No workflow with this ID can be used here. It has to be in this project and start with a '{trigger}' trigger.", "agents.toolConfig.workflow.target.open": "Open workflow in a new tab", "agents.toolConfig.workflow.inputs": "Workflow inputs", - "agents.toolConfig.workflow.inputs.hint": "The agent fills in these fields when it calls the workflow. Set a fixed value for any field the agent should not choose.", - "agents.toolConfig.workflow.inputs.mode.ai": "Determined by agent", - "agents.toolConfig.workflow.inputs.mode.fixed": "Fixed value", - "agents.toolConfig.workflow.inputs.value.placeholder": "Enter a fixed value", - "agents.toolConfig.workflow.inputs.value.placeholder.string": "Enter text", - "agents.toolConfig.workflow.inputs.value.placeholder.number": "Enter a number", - "agents.toolConfig.workflow.inputs.value.placeholder.boolean": "true or false", - "agents.toolConfig.workflow.inputs.value.placeholder.array": "Enter a JSON array, e.g. [\"a\", \"b\"]", - "agents.toolConfig.workflow.inputs.value.placeholder.object": "Enter a JSON object, e.g. {'{'}\"key\": \"value\"{'}'}", + "agents.toolConfig.workflow.advanced": "Advanced", + "agents.toolConfig.workflow.inputs.description": "Description (optional)", + "agents.toolConfig.workflow.inputs.description.placeholder": "Tell the agent what value to use", + "agents.toolConfig.workflow.inputs.empty": "This workflow has no defined inputs. Define inputs in its trigger to configure them here.", + "agents.toolConfig.workflow.inputs.loading": "Loading workflow inputs…", + "agents.toolConfig.workflow.inputs.loadError": "Couldn't load workflow inputs. Try again.", + "agents.toolConfig.workflow.inputs.expression.empty": "Enter an expression", + "agents.toolConfig.workflow.inputs.value.invalid": "Enter a valid {type} value", "agents.toolConfig.validation.fixFields": "Fix the highlighted fields before saving.", "agents.toolConfig.approval.label": "Require approval", "agents.toolConfig.approval.hint": "Pause before this tool runs until a human approves it.", diff --git a/packages/frontend/editor-ui/src/app/constants/injectionKeys.ts b/packages/frontend/editor-ui/src/app/constants/injectionKeys.ts index 88984f667d1..73f3cd841f4 100644 --- a/packages/frontend/editor-ui/src/app/constants/injectionKeys.ts +++ b/packages/frontend/editor-ui/src/app/constants/injectionKeys.ts @@ -34,6 +34,11 @@ export const ExecutionDataStoreKey: InjectionKey< export const CanvasRenderDataKey: InjectionKey> = Symbol('CanvasRenderData'); /** Keep setup hints compact while retaining their full text on the field. */ export const CompactParameterHintsKey: InjectionKey = Symbol('CompactParameterHints'); +/** Lets modal hosts coordinate focus and portals for expanded parameter editors. */ +export const ParameterInputModalContextKey: InjectionKey<{ + openDialogs: Ref>; + appendTo: string; +}> = Symbol('ParameterInputModalContext'); /** * Opts resource-locator dropdowns into teleporting to ``. Defaults to * `false` (stay in the local stacking context, e.g. inside the NDV dialog). diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts index 83c0ace663a..722887d1037 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentBuilderView.test.ts @@ -2586,6 +2586,15 @@ describe('AgentBuilderView — preview routing', { timeout: 60_000 }, () => { }); it('refreshes runnable state from the backend after saving manual config edits', async () => { + const workflowTool: AgentJsonToolRef = { + type: 'workflow', + workflow: 'Test workflow', + inputs: { + constructor: { mode: 'ai' as const, description: 'Use the label.' }, + hasOwnProperty: { mode: 'fixed' as const, value: '=literal' }, + }, + }; + intendedConfig!.tools = [workflowTool]; getAgentMock .mockResolvedValueOnce(makeAgentResponse({ isRunnable: false })) .mockResolvedValueOnce(makeAgentResponse({ isRunnable: true, versionId: 'v2' })); @@ -2608,7 +2617,12 @@ describe('AgentBuilderView — preview routing', { timeout: 60_000 }, () => { await vm.flushAutosave(); await nextTick(); - expect(updateConfigMock).toHaveBeenCalled(); + expect(updateConfigMock).toHaveBeenCalledWith( + 'p1', + 'a1', + expect.objectContaining({ tools: [workflowTool] }), + 'hash-1', + ); expect(getAgentMock).toHaveBeenLastCalledWith({ baseUrl: 'http://localhost:5678' }, 'p1', 'a1'); expect(vm.isBuilt).toBe(true); }); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentModal.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentModal.test.ts index 212e385cfa4..e70e83bce4f 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentModal.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentModal.test.ts @@ -1,5 +1,8 @@ +import { N8nDialog } from '@n8n/design-system'; import { flushPromises, mount } from '@vue/test-utils'; import { describe, expect, it, vi } from 'vitest'; +import { defineComponent, h, inject, Teleport } from 'vue'; +import { ParameterInputModalContextKey } from '@/app/constants/injectionKeys'; import AgentModal from '../components/modals/AgentModal.vue'; import AgentModalMultiStep from '../components/modals/AgentModalMultiStep.vue'; @@ -185,6 +188,60 @@ describe('AgentModal', () => { expect(wrapper.emitted('update:open')).toBeUndefined(); }); + it('releases and restores focus and dismissal for a nested parameter editor', async () => { + const editor = defineComponent({ + setup() { + const context = inject(ParameterInputModalContextKey)!; + return () => [ + h( + 'button', + { + 'data-test-id': 'open-editor', + onClick: () => context.openDialogs.value.add('expression'), + }, + 'Open editor', + ), + h( + Teleport, + { to: document.body }, + h('input', { + 'data-test-id': 'parameter-editor-input', + onKeydown: () => context.openDialogs.value.delete('expression'), + }), + ), + ]; + }, + }); + const wrapper = mount(AgentModal, { + props: { open: true, title: 'Tool settings' }, + slots: { default: () => h(editor) }, + }); + const dialog = wrapper.get('[role="dialog"]'); + await wrapper.get('[data-test-id="open-editor"]').trigger('click'); + expect(dialog.attributes('data-trap-focus')).toBe('false'); + expect(dialog.attributes('data-disable-outside-pointer-events')).toBe('false'); + await wrapper.get('[data-test-id="dialog-escape"]').trigger('click'); + await wrapper.get('[data-test-id="dialog-dismiss"]').trigger('click'); + expect(dialog.attributes('data-escape-prevented')).toBe('true'); + expect(wrapper.emitted('update:open')).toBeUndefined(); + + const escape = new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true, + }); + document.querySelector('[data-test-id="parameter-editor-input"]')!.dispatchEvent(escape); + wrapper.getComponent(N8nDialog).vm.$emit('escapeKeyDown', escape); + await flushPromises(); + expect(escape.defaultPrevented).toBe(true); + expect(wrapper.emitted('update:open')).toBeUndefined(); + expect(dialog.attributes('data-trap-focus')).toBe('true'); + expect(dialog.attributes('data-disable-outside-pointer-events')).toBe('true'); + await wrapper.get('[data-test-id="dialog-dismiss"]').trigger('click'); + expect(wrapper.emitted('update:open')).toEqual([[false]]); + wrapper.unmount(); + }); + it('emits Back and Close when the modal is idle', async () => { const wrapper = mountModal({ showBack: true }); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/WorkflowToolConfigContent.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/WorkflowToolConfigContent.test.ts index b1f22a14289..39aa75ccdda 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/WorkflowToolConfigContent.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/WorkflowToolConfigContent.test.ts @@ -2,17 +2,22 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createTestingPinia } from '@pinia/testing'; import { createComponentRenderer } from '@/__tests__/render'; import { mockedStore } from '@/__tests__/utils'; -import { fireEvent, waitFor } from '@testing-library/vue'; +import { fireEvent, waitFor, within } from '@testing-library/vue'; +import { defineComponent, h, nextTick, ref, shallowRef } from 'vue'; +import { EditorView } from '@codemirror/view'; import { SUPPORTED_WORKFLOW_TOOL_TRIGGERS } from '@n8n/api-types'; import { useWorkflowsListStore } from '@/app/stores/workflowsList.store'; import WorkflowToolConfigContent from '../components/WorkflowToolConfigContent.vue'; +import WorkflowToolInputs from '../components/WorkflowToolInputs.vue'; +import AgentToolConfigForm from '../components/AgentToolConfigForm.vue'; import type { WorkflowToolRef } from '../types'; vi.mock('vue-router', async (importOriginal) => { const actual = (await importOriginal()) as Record; return { ...actual, + useRoute: () => ({ params: {}, query: {}, meta: {} }), useRouter: () => ({ resolve: ({ params }: { params: { workflowId: string } }) => ({ href: `/workflow/${params.workflowId}`, @@ -21,11 +26,11 @@ vi.mock('vue-router', async (importOriginal) => { }; }); -vi.mock('@n8n/i18n', () => { - const i18n = { - baseText: (key: string) => key, - }; - return { useI18n: () => i18n, i18n, i18nInstance: { install: vi.fn() } }; +vi.mock('@n8n/i18n', async (importOriginal) => { + const actual = await importOriginal(); + const i18n = Object.create(actual.i18n) as typeof actual.i18n; + i18n.baseText = (key: string) => key; + return { ...actual, useI18n: () => i18n, i18n }; }); // Element Plus' select is unusable in jsdom, so only the select/option pair is @@ -86,6 +91,27 @@ function workflow( const renderComponent = createComponentRenderer(WorkflowToolConfigContent); +function renderForm(initialRef: WorkflowToolRef) { + const form = ref>(); + const onConfirm = vi.fn(); + const render = createComponentRenderer( + defineComponent({ + setup: () => () => + h('div', [ + h(AgentToolConfigForm, { ref: form, data: { toolRef: initialRef, onConfirm } }), + h('button', { onClick: () => form.value?.confirm() }, 'Save'), + ]), + }), + ); + return { ...render(), onConfirm, form }; +} + +function editExpression(field: HTMLElement, expression: string) { + const content = field.querySelector('.cm-content')!; + const editor = EditorView.findFromDOM(content)!; + editor.dispatch({ changes: { from: 0, to: editor.state.doc.length, insert: expression } }); +} + function createRef(overrides: Partial = {}): WorkflowToolRef { return { type: 'workflow', @@ -382,9 +408,11 @@ describe('WorkflowToolConfigContent', () => { ]), ]); - const { findByTestId } = renderComponent({ props: { initialRef: createRef() } }); + const { findByTestId, getByTestId } = renderComponent({ props: { initialRef: createRef() } }); - expect(await findByTestId('agent-workflow-tool-inputs')).toBeTruthy(); + expect(await findByTestId('agent-workflow-tool-inputs')).not.toBeVisible(); + await fireEvent.click(getByTestId('agent-workflow-tool-advanced')); + expect(getByTestId('agent-workflow-tool-inputs')).toBeVisible(); expect(await findByTestId('agent-workflow-tool-input-chatId')).toBeTruthy(); expect(await findByTestId('agent-workflow-tool-input-botName')).toBeTruthy(); }); @@ -405,12 +433,160 @@ describe('WorkflowToolConfigContent', () => { }, }); - await findByTestId('agent-workflow-tool-input-value-botName'); - expect( - (getByTestId('agent-workflow-tool-input-value-botName') as HTMLInputElement).value, - ).toBe('Jarvis'); + const field = await findByTestId('agent-workflow-tool-input-botName'); + await fireEvent.click(getByTestId('agent-workflow-tool-advanced')); + expect(within(field).getByRole('textbox')).toHaveValue('Jarvis'); }); + it.each(['constructor', 'toString', 'hasOwnProperty', '__proto__'])( + 'configures and reopens an input named %s', + async (name) => { + const inputs = shallowRef>({}); + const inputType = ref('string'); + const render = createComponentRenderer( + defineComponent({ + setup: () => () => + h(WorkflowToolInputs, { + fields: [{ name, type: inputType.value }], + toolName: 'Notify Sales', + submitted: false, + modelValue: inputs.value, + 'onUpdate:modelValue': (updated: NonNullable) => { + inputs.value = updated; + }, + }), + }), + ); + const view = render(); + const field = view.getByTestId(`agent-workflow-tool-input-${name}`); + await fireEvent.click( + within(field).getByRole('button', { + name: 'parameterOverride.editValue', + }), + ); + expect(within(field).getByRole('textbox')).toHaveValue(''); + expect(inputs.value).toEqual({ [name]: { mode: 'fixed', value: '' } }); + await fireEvent.update(within(field).getByRole('textbox'), 'Literal value'); + expect(inputs.value).toEqual({ [name]: { mode: 'fixed', value: 'Literal value' } }); + + view.unmount(); + const reopened = render(); + const savedField = reopened.getByTestId(`agent-workflow-tool-input-${name}`); + await waitFor(() => + expect(within(savedField).getByRole('textbox')).toHaveValue('Literal value'), + ); + await fireEvent.click( + within(savedField).getByRole('button', { + name: 'parameterOverride.applyOverrideButtonTooltip', + }), + ); + expect(inputs.value).toEqual({}); + await fireEvent.update(within(savedField).getByRole('textbox'), 'Input guidance'); + expect(inputs.value).toEqual({ [name]: { mode: 'ai', description: 'Input guidance' } }); + + inputType.value = 'object'; + await nextTick(); + await fireEvent.click( + within(savedField).getByRole('button', { name: 'parameterOverride.editValue' }), + ); + const rawValue = '{ "answer": 42 }'; + editExpression(savedField, rawValue); + await waitFor(() => + expect(inputs.value).toEqual({ [name]: { mode: 'fixed', value: { answer: 42 } } }), + ); + await nextTick(); + const content = savedField.querySelector('.cm-content')!; + expect(EditorView.findFromDOM(content)?.state.doc.toString()).toBe(rawValue); + }, + ); + + it('saves and reopens all binding modes without changing the original reference', async () => { + setProjectWorkflows([ + workflowWithInputs('Notify Sales', [ + { name: 'query', type: 'string' }, + { name: 'label', type: 'string' }, + { name: 'count', type: 'number' }, + ]), + ]); + const initialRef = createRef(); + const view = renderForm(initialRef); + await view.findByTestId('agent-workflow-tool-inputs'); + await fireEvent.click(view.getByTestId('agent-workflow-tool-advanced')); + await fireEvent.update( + within(view.getByTestId('agent-workflow-tool-input-query')).getByRole('textbox'), + 'Input guidance', + ); + const label = within(view.getByTestId('agent-workflow-tool-input-label')); + await fireEvent.click(label.getByRole('button', { name: 'parameterOverride.editValue' })); + await fireEvent.update(label.getByRole('textbox'), '={{ literal }}'); + const count = view.getByTestId('agent-workflow-tool-input-count'); + await fireEvent.click( + within(count).getByRole('button', { name: 'parameterOverride.editValue' }), + ); + await fireEvent.click(within(count).getByText('parameterInput.expression')); + editExpression(count, '{{ $json.query.length }}'); + await waitFor(() => expect(view.form.value?.canSave).toBe(true)); + await fireEvent.click(view.getByTestId('agent-workflow-tool-advanced')); + await fireEvent.click(view.getByRole('button', { name: 'Save' })); + expect(view.onConfirm).toHaveBeenCalledWith( + expect.objectContaining({ + inputs: { + query: { mode: 'ai', description: 'Input guidance' }, + label: { mode: 'fixed', value: '={{ literal }}' }, + count: { mode: 'expression', value: '={{ $json.query.length }}' }, + }, + }), + ); + expect(initialRef.inputs).toBeUndefined(); + const saved = view.onConfirm.mock.calls[0][0] as WorkflowToolRef; + view.unmount(); + const reopened = renderForm(saved); + await reopened.findByTestId('agent-workflow-tool-inputs'); + await fireEvent.click(reopened.getByTestId('agent-workflow-tool-advanced')); + expect( + within(reopened.getByTestId('agent-workflow-tool-input-query')).getByRole('textbox'), + ).toHaveValue('Input guidance'); + expect( + within(reopened.getByTestId('agent-workflow-tool-input-label')).getByRole('textbox'), + ).toHaveValue('={{ literal }}'); + expect(reopened.getByTestId('agent-workflow-tool-input-count')).toHaveTextContent( + '$json.query.length', + ); + }); + + it.each([ + { mode: 'expression', value: '=' }, + { mode: 'fixed', value: 'not a number' }, + ] as const)( + 'validates $mode inputs on Save and preserves them when loading fails', + async (binding) => { + const inputs: WorkflowToolRef['inputs'] = { count: binding }; + setProjectWorkflows([ + workflowWithInputs('Notify Sales', [{ name: 'count', type: 'number' }]), + ]); + const view = renderForm(createRef({ inputs })); + await view.findByTestId('agent-workflow-tool-inputs'); + for (let attempt = 0; attempt < 2; attempt++) { + await fireEvent.click(view.getByRole('button', { name: 'Save' })); + expect(view.onConfirm).not.toHaveBeenCalled(); + expect(view.getByTestId('agent-workflow-tool-advanced')).toHaveAttribute( + 'aria-expanded', + 'true', + ); + await fireEvent.click(view.getByTestId('agent-workflow-tool-advanced')); + } + view.unmount(); + mockedStore(useWorkflowsListStore).searchWorkflows = vi + .fn() + .mockRejectedValue(new Error('offline')); + const failedLoad = renderForm(createRef({ inputs })); + await fireEvent.click(failedLoad.getByTestId('agent-workflow-tool-advanced')); + await failedLoad.findByText('agents.toolConfig.workflow.inputs.loadError'); + await fireEvent.click(failedLoad.getByRole('button', { name: 'Save' })); + expect(failedLoad.onConfirm).toHaveBeenCalledWith(expect.objectContaining({ inputs })); + }, + ); + it('does not render input bindings for passthrough workflows', async () => { setProjectWorkflows([ { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/agentSectionEditor.utils.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/agentSectionEditor.utils.test.ts index 11bec577ff6..f61571cb813 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/agentSectionEditor.utils.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/agentSectionEditor.utils.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import { reactive } from 'vue'; import { configToDoc, getSlice, @@ -45,10 +46,21 @@ describe('agentSectionEditor.utils', () => { }); describe('setSlice', () => { - it('does not mutate the input', () => { - const next = setSlice(cfg, 'name', 'y'); + it('does not mutate the input or remove workflow input bindings', () => { + const tool: NonNullable[number] = { + type: 'workflow', + workflow: 'Test workflow', + inputs: { + constructor: { mode: 'ai' as const, description: 'Use the label.' }, + hasOwnProperty: { mode: 'fixed' as const, value: '=literal' }, + }, + }; + const source = reactive({ ...cfg, tools: [tool] }); + const next = setSlice(source, 'name', 'y'); expect(next.name).toBe('y'); - expect(cfg.name).toBe('x'); + expect(source.name).toBe('x'); + expect(next.tools).toEqual([tool]); + expect(next.tools?.[0]).not.toBe(tool); }); it('replaces nested object values', () => { const next = setSlice(cfg, 'config.toolCallConcurrency', 8); diff --git a/packages/frontend/editor-ui/src/features/agents/agents-design-language.md b/packages/frontend/editor-ui/src/features/agents/agents-design-language.md index ae077d3018e..fb461016202 100644 --- a/packages/frontend/editor-ui/src/features/agents/agents-design-language.md +++ b/packages/frontend/editor-ui/src/features/agents/agents-design-language.md @@ -109,8 +109,10 @@ Do not add top padding or a top margin to a modal's first content wrapper. The shell supplies that space. Use the flush body only for a full-bleed workspace. The workspace must then own all of its edge spacing. -When a nested credential dialog is open, release the parent focus trap and -block parent dismissal. The nested dialog owns Escape until it closes. +When a nested credential or parameter editor dialog is open, release the parent +focus trap and block parent dismissal. The nested dialog owns Escape until it +closes. Render expanded parameter editors in the body portal above the Agent +modal. ### Title contract @@ -228,6 +230,15 @@ Add a section when an Agent-specific pattern applies to two or more Agent surfaces. Keep implementation details with the owning pattern. Do not duplicate global Design System guidance. +## Model-defined inputs + +Use `ParameterInputFull` for workflow inputs and node tool parameters. +It owns the field label, Fixed/Expression controls, AI button, model chip, +and hover and focus behavior. Use its controlled input mode for workflow +bindings. Keep binding conversion and optional input guidance in the caller. +Hide the Edit value action in read-only forms. Keep input issues visible +beside the model label. + ## Tool approvals Use `N8nApprovalCard` for Preview tool approvals, including background child diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigForm.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigForm.vue index f4ecb41582d..cbac3920f89 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigForm.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigForm.vue @@ -97,7 +97,7 @@ const nodeContentRef = ref | nul const mcpContentRef = ref | null>(null); const workflowContentRef = ref | null>(null); const isValid = ref(false); -const submitted = ref(false); +const submitCount = ref(0); const approvalRequired = ref(false); const mcpApproval = ref(); const mcpApprovalValid = ref(true); @@ -235,7 +235,7 @@ function withMcpApproval(server: AgentJsonMcpServerConfig): AgentJsonMcpServerCo } function confirm(): boolean { - submitted.value = true; + submitCount.value += 1; if (!canSave.value) return false; if (isCustomTool.value) { @@ -300,7 +300,7 @@ defineExpose({ canSave, confirm, remove, changeTitle, credentialModalOpen, title