From b754e2ca833dc7aaccd036de157ed2e876c42fcf Mon Sep 17 00:00:00 2001 From: Farzan Date: Sat, 17 Oct 2026 20:59:12 +0530 Subject: [PATCH] fix(agents): Address review feedback on variables and test values Co-Authored-By: Claude Sonnet 5.5 --- .../__tests__/node-tool-variables.test.ts | 64 ++++++++++++++++ .../src/agents/node-tool-variables.ts | 70 ++++++++++++++--- .../__tests__/agent-chat.controller.test.ts | 4 +- ...ent-message-queue-consumer.service.test.ts | 6 +- ...ent-preview-test-variables.service.test.ts | 35 +++++++++ .../agent-preview-test-variables.service.ts | 19 ++++- .../tools/__tests__/node-tool-factory.test.ts | 75 +++++++++++++++++++ .../modules/agents/tools/node-tool-factory.ts | 43 +++++++---- .../frontend/@n8n/i18n/src/locales/en.json | 2 +- .../__tests__/AgentTestVariablesPanel.test.ts | 21 ++++++ .../__tests__/AgentVariablesOverview.test.ts | 9 +++ .../NodeToolTrustedMetadataSection.test.ts | 9 +++ .../__tests__/useAgentChatStream.test.ts | 42 +++++++++++ .../components/AgentTestVariablesPanel.vue | 7 +- .../components/AgentVariablesOverview.vue | 8 +- .../components/NodeToolTrustedMetadata.vue | 13 +++- .../agents/composables/useAgentChatStream.ts | 3 +- .../composables/useAgentTestVariables.ts | 36 +++++++-- .../AgentVariablesParameter.test.ts | 52 +++++++++++++ .../AgentVariablesParameter.vue | 71 ++++++++++++------ .../__tests__/MessageAnAgent.node.test.ts | 38 ++++++++++ .../nodes-base/nodes/MessageAnAgent/shared.ts | 40 +++++++++- 22 files changed, 597 insertions(+), 70 deletions(-) create mode 100644 packages/cli/src/modules/agents/__tests__/agent-preview-test-variables.service.test.ts diff --git a/packages/@n8n/api-types/src/agents/__tests__/node-tool-variables.test.ts b/packages/@n8n/api-types/src/agents/__tests__/node-tool-variables.test.ts index 53f705c5319..941490ae672 100644 --- a/packages/@n8n/api-types/src/agents/__tests__/node-tool-variables.test.ts +++ b/packages/@n8n/api-types/src/agents/__tests__/node-tool-variables.test.ts @@ -33,6 +33,48 @@ describe('extractNodeToolVariables', () => { it('handles missing parameters', () => { expect(extractNodeToolVariables(undefined)).toEqual([]); }); + + it('ignores $json text inside string literals', () => { + expect( + extractNodeToolVariables({ + a: "={{ 'literal $json.foo' }}", + b: '={{ "also $json.bar" + `tpl $json.baz` }}', + }), + ).toEqual([]); + }); + + it('still reads references in code, bracket names and template placeholders', () => { + expect( + extractNodeToolVariables({ + a: "={{ 'x' + $json.foo + $json['a-b'] }}", + b: '={{ `id ${$json.bar}` }}', + }).sort(), + ).toEqual(['a-b', 'bar', 'foo']); + }); + + it('excludes $fromAI keys with whitespace before the parenthesis or backtick quotes', () => { + expect( + extractNodeToolVariables({ + a: "={{ $fromAI ('spaced', 'd') }}", + b: '={{ $fromAI(`ticked`, `d`) }}', + c: '={{ $json.spaced + $json.ticked + $json.other }}', + }), + ).toEqual(['other']); + }); + + it('supports optional chaining and stops dot names at non-identifier characters', () => { + expect(extractNodeToolVariables({ a: '={{ $json?.opt + $json.a-b }}' }).sort()).toEqual([ + 'a', + 'opt', + ]); + }); + + it('drops names that cannot be persisted as trusted metadata keys', () => { + const long = 'a'.repeat(64); + expect( + extractNodeToolVariables({ a: `={{ $json.${long} + $json.ok_name + $json.constructor }}` }), + ).toEqual(['ok_name']); + }); }); describe('collectAgentVariables', () => { @@ -61,4 +103,26 @@ describe('collectAgentVariables', () => { it('returns an empty list without tools', () => { expect(collectAgentVariables(undefined)).toEqual([]); }); + + it('ignores inherited members when reading defaults', () => { + const result = collectAgentVariables([ + { + type: 'node', + name: 'one', + node: { nodeParameters: { a: '={{ $json.toString + $json.valueOf }}' } }, + }, + { + type: 'node', + name: 'two', + node: { nodeParameters: { a: '={{ $json.toString }}' } }, + trustedMetadata: Object.fromEntries([ + ['toString', { mode: 'fixed' as const, value: 'real' }], + ]), + }, + ]); + expect(result).toEqual([ + { name: 'toString', tools: ['one', 'two'], default: { mode: 'fixed', value: 'real' } }, + { name: 'valueOf', tools: ['one'] }, + ]); + }); }); diff --git a/packages/@n8n/api-types/src/agents/node-tool-variables.ts b/packages/@n8n/api-types/src/agents/node-tool-variables.ts index 1254e588fce..530a5e2b31c 100644 --- a/packages/@n8n/api-types/src/agents/node-tool-variables.ts +++ b/packages/@n8n/api-types/src/agents/node-tool-variables.ts @@ -1,11 +1,53 @@ -import { - TRUSTED_METADATA_RESERVED_KEYS, - type NodeToolTrustedMetadata, -} from './agent-json-config.schema'; +import { TrustedMetadataKeySchema, type NodeToolTrustedMetadata } from './agent-json-config.schema'; -const JSON_DOT_REFERENCE = /\$json\.([A-Za-z_][A-Za-z0-9_-]*)/g; -const JSON_BRACKET_REFERENCE = /\$json\[\s*(['"])([A-Za-z0-9_-]+)\1\s*\]/g; -const FROM_AI_KEY = /\$fromAI\(\s*(['"])([^'"]+)\1/g; +// Hyphenated names are only valid through brackets: `$json.a-b` is `$json.a` minus `b`. +const JSON_DOT_REFERENCE = /\$json\??\.([A-Za-z_][A-Za-z0-9_]*)/g; +const JSON_BRACKET_REFERENCE = /\$json\??\.?\[\s*(['"`])([A-Za-z0-9_-]+)\1\s*\]/g; +// Same call shape the shared `$fromAI` parser accepts: optional whitespace before `(`. +const FROM_AI_KEY = /\$fromAI\s*\(\s*(['"`])((?:(?!\1)[^\\])+)\1/gi; + +/** + * Copy of an expression with the contents of quoted strings and template + * literals blanked out, so text inside a literal is not read as a reference. + * Code inside a template `${...}` placeholder is kept. + */ +function blankStringLiterals(expression: string): string { + let out = ''; + let quote: string | null = null; + let placeholderDepth = 0; + for (let i = 0; i < expression.length; i++) { + const char = expression[i]; + if (quote === null) { + if (char === "'" || char === '"' || char === '`') quote = char; + out += char; + continue; + } + if (char === '\\') { + out += ' '; + i++; + continue; + } + if (quote === '`' && placeholderDepth === 0 && char === '$' && expression[i + 1] === '{') { + placeholderDepth = 1; + out += '${'; + i++; + continue; + } + if (placeholderDepth > 0) { + if (char === '{') placeholderDepth++; + if (char === '}') placeholderDepth--; + out += char; + continue; + } + if (char === quote) { + quote = null; + out += char; + continue; + } + out += ' '; + } + return out; +} function collectStrings(value: unknown, out: string[]): void { if (typeof value === 'string') { @@ -32,13 +74,15 @@ export function extractNodeToolVariables(nodeParameters: unknown): string[] { const names = new Set(); for (const expression of expressions) { for (const match of expression.matchAll(FROM_AI_KEY)) modelKeys.add(match[2]); - for (const match of expression.matchAll(JSON_DOT_REFERENCE)) names.add(match[1]); + for (const match of blankStringLiterals(expression).matchAll(JSON_DOT_REFERENCE)) { + names.add(match[1]); + } for (const match of expression.matchAll(JSON_BRACKET_REFERENCE)) names.add(match[2]); } + // Every returned name must be persistable as a trusted metadata key. return [...names].filter( - (name) => - !modelKeys.has(name) && !(TRUSTED_METADATA_RESERVED_KEYS as readonly string[]).includes(name), + (name) => !modelKeys.has(name) && TrustedMetadataKeySchema.safeParse(name).success, ); } @@ -68,8 +112,10 @@ export function collectAgentVariables(tools: readonly ToolWithNode[] | undefined for (const name of extractNodeToolVariables(tool.node?.nodeParameters)) { const entry = byName.get(name) ?? { name, tools: [] }; if (tool.name) entry.tools.push(tool.name); - const binding = tool.trustedMetadata?.[name]; - if (binding && entry.default === undefined) entry.default = binding; + // Own-property lookup: `toString` and friends are not defaults. + if (entry.default === undefined && Object.hasOwn(tool.trustedMetadata ?? {}, name)) { + entry.default = tool.trustedMetadata?.[name]; + } byName.set(name, entry); } } diff --git a/packages/cli/src/modules/agents/__tests__/agent-chat.controller.test.ts b/packages/cli/src/modules/agents/__tests__/agent-chat.controller.test.ts index 4b7a6b2a773..3aabb0c55ce 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-chat.controller.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-chat.controller.test.ts @@ -93,7 +93,9 @@ function makeController() { mock(), ); - const previewTestVariables = new AgentPreviewTestVariablesService(); + const previewTestVariables = new AgentPreviewTestVariablesService( + mock({ checkpointTtlSeconds: 96 * 3600 }), + ); const controller = new AgentChatController( agentExecutionOrchestratorService, agentTestRunService, diff --git a/packages/cli/src/modules/agents/__tests__/agent-message-queue-consumer.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-message-queue-consumer.service.test.ts index 3d74ea9b468..fcbd72e3ab2 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-message-queue-consumer.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-message-queue-consumer.service.test.ts @@ -1,4 +1,5 @@ import { mockLogger } from '@n8n/backend-test-utils'; +import type { AgentsConfig } from '@n8n/config'; import type { AgentIntegrationConfig } from '@n8n/api-types'; import type { User, UserRepository } from '@n8n/db'; import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise'; @@ -36,7 +37,9 @@ describe('AgentMessageQueueConsumer', () => { const chatExecutions = mock(); const integrations = mock(); const orchestrator = mock(); - const previewTestVariables = new AgentPreviewTestVariablesService(); + const previewTestVariables = new AgentPreviewTestVariablesService( + mock({ checkpointTtlSeconds: 96 * 3600 }), + ); const sender = { send: vi.fn(), close: vi.fn(async () => {}) }; let consumer: AgentMessageQueueConsumer; @@ -209,6 +212,7 @@ describe('AgentMessageQueueConsumer', () => { await vi.waitFor(() => expect(queue.settle).toHaveBeenCalledWith('session', item.admission.executionId), ); + expect(testRuns.executePreparedDraftRun).toHaveBeenCalledTimes(1); expect(testRuns.executePreparedDraftRun.mock.lastCall?.[0]).not.toHaveProperty( 'runtimeMetadata', ); diff --git a/packages/cli/src/modules/agents/__tests__/agent-preview-test-variables.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-preview-test-variables.service.test.ts new file mode 100644 index 00000000000..aabb28b6280 --- /dev/null +++ b/packages/cli/src/modules/agents/__tests__/agent-preview-test-variables.service.test.ts @@ -0,0 +1,35 @@ +import type { AgentsConfig } from '@n8n/config'; +import { mock } from 'vitest-mock-extended'; + +import { AgentPreviewTestVariablesService } from '../agent-preview-test-variables.service'; + +describe('AgentPreviewTestVariablesService', () => { + afterEach(() => vi.useRealTimers()); + + const create = (checkpointTtlSeconds: number) => + new AgentPreviewTestVariablesService(mock({ checkpointTtlSeconds })); + + it('holds values for the checkpoint TTL when it is longer than the floor', () => { + vi.useFakeTimers(); + const service = create(96 * 3600); + service.hold('q', { tenant: 'acme' }); + vi.advanceTimersByTime(95 * 3600 * 1000); + expect(service.take('q')).toEqual({ tenant: 'acme' }); + }); + + it('expires values after the checkpoint TTL', () => { + vi.useFakeTimers(); + const service = create(96 * 3600); + service.hold('q', { tenant: 'acme' }); + vi.advanceTimersByTime(96 * 3600 * 1000 + 1); + expect(service.take('q')).toBeUndefined(); + }); + + it('keeps at least one hour when the checkpoint TTL is shorter', () => { + vi.useFakeTimers(); + const service = create(60); + service.hold('q', { tenant: 'acme' }); + vi.advanceTimersByTime(59 * 60 * 1000); + expect(service.take('q')).toEqual({ tenant: 'acme' }); + }); +}); diff --git a/packages/cli/src/modules/agents/agent-preview-test-variables.service.ts b/packages/cli/src/modules/agents/agent-preview-test-variables.service.ts index 18a2879ac65..0605732e6d6 100644 --- a/packages/cli/src/modules/agents/agent-preview-test-variables.service.ts +++ b/packages/cli/src/modules/agents/agent-preview-test-variables.service.ts @@ -1,7 +1,9 @@ +import { AgentsConfig } from '@n8n/config'; +import { Time } from '@n8n/constants'; import { Service } from '@n8n/di'; -/** A queued message can wait behind a running turn. Drop values nobody consumed after this time. */ -const HOLD_TTL_MS = 60 * 60 * 1000; +/** Minimum hold time, used when the configured checkpoint TTL is shorter or invalid. */ +const MIN_HOLD_TTL_MS = 60 * 60 * 1000; /** * Holds editor preview test variables in memory, keyed by queue item ID. @@ -12,11 +14,22 @@ const HOLD_TTL_MS = 60 * 60 * 1000; export class AgentPreviewTestVariablesService { private readonly held = new Map; expiresAt: number }>(); + constructor(private readonly agentsConfig: AgentsConfig) {} + + /** + * A queued message can wait behind a suspended run. That run lives as long as + * its checkpoint, so hold the values at least that long. + */ + private get holdTtlMs(): number { + const checkpointMs = this.agentsConfig.checkpointTtlSeconds * Time.seconds.toMilliseconds; + return Math.max(MIN_HOLD_TTL_MS, Number.isFinite(checkpointMs) ? checkpointMs : 0); + } + hold(queueId: string, values: Record | undefined): void { const now = Date.now(); for (const [id, entry] of this.held) if (entry.expiresAt <= now) this.held.delete(id); if (!values || Object.keys(values).length === 0) return; - this.held.set(queueId, { values, expiresAt: now + HOLD_TTL_MS }); + this.held.set(queueId, { values, expiresAt: now + this.holdTtlMs }); } /** Return the values for a queue item and forget them. */ diff --git a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts index e5c5bad1ee6..2cbf8a024f2 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts @@ -833,11 +833,13 @@ describe('resolveNodeTool → schema sanitizing of non-object shapes', () => { z.object({ b: z.string() }), ]), ) + .nullable() .optional(), ); const schema = tool.inputSchema as z.ZodTypeAny; expect(schema.safeParse({ q: 'x', a: 'y' }).success).toBe(true); expect(schema.safeParse(undefined).success).toBe(true); + expect(schema.safeParse(null).success).toBe(true); const json = JSON.stringify( (await import('zod-to-json-schema')).zodToJsonSchema(schema as never), ); @@ -857,6 +859,79 @@ describe('resolveNodeTool → schema sanitizing of non-object shapes', () => { }); }); +describe('resolveNodeTool → error redaction and trusted precedence', () => { + const secretTool = async (executeInline: ReturnType, extra = {}) => + await resolveNodeTool( + { + ...baseToolSchema, + node: { ...baseToolSchema.node, nodeParameters: {} }, + trustedMetadata: { tenant_id: { mode: 'fixed', value: 'tenant_789_secret' } }, + ...extra, + }, + { executor: { executeInline } as unknown as EphemeralNodeExecutor, projectId: 'p1' }, + ); + + it('redacts trusted values from a thrown executor error', async () => { + const executeInline = vi.fn().mockResolvedValue({ + status: 'error', + data: [], + error: 'Request failed for tenant_789_secret and caller-secret-value', + }); + const tool = await secretTool(executeInline); + const error = await tool.handler!({}, { + runtimeMetadata: { tenant_id: 'caller-secret-value' }, + } as never).catch((e: Error) => e); + expect((error as Error).message).toBe('Request failed for [redacted] and [redacted]'); + }); + + it('redacts trusted values from a policy refusal message', async () => { + const executeInline = vi.fn().mockResolvedValue({ + status: 'error', + data: [], + error: 'Blocked for tenant_789_secret', + violations: [violation()], + }); + const tool = await secretTool(executeInline); + const output = await tool.handler!({}, {} as never); + expect(output).toMatchObject({ isPolicyRefusal: true, message: 'Blocked for [redacted]' }); + }); + + it('does not let a model argument survive when a non-variable trusted default is empty', async () => { + const executeInline = vi.fn().mockResolvedValue({ status: 'success', data: [] }); + const tool = await secretTool(executeInline, { + trustedMetadata: { tenant_id: { mode: 'fixed', value: '' } }, + }); + await tool.handler!({ tenant_id: 'invented' } as never, {} as never); + expect(executeInline.mock.calls[0][0].inputData[0].json).toEqual({ tenant_id: '' }); + }); + + it('advertises no input field in the native string fallback when input is trusted', async () => { + Container.set(NodeTypes, { + getByNameAndVersion: vi.fn().mockReturnValue({ + description: { description: 'Think' }, + supplyData: vi.fn(), + }), + } as unknown as NodeTypes); + const tool = await resolveNodeTool( + { + ...baseToolSchema, + node: { ...baseToolSchema.node, nodeParameters: {} }, + trustedMetadata: { input: { mode: 'fixed', value: 'fixed-input-value' } }, + }, + { + executor: { + introspectSupplyDataToolSchema: vi.fn().mockResolvedValue(null), + } as unknown as EphemeralNodeExecutor, + projectId: 'p1', + }, + ); + const schema = tool.inputSchema as z.ZodTypeAny; + const parsed = schema.safeParse({ input: 'model value' }); + expect(parsed.success).toBe(true); + expect(parsed.success && parsed.data).toEqual({}); + }); +}); + describe('resolveNodeTool → result redaction', () => { it('redacts echoed trusted values without mutating the original result', async () => { const original = { diff --git a/packages/cli/src/modules/agents/tools/node-tool-factory.ts b/packages/cli/src/modules/agents/tools/node-tool-factory.ts index 2830e4ca043..51a7cd09f0d 100644 --- a/packages/cli/src/modules/agents/tools/node-tool-factory.ts +++ b/packages/cli/src/modules/agents/tools/node-tool-factory.ts @@ -53,7 +53,13 @@ function toExecutorCredentials( return Object.keys(out).length > 0 ? out : undefined; } -function createNativeStringToolInputSchema(description: string): z.ZodType { +function createNativeStringToolInputSchema( + description: string, + trustedKeys: ReadonlySet, +): z.ZodType { + // `input` is the only field of this fallback. When it is trusted, the model + // has nothing to choose. + if (trustedKeys.has('input')) return z.object({}); return z.preprocess( (value) => (typeof value === 'string' ? { input: value } : value), z.object({ @@ -126,10 +132,6 @@ export async function resolveNodeToolMetadata( return { ...getFixedNodeToolMetadata(metadata), ...resolved }; } -function withoutEmpty(values: Record): Record { - return Object.fromEntries(Object.entries(values).filter(([, value]) => hasValue(value))); -} - function hasValue(value: unknown): boolean { return value !== undefined && value !== null && value !== ''; } @@ -259,6 +261,13 @@ function redactTrustedValues(value: unknown, secrets: ReadonlySet): unkn return value; } +/** Replace trusted strings contained in free text, such as an error message. */ +function redactTrustedText(text: string, secrets: ReadonlySet): string { + let out = text; + for (const secret of secrets) out = out.split(secret).join('[redacted]'); + return out; +} + /** * Native tool nodes expose a LangChain tool via `supplyData`. The shape of its * schema depends on the class: @@ -325,6 +334,7 @@ async function resolveInputSchema( toolSchema.description ?? nodeType.description.description ?? `The query or input text to pass to ${toolSchema.node.nodeType}.`, + trustedKeys, ); } @@ -389,19 +399,25 @@ export async function resolveNodeTool( `Tool "${sanitizedName}" needs a value for ${missing.map((name) => `"${name}"`).join(', ')}. Map it in the Message an Agent node. Runs without a calling workflow (chat, schedule, channels, tests) use the default value from the tool configuration, so set one there.`, ); } + const secrets = new Set( + [...Object.values(trusted), ...Object.values(runtime)].filter( + (value): value is string => + typeof value === 'string' && value.length >= MIN_REDACTED_LENGTH, + ), + ); const result = await ctx.executor.executeInline({ nodeType, nodeTypeVersion: toolSchema.node.nodeTypeVersion, nodeParameters: toolSchema.node.nodeParameters as INodeParameters, credentialDetails: toExecutorCredentials(toolSchema.node.credentials), - inputData: [{ json: { ...input, ...withoutEmpty(trusted), ...runtime } as IDataObject }], + inputData: [{ json: { ...input, ...trusted, ...runtime } as IDataObject }], projectId: ctx.projectId, ...instrumentedExecution, }); // A refusal is returned, not thrown: thrown errors reach the chat as plain text. if (result.status === 'error' && result.violations) { return toPolicyRefusalResult( - result.error ?? result.violations[0].message, + redactTrustedText(result.error ?? result.violations[0].message, secrets), result.violations, ); } @@ -410,14 +426,13 @@ export async function resolveNodeTool( // marks the timeline entry as a failed call. Returning the error // object normally would otherwise read as a successful tool call. if (result.status === 'error') { - throw new Error(result.error ?? `Node "${toolSchema.node.nodeType}" failed to execute`); + throw new Error( + redactTrustedText( + result.error ?? `Node "${toolSchema.node.nodeType}" failed to execute`, + secrets, + ), + ); } - const secrets = new Set( - Object.values({ ...withoutEmpty(trusted), ...runtime }).filter( - (value): value is string => - typeof value === 'string' && value.length >= MIN_REDACTED_LENGTH, - ), - ); return secrets.size > 0 ? redactTrustedValues(result, secrets) : result; }) .build(); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 4d3041114b1..9e1391d7690 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -8670,7 +8670,7 @@ "agents.variables.test.placeholder.default": "Default: {value}", "agents.variables.test.inputLabel": "Test value for {name}", "agents.toolConfig.metadata.default.label": "Default value (optional)", - "agents.toolConfig.metadata.hint": "Values this tool reads with $json. A Message an Agent node supplies them. Runs without one (chat, schedule, channels, test runs and sub-agents) use the default value, so set one for any variable those runs need.", + "agents.toolConfig.metadata.hint": "Values this tool reads with $json. They come from a Message an Agent node, and sub-agents it delegates to inherit them. Runs without one (chat, schedule, channels, test runs) use the default value, so set one for any variable those runs need.", "agents.toolConfig.metadata.value.invalid-json": "Enter valid JSON, or plain text without braces.", "agents.toolConfig.validation.fixFields": "Fix the highlighted fields before saving.", "agents.toolConfig.name.validation.required": "Enter a name.", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentTestVariablesPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentTestVariablesPanel.test.ts index e37bcfa8820..011ff5d0f0a 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentTestVariablesPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentTestVariablesPanel.test.ts @@ -72,6 +72,27 @@ describe('AgentTestVariablesPanel', () => { expect(payload.value).toEqual({ token: 'secret' }); }); + it('does not read inherited object members for variables named like them (toString, valueOf)', async () => { + const config = { + tools: [ + { + type: 'node', + name: 'Fetch', + node: { nodeParameters: { a: '={{ $json.valueOf }}/{{ $json.toString }}' } }, + }, + ], + } as unknown as AgentJsonConfig; + const { wrapper, agentId } = render(config); + await wrapper.find('[data-testid="agent-test-values-toggle"]').trigger('click'); + const inputs = wrapper.findAll('input'); + expect(inputs.map((i) => i.element.value)).toEqual(['', '']); + expect(useAgentTestVariables(agentId, config).payload.value).toBeUndefined(); + + await inputs[0].setValue('x'); + expect(useAgentTestVariables(agentId, config).payload.value).toEqual({ valueOf: 'x' }); + expect(wrapper.findAll('input')[1].element.value).toBe(''); + }); + it('has no payload when nothing is entered', () => { const { payload } = useAgentTestVariables('fresh-agent', makeConfig()); expect(payload.value).toBeUndefined(); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentVariablesOverview.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentVariablesOverview.test.ts index 37f0b4b8443..6352e6d8bcc 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentVariablesOverview.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentVariablesOverview.test.ts @@ -49,6 +49,15 @@ describe('AgentVariablesOverview', () => { expect(toolNames).toEqual(['Fetch', 'Post']); }); + it('treats an empty fixed default as no default', () => { + const wrapper = render([ + { name: 'token', tools: ['Fetch'], default: { mode: 'fixed', value: '' } }, + ]); + const status = wrapper.find('[data-testid="agent-variable-status"]'); + expect(status.text()).toContain('agents.variables.status.noDefault'); + expect(wrapper.find('[data-testid="agent-variable-default"]').exists()).toBe(false); + }); + it('flags a variable without a default', () => { const wrapper = render([{ name: 'token', tools: ['Fetch'] }]); const status = wrapper.find('[data-testid="agent-variable-status"]'); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/NodeToolTrustedMetadataSection.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/NodeToolTrustedMetadataSection.test.ts index 9c19043176e..99abe952bba 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/NodeToolTrustedMetadataSection.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/NodeToolTrustedMetadataSection.test.ts @@ -43,6 +43,15 @@ describe('NodeToolTrustedMetadata', () => { expect(wrapper.findAll('button')).toHaveLength(0); }); + it('does not render rows for names the wire schema rejects', () => { + const long = 'a'.repeat(64); + const wrapper = render({ + nodeParameters: { url: `={{ $json.ok }}/{{ $json.${long} }}` }, + }); + const names = wrapper.findAll('[data-test-id="agent-trusted-metadata-name"]'); + expect(names.map((n) => n.text())).toEqual(['ok']); + }); + it('is hidden when no variables are detected', () => { const wrapper = render({ nodeParameters: { url: 'https://x.test' } }); expect(wrapper.find('[data-test-id="agent-node-tool-trusted-metadata"]').exists()).toBe(false); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts index a67d0c203d4..0aee48c99bb 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts @@ -357,6 +357,48 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { expect(assistantMessages[0].toolCalls?.[0].state).toBe('suspended'); }); + it.each([ + ['sends non-empty testVariables', { token: 'abc' }, { token: 'abc' }], + ['omits empty testVariables', {}, undefined], + ['omits unset testVariables', undefined, undefined], + ])('resume %s in the request body', async (_label, testVariables, expected) => { + const suspended = (): Response => + makeSseResponse([ + { + type: 'tool-call', + toolCallId: 'tc-v', + toolName: 'calculator', + input: { input: '1' }, + }, + { + type: 'tool-call-suspended', + payload: { + toolCallId: 'tc-v', + runId: 'run-v', + toolName: 'calculator', + input: { type: 'approval', toolName: 'calculator', args: { input: '1' } }, + }, + }, + { type: 'done' }, + ]); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(suspended()) + .mockResolvedValueOnce(makeSseResponse([{ type: 'done' }])); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const hook = buildHook(undefined, { testVariables: () => testVariables }); + await hook.sendMessage('go'); + await flushPromises(); + await nextTick(); + await hook.resume({ runId: 'run-v', toolCallId: 'tc-v', resumeData: { approved: true } }); + + const resumeCall = fetchMock.mock.calls.find(([url]) => String(url).endsWith('/chat/resume')); + const body = JSON.parse((resumeCall?.[1] as { body: string }).body) as Record; + if (expected) expect(body.testVariables).toEqual(expected); + else expect(body).not.toHaveProperty('testVariables'); + }); + it('posts approval resumes to the chat resume endpoint in preview chat mode', async () => { const fetchMock = vi .fn() diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentTestVariablesPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentTestVariablesPanel.vue index c98caf1e6b4..963bdbdf590 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentTestVariablesPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentTestVariablesPanel.vue @@ -5,6 +5,7 @@ import { useI18n } from '@n8n/i18n'; import { describeVariableDefault, + hasUsableDefault, useAgentTestVariables, } from '../composables/useAgentTestVariables'; import type { AgentJsonConfig } from '../types'; @@ -17,7 +18,7 @@ const props = defineProps<{ const i18n = useI18n(); const expanded = ref(false); -const { variables, values, setValue } = useAgentTestVariables( +const { variables, getValue, setValue } = useAgentTestVariables( () => props.agentId, () => props.config, ); @@ -25,7 +26,7 @@ const { variables, values, setValue } = useAgentTestVariables( const rows = computed(() => variables.value.map((variable) => { const text = describeVariableDefault(variable); - const placeholder = !variable.default + const placeholder = !hasUsableDefault(variable) ? i18n.baseText('agents.variables.test.placeholder.required') : text === undefined ? i18n.baseText('agents.variables.test.placeholder.expression') @@ -61,7 +62,7 @@ const rows = computed(() =>