mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(core): Harden goal-graph binding injection at the tool boundary (no-changelog)
Bound tool inputs now degrade gracefully instead of passing broken values into the tool: a state value that misses the tool's declared input type is losslessly coerced when the coerced value validates (numeric string to number and the reverse, boolean strings), a binding whose slot is still empty falls back to a model-supplied value for that step, and a binding with no value from either side returns a clear "required input not set" result instead of an opaque schema error from the tool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5158304aed
commit
8b98cc6d64
|
|
@ -1,6 +1,7 @@
|
|||
import { AgentEvent, type AgentEventData, type BuiltTool, type ToolContext } from '@n8n/agents';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import type { JSONSchema7 } from 'json-schema';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { GoalGraphStateService } from '../goal-graph-state.service';
|
||||
import type { GoalGraphDefinition, SlotValues } from '../types';
|
||||
|
|
@ -8,9 +9,9 @@ import { wrapGoalTool } from '../wrap-tool';
|
|||
|
||||
const definition: GoalGraphDefinition = {
|
||||
slots: [
|
||||
{ name: 'customerEmail', type: 'string', source: 'agent' },
|
||||
{ name: 'customerSalesforceId', type: 'string', source: 'tool' },
|
||||
{ name: 'trialExtendedUntil', type: 'string', source: 'tool' },
|
||||
{ name: 'customerEmail', type: 'string', access: 'standard' },
|
||||
{ name: 'customerSalesforceId', type: 'string', access: 'protected' },
|
||||
{ name: 'trialExtendedUntil', type: 'string', access: 'protected' },
|
||||
],
|
||||
goals: [
|
||||
{
|
||||
|
|
@ -160,6 +161,75 @@ describe('wrapGoalTool', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('coerces an injected string to number when a zod input schema demands it', async () => {
|
||||
const handler = vi.fn().mockResolvedValue({ newTrialEnd: '2026-07-01' });
|
||||
// The lookup wrote a string id into state, but the tool declares a number.
|
||||
const { stateService } = makeStateService({ customerSalesforceId: '100500' });
|
||||
const wrapped = wrapGoalTool({
|
||||
tool: {
|
||||
name: 'extend_trial',
|
||||
description: 'Extend a trial',
|
||||
inputSchema: z.object({ customerId: z.number(), days: z.number() }),
|
||||
handler: handler as unknown as BuiltTool['handler'],
|
||||
},
|
||||
attachments: [extendTrialAttachment],
|
||||
definition,
|
||||
agentId: 'agent-1',
|
||||
stateService,
|
||||
});
|
||||
|
||||
await wrapped.handler!({ days: 14 }, makeCtx([]));
|
||||
|
||||
expect(handler).toHaveBeenCalledWith({ days: 14, customerId: 100500 }, expect.anything());
|
||||
});
|
||||
|
||||
it('coerces an injected string to number when a JSON input schema demands it', async () => {
|
||||
const handler = vi.fn().mockResolvedValue({ newTrialEnd: '2026-07-01' });
|
||||
const { stateService } = makeStateService({ customerSalesforceId: '100500' });
|
||||
const numberSchema: JSONSchema7 = {
|
||||
type: 'object',
|
||||
properties: { customerId: { type: 'number' }, days: { type: 'number' } },
|
||||
required: ['customerId', 'days'],
|
||||
};
|
||||
const wrapped = wrapGoalTool({
|
||||
tool: {
|
||||
name: 'extend_trial',
|
||||
description: 'Extend a trial',
|
||||
inputSchema: numberSchema,
|
||||
handler: handler as unknown as BuiltTool['handler'],
|
||||
},
|
||||
attachments: [extendTrialAttachment],
|
||||
definition,
|
||||
agentId: 'agent-1',
|
||||
stateService,
|
||||
});
|
||||
|
||||
await wrapped.handler!({ days: 14 }, makeCtx([]));
|
||||
|
||||
expect(handler).toHaveBeenCalledWith({ days: 14, customerId: 100500 }, expect.anything());
|
||||
});
|
||||
|
||||
it('keeps the injected value untouched when it already fits the schema', async () => {
|
||||
const handler = vi.fn().mockResolvedValue({ newTrialEnd: '2026-07-01' });
|
||||
const { stateService } = makeStateService({ customerSalesforceId: 'SF-1' });
|
||||
const wrapped = wrapGoalTool({
|
||||
tool: {
|
||||
name: 'extend_trial',
|
||||
description: 'Extend a trial',
|
||||
inputSchema: z.object({ customerId: z.string(), days: z.number() }),
|
||||
handler: handler as unknown as BuiltTool['handler'],
|
||||
},
|
||||
attachments: [extendTrialAttachment],
|
||||
definition,
|
||||
agentId: 'agent-1',
|
||||
stateService,
|
||||
});
|
||||
|
||||
await wrapped.handler!({ days: 14 }, makeCtx([]));
|
||||
|
||||
expect(handler).toHaveBeenCalledWith({ days: 14, customerId: 'SF-1' }, expect.anything());
|
||||
});
|
||||
|
||||
it('respects availableWhen on the attachment', async () => {
|
||||
const handler = vi.fn();
|
||||
const { stateService } = makeStateService({ customerSalesforceId: 'SF-1' });
|
||||
|
|
|
|||
|
|
@ -59,8 +59,35 @@ export function wrapGoalTool(options: WrapGoalToolOptions): BuiltTool {
|
|||
typeof input === 'object' && input !== null && !Array.isArray(input)
|
||||
? { ...(input as Record<string, unknown>) }
|
||||
: {};
|
||||
const missingBindings: string[] = [];
|
||||
for (const [param, expression] of Object.entries(active.attachment.bindings ?? {})) {
|
||||
injected[param] = evaluateGoalExpression(expression, { state });
|
||||
const value = evaluateGoalExpression(expression, { state });
|
||||
if (value !== null && value !== undefined) {
|
||||
// State holds the canonical value — it overrides whatever the model
|
||||
// passed (the core guarantee: the model can't forge a bound input).
|
||||
// Slot values keep their declared slot type, which may differ from the
|
||||
// tool's input type (e.g. a string id feeding a number input) — bridge
|
||||
// the boundary with a lossless coercion when the schema demands it.
|
||||
injected[param] = coerceBoundValue(tool.inputSchema, param, value);
|
||||
} else if (injected[param] === null || injected[param] === undefined) {
|
||||
// Neither state nor the model supplied it — genuinely missing.
|
||||
missingBindings.push(param);
|
||||
}
|
||||
// Else: state is empty but the model supplied the value this step (e.g. a
|
||||
// standard slot whose sibling fill_slot call in the same step hasn't
|
||||
// landed yet). Keep the model's value so the call still succeeds — the
|
||||
// gate already blocks tools whose prerequisite slots are unset.
|
||||
}
|
||||
|
||||
if (missingBindings.length > 0) {
|
||||
// A bound input has no value from state and none from the model. Return a
|
||||
// clear result instead of passing `undefined` into the tool, which would
|
||||
// fail its own input validation opaquely.
|
||||
return {
|
||||
ok: false,
|
||||
missingInputs: missingBindings,
|
||||
message: `Cannot run "${tool.name}" yet — required input(s) not set: ${missingBindings.join(', ')}. Set the state slot(s) that populate them first (via fill_slot for a standard slot, or by completing the goal whose tool writes them).`,
|
||||
};
|
||||
}
|
||||
|
||||
if (!tool.handler) {
|
||||
|
|
@ -112,6 +139,78 @@ export function findActiveAttachment(
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Lossless coercion candidates for a value that missed the expected primitive
|
||||
* type — expression-derived slot values often arrive as strings (everything
|
||||
* round-trips through JSON) while tool inputs declare numbers/booleans.
|
||||
*/
|
||||
function coercionCandidates(value: unknown): unknown[] {
|
||||
if (typeof value === 'string') {
|
||||
const candidates: unknown[] = [];
|
||||
const trimmed = value.trim();
|
||||
if (trimmed !== '' && Number.isFinite(Number(trimmed))) candidates.push(Number(trimmed));
|
||||
if (trimmed === 'true' || trimmed === 'false') candidates.push(trimmed === 'true');
|
||||
return candidates;
|
||||
}
|
||||
if (typeof value === 'number' || typeof value === 'boolean') return [String(value)];
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Fit an injected binding value to the tool's declared input type for `param`,
|
||||
* coercing only when the coerced value actually validates. The original value
|
||||
* is returned untouched when it already fits, when the schema cannot be
|
||||
* introspected, or when no lossless coercion satisfies it (the tool's own
|
||||
* validation then reports the mismatch).
|
||||
*/
|
||||
function coerceBoundValue(
|
||||
inputSchema: BuiltTool['inputSchema'],
|
||||
param: string,
|
||||
value: unknown,
|
||||
): unknown {
|
||||
if (!inputSchema) return value;
|
||||
|
||||
// Zod object schema (workflow tools) — probe the field validator directly.
|
||||
if (isZodSchema(inputSchema)) {
|
||||
const shape = (inputSchema as { shape?: Record<string, unknown> }).shape;
|
||||
const field = shape?.[param] as
|
||||
| { safeParse?: (v: unknown) => { success: boolean } }
|
||||
| undefined;
|
||||
if (!field?.safeParse) return value;
|
||||
if (field.safeParse(value).success) return value;
|
||||
for (const candidate of coercionCandidates(value)) {
|
||||
if (field.safeParse(candidate).success) return candidate;
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
// JSON Schema (custom tools) — match the declared primitive type.
|
||||
const property = inputSchema.properties?.[param];
|
||||
if (!property || typeof property !== 'object') return value;
|
||||
const expected = property.type;
|
||||
const actual = typeof value;
|
||||
if (
|
||||
(expected === 'number' && actual === 'number') ||
|
||||
(expected === 'integer' && actual === 'number') ||
|
||||
(expected === 'string' && actual === 'string') ||
|
||||
(expected === 'boolean' && actual === 'boolean')
|
||||
) {
|
||||
return value;
|
||||
}
|
||||
for (const candidate of coercionCandidates(value)) {
|
||||
const candidateType = typeof candidate;
|
||||
if (
|
||||
((expected === 'number' || expected === 'integer') && candidateType === 'number') ||
|
||||
(expected === 'string' && candidateType === 'string') ||
|
||||
(expected === 'boolean' && candidateType === 'boolean')
|
||||
) {
|
||||
if (expected === 'integer' && !Number.isInteger(candidate)) continue;
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove bound parameters from the LLM-visible input schema. Only JSON-Schema
|
||||
* inputs are rewritten (custom tools persist their schema as JSON Schema);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user