fix(agents): Address review feedback on variables and test values

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Farzan 2026-10-17 20:59:12 +05:30
parent c23e28ba08
commit b754e2ca83
22 changed files with 597 additions and 70 deletions

View File

@ -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'] },
]);
});
});

View File

@ -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<string>();
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);
}
}

View File

@ -93,7 +93,9 @@ function makeController() {
mock<Subscriber>(),
);
const previewTestVariables = new AgentPreviewTestVariablesService();
const previewTestVariables = new AgentPreviewTestVariablesService(
mock<AgentsConfig>({ checkpointTtlSeconds: 96 * 3600 }),
);
const controller = new AgentChatController(
agentExecutionOrchestratorService,
agentTestRunService,

View File

@ -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<AgentChatExecutionService>();
const integrations = mock<ChatIntegrationService>();
const orchestrator = mock<AgentExecutionOrchestratorService>();
const previewTestVariables = new AgentPreviewTestVariablesService();
const previewTestVariables = new AgentPreviewTestVariablesService(
mock<AgentsConfig>({ 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',
);

View File

@ -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<AgentsConfig>({ 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' });
});
});

View File

@ -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<string, { values: Record<string, string>; 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<string, string> | 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. */

View File

@ -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<typeof vi.fn>, 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 = {

View File

@ -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<string>,
): 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<string, unknown>): Record<string, unknown> {
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<string>): unkn
return value;
}
/** Replace trusted strings contained in free text, such as an error message. */
function redactTrustedText(text: string, secrets: ReadonlySet<string>): 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();

View File

@ -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.",

View File

@ -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();

View File

@ -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"]');

View File

@ -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);

View File

@ -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<string, unknown>;
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()

View File

@ -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(() =>
<label v-for="row in rows" :key="row.name" :class="$style.field">
<code :class="$style.name">{{ row.name }}</code>
<N8nInput
:model-value="values[row.name] ?? ''"
:model-value="getValue(row.name)"
:placeholder="row.placeholder"
size="small"
:aria-label="

View File

@ -3,7 +3,7 @@ import { N8nIcon, N8nText } from '@n8n/design-system';
import { useI18n } from '@n8n/i18n';
import type { AgentVariable } from '@n8n/api-types';
import { describeVariableDefault } from '../composables/useAgentTestVariables';
import { describeVariableDefault, hasUsableDefault } from '../composables/useAgentTestVariables';
import type { ToolOpenTarget } from './AgentCapabilitiesSection.types';
defineProps<{ variables: AgentVariable[] }>();
@ -42,7 +42,11 @@ function openTool(name: string) {
</template>
</N8nText>
</div>
<div v-if="variable.default" :class="$style.status" data-testid="agent-variable-status">
<div
v-if="hasUsableDefault(variable)"
:class="$style.status"
data-testid="agent-variable-status"
>
<N8nIcon icon="check" size="small" color="success" aria-hidden="true" />
<N8nText size="small" color="text-base">
{{ i18n.baseText('agents.variables.status.hasDefault') }}

View File

@ -2,7 +2,11 @@
import { computed, onBeforeUnmount, provide, ref, useId, watch } from 'vue';
import { N8nText } from '@n8n/design-system';
import { useI18n, type BaseTextKey } from '@n8n/i18n';
import { extractNodeToolVariables, type NodeToolTrustedMetadata } from '@n8n/api-types';
import {
extractNodeToolVariables,
TrustedMetadataKeySchema,
type NodeToolTrustedMetadata,
} from '@n8n/api-types';
import { WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE, type INodeProperties } from 'n8n-workflow';
import type { IUpdateInformation } from '@/Interface';
import { ExpressionLocalResolveContextSymbol, WorkflowDocumentStoreKey } from '@/app/constants';
@ -39,7 +43,12 @@ const emit = defineEmits<{ 'update:valid': [valid: boolean] }>();
const i18n = useI18n();
const id = useId();
const variableNames = computed(() => extractNodeToolVariables(props.nodeParameters));
// Names the wire schema rejects can't be saved, so they never become rows.
const variableNames = computed(() =>
extractNodeToolVariables(props.nodeParameters).filter(
(name) => TrustedMetadataKeySchema.safeParse(name).success,
),
);
/** Rows the user edited in this session. Other rows come from the stored defaults. */
const edited = ref<Record<string, TrustedMetadataRow>>({});

View File

@ -1653,7 +1653,8 @@ export function useAgentChatStream(params: UseAgentChatStreamParams) {
runId: payload.runId,
toolCallId: payload.toolCallId,
resumeData,
...(resumeTestVariables && { testVariables: resumeTestVariables }),
...(resumeTestVariables &&
Object.keys(resumeTestVariables).length > 0 && { testVariables: resumeTestVariables }),
},
onAccepted,
);

View File

@ -7,6 +7,18 @@ import { formatTrustedMetadataFixedValue } from '../utils/nodeToolTrustedMetadat
/** Values typed in the preview chat, per agent. They live for the editor session only. */
const valuesByAgent = reactive<Record<string, Record<string, string>>>({});
/** Own-property read, so names like `constructor` never hit Object.prototype. */
function readOwn(record: Record<string, string> | undefined, name: string): string | undefined {
return record && Object.hasOwn(record, name) ? record[name] : undefined;
}
/** An empty fixed default means "no default". */
export function hasUsableDefault(variable: AgentVariable): boolean {
const binding = variable.default;
if (!binding) return false;
return !(binding.mode === 'fixed' && binding.value === '');
}
export function useAgentVariables(config: MaybeRefOrGetter<AgentJsonConfig | null | undefined>) {
return computed<AgentVariable[]>(() => collectAgentVariables(toValue(config)?.tools));
}
@ -25,20 +37,32 @@ export function useAgentTestVariables(
) {
const variables = useAgentVariables(config);
const values = computed(() => valuesByAgent[toValue(agentId)] ?? {});
const values = computed(() => {
const id = toValue(agentId);
return Object.hasOwn(valuesByAgent, id) ? valuesByAgent[id] : {};
});
function getValue(name: string): string {
return readOwn(values.value, name) ?? '';
}
function setValue(name: string, value: string) {
const id = toValue(agentId);
valuesByAgent[id] = { ...valuesByAgent[id], [name]: value };
const next: Record<string, string> = Object.create(null) as Record<string, string>;
Object.assign(next, Object.hasOwn(valuesByAgent, id) ? valuesByAgent[id] : {});
next[name] = value;
valuesByAgent[id] = next;
}
/** Only filled entries for variables the agent still reads. */
const payload = computed<Record<string, string> | undefined>(() => {
const entries = variables.value
.map(({ name }) => [name, values.value[name]] as const)
.filter(([, value]) => value !== undefined && value !== '');
const entries: Array<[string, string]> = [];
for (const { name } of variables.value) {
const value = readOwn(values.value, name);
if (value !== undefined && value !== '') entries.push([name, value]);
}
return entries.length ? Object.fromEntries(entries) : undefined;
});
return { variables, values, setValue, payload };
return { variables, values, getValue, setValue, payload };
}

View File

@ -30,6 +30,7 @@ vi.mock('../ParameterInputFull.vue', async () => {
'data-test-id': `input-${props.parameter.name}`,
placeholder: props.parameter.placeholder,
title: props.parameter.description,
'data-path': props.path,
value: props.value,
onInput: (e: Event) =>
emit('update', {
@ -127,4 +128,55 @@ describe('AgentVariablesParameter', () => {
expect(events.at(-1)?.[0]).toEqual({ name: 'trustedValues.values', value: undefined });
});
});
it('uses the saved entry index for the row path, not the render index', async () => {
const { findByTestId } = render({
values: [
{ name: 'tenant', value: 'a' },
{ name: 'dashboard_url', value: 'x' },
],
});
const dashboard = await findByTestId('input-dashboard_url');
const tenant = await findByTestId('input-tenant');
expect(dashboard).toHaveValue('x');
expect(dashboard).toHaveAttribute('data-path', 'trustedValues.values[1].value');
expect(tenant).toHaveValue('a');
expect(tenant).toHaveAttribute('data-path', 'trustedValues.values[0].value');
});
it('uses append positions for variables without a saved entry', async () => {
const { findByTestId } = render({ values: [{ name: 'tenant', value: 'a' }] });
expect(await findByTestId('input-dashboard_url')).toHaveAttribute(
'data-path',
'trustedValues.values[1].value',
);
expect(await findByTestId('input-tenant')).toHaveAttribute(
'data-path',
'trustedValues.values[0].value',
);
});
it('reads the variables of an inline agent stored as a JSON string', async () => {
const { findByTestId } = renderComponent({
props: {
parameter,
path: 'trustedValues',
nodeValues: { agentSource: 'inline', inlineAgent: JSON.stringify({ config: { tools } }) },
} as never,
});
await findByTestId('input-dashboard_url');
expect(getAgentConfig).not.toHaveBeenCalled();
});
it('treats an invalid inline agent JSON string as no variables', async () => {
const { findByTestId, queryByTestId } = renderComponent({
props: {
parameter,
path: 'trustedValues',
nodeValues: { agentSource: 'inline', inlineAgent: '{not json' },
} as never,
});
await findByTestId('agent-variables-empty');
expect(queryByTestId('input-dashboard_url')).toBeNull();
});
});

View File

@ -27,6 +27,7 @@ const emit = defineEmits<{
type Tools = Parameters<typeof collectAgentVariables>[0];
type Entry = { name: string; value: string };
type SavedEntry = Entry & { index: number };
const i18n = useI18n();
const rootStore = useRootStore();
@ -44,8 +45,21 @@ const agentId = computed(() => {
});
const isInline = computed(() => props.nodeValues.agentSource === 'inline');
const inlineTools = computed(() => {
const inline = props.nodeValues.inlineAgent as { config?: { tools?: unknown } } | undefined;
return inline?.config?.tools;
let inline: unknown = props.nodeValues.inlineAgent;
// The parameter can be stored as a JSON string. Invalid JSON means no agent is known.
if (typeof inline === 'string') {
try {
inline = JSON.parse(inline);
} catch {
return undefined;
}
}
if (!inline || typeof inline !== 'object') return undefined;
const config: unknown = Object.hasOwn(inline, 'config')
? (inline as { config?: unknown }).config
: undefined;
if (!config || typeof config !== 'object' || !Object.hasOwn(config, 'tools')) return undefined;
return (config as { tools?: unknown }).tools;
});
let requestId = 0;
@ -81,17 +95,26 @@ async function loadVariables() {
watch([agentId, isInline, inlineTools, projectId], loadVariables, { immediate: true });
const saved = computed<Entry[]>(() => {
const rawValues = computed(() => {
const raw = props.values?.values;
if (!Array.isArray(raw)) return [];
return raw
.filter((entry) => typeof entry?.name === 'string' && entry.name !== '')
.map((entry) => ({
name: entry.name as string,
value: typeof entry.value === 'string' ? entry.value : String(entry.value ?? ''),
}));
return Array.isArray(raw) ? raw : [];
});
/** Valid saved entries, each with its index in the stored `values` array. */
const saved = computed<SavedEntry[]>(() =>
rawValues.value.flatMap((entry, index) =>
typeof entry?.name === 'string' && entry.name !== ''
? [
{
name: entry.name,
value: typeof entry.value === 'string' ? entry.value : String(entry.value ?? ''),
index,
},
]
: [],
),
);
function describeDefault(variable: AgentVariable): string {
const fallback = variable.default;
if (!fallback) return '';
@ -103,14 +126,20 @@ function describeDefault(variable: AgentVariable): string {
}
const rows = computed(() => {
const savedByName = new Map(saved.value.map((entry) => [entry.name, entry.value]));
const result = variables.value.map((variable) => ({
name: variable.name,
value: savedByName.get(variable.name) ?? '',
placeholder: describeDefault(variable),
tools: variable.tools,
unused: false,
}));
const savedByName = new Map(saved.value.map((entry) => [entry.name, entry]));
// Entries without a saved value get the append positions they will get on insert.
let nextIndex = rawValues.value.length;
const result = variables.value.map((variable) => {
const entry = savedByName.get(variable.name);
return {
name: variable.name,
value: entry?.value ?? '',
index: entry ? entry.index : nextIndex++,
placeholder: describeDefault(variable),
tools: variable.tools,
unused: false,
};
});
if (agentKnown.value) {
const known = new Set(variables.value.map((variable) => variable.name));
for (const entry of saved.value) {
@ -154,7 +183,7 @@ function commit(name: string, value: NodeParameterValueType) {
if (text !== '') merged.push({ name, value: text });
placed = true;
} else {
merged.push(entry);
merged.push({ name: entry.name, value: entry.value });
}
}
if (!placed && text !== '') merged.push({ name, value: text });
@ -187,7 +216,7 @@ function onUpdate(name: string, update: IUpdateInformation) {
}}
</N8nText>
<div
v-for="(row, index) in rows"
v-for="row in rows"
:key="row.name"
:class="$style.row"
:data-test-id="`agent-variable-${row.name}`"
@ -196,7 +225,7 @@ function onUpdate(name: string, update: IUpdateInformation) {
<ParameterInputFull
:parameter="rowParameter(row)"
:value="row.value"
:path="`${path}.values[${index}].value`"
:path="`${path}.values[${row.index}].value`"
:is-read-only="isReadOnly"
@update="onUpdate(row.name, $event)"
/>

View File

@ -1076,6 +1076,8 @@ describe('MessageAnAgent Node', () => {
}
});
// Trusted value entries use `name:` keys that are not node parameter display names.
/* eslint-disable n8n-nodes-base/node-param-display-name-miscased, n8n-nodes-base/node-param-display-name-excess-inner-whitespace, n8n-nodes-base/node-param-display-name-untrimmed */
describe('trusted tool values', () => {
it('passes mapped trusted values through to executeAgent', async () => {
executeFunctions.getInputData.mockReturnValue([{ json: {} }]);
@ -1186,6 +1188,41 @@ describe('MessageAnAgent Node', () => {
expect(agentInfo.trustedValues).toEqual({ tenant_id: 'tenant-42' });
});
it.each([[undefined], [null], ['']])(
'throws when an expression mapping resolves to %p',
async (resolved) => {
executeFunctions.getInputData.mockReturnValue([{ json: {} }]);
executeFunctions.getNodeParameter.mockImplementation(
(
param: string,
_i?: number,
fallback?: unknown,
options?: { rawExpressions?: boolean },
) => {
if (param === 'trustedValues') {
return {
values: [
{
name: 'tenant_id',
value: options?.rawExpressions ? '={{ $json.tenant }}' : resolved,
},
],
} as unknown as NodeParameterValueType;
}
if (param === 'agentId') return { mode: 'id', value: 'agent-1' };
if (param === 'message') return 'Hello agent';
if (param === 'advanced.invokeMode') return 'perItem';
if (param === 'advanced' || param === 'advanced.session.session') return fallback ?? {};
return undefined;
},
);
executeFunctions.executeAgent.mockResolvedValue(mockAgentResult);
await expect(node.execute.call(executeFunctions)).rejects.toThrow('"tenant_id"');
expect(executeFunctions.executeAgent).not.toHaveBeenCalled();
},
);
it('omits trustedValues when every mapped value is empty', async () => {
executeFunctions.getInputData.mockReturnValue([{ json: {} }]);
mockParams({ trustedValues: { values: [{ name: 'region', value: '' }] } });
@ -1197,6 +1234,7 @@ describe('MessageAnAgent Node', () => {
expect('trustedValues' in agentInfo).toBe(false);
});
});
/* eslint-enable n8n-nodes-base/node-param-display-name-miscased, n8n-nodes-base/node-param-display-name-excess-inner-whitespace, n8n-nodes-base/node-param-display-name-untrimmed */
});
describe('MessageAnAgent versioning', () => {

View File

@ -99,6 +99,27 @@ function validateTrustedValueName(ctx: IExecuteFunctions, name: string, itemInde
}
}
function isNonEmptyExpression(value: unknown): boolean {
return typeof value === 'string' && value.startsWith('=') && value.slice(1).trim() !== '';
}
/** Unresolved "Trusted Tool Values" entries, to tell expressions from literal values. */
function getRawTrustedEntries(
ctx: IExecuteFunctions,
itemIndex: number,
): Array<{ value?: unknown } | undefined> {
const raw = ctx.getNodeParameter(
'trustedValues',
itemIndex,
{},
{
rawExpressions: true,
},
) as unknown;
const values = (raw as { values?: unknown } | undefined)?.values;
return Array.isArray(values) ? (values as Array<{ value?: unknown } | undefined>) : [];
}
/**
* Read the node's "Trusted Tool Values" mappings as a plain record keyed by
* trusted metadata name. Each entry's value resolves expressions in this
@ -120,10 +141,11 @@ export function getTrustedValues(
? (raw as { values: Array<{ name?: unknown; value?: unknown }> }).values
: [];
if (entries.length === 0) return undefined;
const rawEntries = getRawTrustedEntries(ctx, itemIndex);
const record: Record<string, unknown> = Object.create(null);
const seen = new Set<string>();
for (const entry of entries) {
for (const [index, entry] of entries.entries()) {
const name = typeof entry?.name === 'string' ? entry.name.trim() : '';
if (!name) {
throw new NodeOperationError(
@ -141,8 +163,20 @@ export function getTrustedValues(
);
}
seen.add(name);
// An empty value means "not provided": the agent tool falls back to its default.
if (entry?.value === undefined || entry.value === null || entry.value === '') continue;
if (entry?.value === undefined || entry.value === null || entry.value === '') {
// A mapping written as an expression that resolves to nothing is a
// failed lookup, not "not provided". Falling back to the tool default
// could send the call to the wrong tenant.
if (isNonEmptyExpression(rawEntries[index]?.value)) {
throw new NodeOperationError(
ctx.getNode(),
`The expression for trusted value "${name}" resolved to an empty value. Check the field it reads, or clear the value to use the tool default.`,
{ itemIndex },
);
}
// A literally empty value means "not provided": the agent tool falls back to its default.
continue;
}
if (isSafeObjectProperty(name)) {
setSafeObjectProperty(record, name, entry.value);
}