From 8b98cc6d64d83e85ab0b28e8da8663319661e98b Mon Sep 17 00:00:00 2001 From: Eugene Molodkin Date: Wed, 12 Aug 2026 14:28:42 +0200 Subject: [PATCH] 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) --- .../goal-graph/__tests__/wrap-tool.test.ts | 76 ++++++++++++- .../modules/agents/goal-graph/wrap-tool.ts | 101 +++++++++++++++++- 2 files changed, 173 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/modules/agents/goal-graph/__tests__/wrap-tool.test.ts b/packages/cli/src/modules/agents/goal-graph/__tests__/wrap-tool.test.ts index 1820f7946e1..8f1d6266686 100644 --- a/packages/cli/src/modules/agents/goal-graph/__tests__/wrap-tool.test.ts +++ b/packages/cli/src/modules/agents/goal-graph/__tests__/wrap-tool.test.ts @@ -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' }); diff --git a/packages/cli/src/modules/agents/goal-graph/wrap-tool.ts b/packages/cli/src/modules/agents/goal-graph/wrap-tool.ts index b22e6debd3d..e2f570e8b5c 100644 --- a/packages/cli/src/modules/agents/goal-graph/wrap-tool.ts +++ b/packages/cli/src/modules/agents/goal-graph/wrap-tool.ts @@ -59,8 +59,35 @@ export function wrapGoalTool(options: WrapGoalToolOptions): BuiltTool { typeof input === 'object' && input !== null && !Array.isArray(input) ? { ...(input as Record) } : {}; + 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 }).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);