diff --git a/.github/WORKFLOWS.md b/.github/WORKFLOWS.md index 21cb7aa118a..5b4779e55a5 100644 --- a/.github/WORKFLOWS.md +++ b/.github/WORKFLOWS.md @@ -180,7 +180,7 @@ These only run if specific files changed: | `packages/@n8n/ai-workflow-builder.ee/evaluations/programmatic/python/**` | `test-evals-python.yml` | any | | `packages/@n8n/benchmark/**` | `build-benchmark-image.yml` | master | | `packages/cli/src/public-api/**/*.{css,yaml,yml}` | `util-publish-api-schema.yml` | master | -| `packages/@n8n/instance-ai/src/**`, `packages/@n8n/instance-ai/skills/**`, `packages/@n8n/instance-ai/knowledge-base/**`, `packages/@n8n/instance-ai/evaluations/**`, `packages/cli/src/modules/instance-ai/**`, `packages/core/src/execution-engine/eval-mock-helpers.ts` | `ci-instance-ai-evals.yml` | on PR `opened` / `reopened` / `ready_for_review` | +| `packages/@n8n/instance-ai/src/**`, `packages/@n8n/instance-ai/skills/**`, `packages/@n8n/instance-ai/knowledge-base/**`, `packages/@n8n/instance-ai/evaluations/**`, `packages/cli/src/modules/instance-ai/**`, `packages/core/src/execution-engine/eval-mock-helpers.ts`, `packages/@n8n/agents/src/**` | `ci-instance-ai-evals.yml` | on PR `opened` / `reopened` / `ready_for_review` | | `docker/get-n8n.sh`, `docker/test-get-n8n.sh` | `test-get-n8n.yml` | any | ### On PR Review diff --git a/.github/workflows/ci-instance-ai-evals.yml b/.github/workflows/ci-instance-ai-evals.yml index 0a35dff4aec..26f3ee11bed 100644 --- a/.github/workflows/ci-instance-ai-evals.yml +++ b/.github/workflows/ci-instance-ai-evals.yml @@ -19,6 +19,7 @@ on: - 'packages/@n8n/instance-ai/evaluations/**' - 'packages/cli/src/modules/instance-ai/**' - 'packages/core/src/execution-engine/eval-mock-helpers.ts' + - 'packages/@n8n/agents/src/**' workflow_dispatch: inputs: pr: diff --git a/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts b/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts index 21ab3ed32e0..226472625b6 100644 --- a/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts +++ b/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts @@ -4137,6 +4137,45 @@ describe('AgentRuntime — runtime resume data schema validation', () => { ); await expect(resumeResultPromise).rejects.toThrow('Invalid resume payload'); }); + + it('strips keys the persisted schema does not declare instead of rejecting', async () => { + let observedResumeData: unknown; + const tool: BuiltTool = { + name: 'approve', + description: 'Requires approval', + inputSchema: z.object({ question: z.string() }), + suspendSchema: z.object({ question: z.string() }), + resumeSchema: z.object({ approved: z.boolean() }), + handler: async (_input: unknown, ctx: unknown) => { + const { suspend, resumeData } = ctx as InterruptibleToolContext; + if (!resumeData) return await suspend({ question: 'approve?' }); + observedResumeData = resumeData; + return { approved: true }; + }, + }; + + generateText.mockResolvedValueOnce( + makeGenerateWithToolCall('tc-1', 'approve', { question: 'ok?' }), + ); + + const runtimeWithTool = new AgentRuntime({ + name: 'test', + model: 'openai/gpt-4o-mini', + instructions: 'test', + tools: [tool], + checkpointStorage: 'memory', + }); + + const first = await runtimeWithTool.generate('go'); + const { runId, toolCallId } = first.pendingSuspend![0]; + + generateText.mockResolvedValueOnce(makeGenerateSuccess('done')); + const payload = { approved: true, userInput: 'looks good' }; + await runtimeWithTool.resume('generate', payload, { runId, toolCallId }); + + expect(observedResumeData).toEqual({ approved: true }); + expect(payload).toEqual({ approved: true, userInput: 'looks good' }); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/@n8n/agents/src/runtime/loop/agent-runtime.ts b/packages/@n8n/agents/src/runtime/loop/agent-runtime.ts index f5740fbdcc1..384f286f242 100644 --- a/packages/@n8n/agents/src/runtime/loop/agent-runtime.ts +++ b/packages/@n8n/agents/src/runtime/loop/agent-runtime.ts @@ -372,7 +372,7 @@ export class AgentRuntime { const resumeSchema = toolCall.suspended ? toolCall.resumeSchema : tool.resumeSchema; if (!isCancellation(resumeData) && resumeSchema) { - const parseResult = await parseWithSchema(resumeSchema, data); + const parseResult = await parseWithSchema(resumeSchema, data, { stripUnknown: true }); if (!parseResult.success) { throw new Error(`Invalid resume payload: ${parseResult.error}`); } diff --git a/packages/@n8n/agents/src/utils/__tests__/parse.test.ts b/packages/@n8n/agents/src/utils/__tests__/parse.test.ts index 318cec538e1..85ca70320aa 100644 --- a/packages/@n8n/agents/src/utils/__tests__/parse.test.ts +++ b/packages/@n8n/agents/src/utils/__tests__/parse.test.ts @@ -170,3 +170,67 @@ describe('parseWithSchema — JSON Schema', () => { expect(invalid.success).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// parseWithSchema — stripUnknown +// --------------------------------------------------------------------------- + +describe('parseWithSchema — stripUnknown', () => { + const strictSchema = { + type: 'object' as const, + properties: { approved: { type: 'boolean' } }, + required: ['approved'], + additionalProperties: false, + } as JSONSchema7; + + it('rejects undeclared properties by default', async () => { + const result = await parseWithSchema(strictSchema, { approved: true, userInput: 'hi' }); + + expect(result.success).toBe(false); + if (!result.success) expect(result.error).toContain('additional properties'); + }); + + it('drops undeclared properties when enabled, leaving the input untouched', async () => { + const data = { approved: true, userInput: 'hi' }; + const result = await parseWithSchema(strictSchema, data, { stripUnknown: true }); + + expect(result.success).toBe(true); + if (result.success) expect(result.data).toEqual({ approved: true }); + expect(data).toEqual({ approved: true, userInput: 'hi' }); + }); + + it('drops undeclared properties from nested objects', async () => { + const schema = { + type: 'object' as const, + properties: { + answer: { + type: 'object', + properties: { questionId: { type: 'string' } }, + required: ['questionId'], + additionalProperties: false, + }, + }, + required: ['answer'], + additionalProperties: false, + } as JSONSchema7; + + const result = await parseWithSchema( + schema, + { answer: { questionId: 'q1', skipped: true } }, + { stripUnknown: true }, + ); + + expect(result.success).toBe(true); + if (result.success) expect(result.data).toEqual({ answer: { questionId: 'q1' } }); + }); + + it('still fails on a declared property with the wrong type', async () => { + const result = await parseWithSchema( + strictSchema, + { approved: 'yes', userInput: 'hi' }, + { stripUnknown: true }, + ); + + expect(result.success).toBe(false); + }); +}); diff --git a/packages/@n8n/agents/src/utils/parse.ts b/packages/@n8n/agents/src/utils/parse.ts index e6c5616b895..2b53f09536c 100644 --- a/packages/@n8n/agents/src/utils/parse.ts +++ b/packages/@n8n/agents/src/utils/parse.ts @@ -9,22 +9,27 @@ export type ParseResult = | { success: true; data: T } | { success: false; error: string }; -let ajvInstance: InstanceType | undefined; -let nonUnicodeAjvInstance: InstanceType | undefined; +export interface ParseOptions { + /** For schemas converted from Zod, whose `.strip()` `zodToJsonSchema` renders as the stricter `additionalProperties: false`. */ + stripUnknown?: boolean; +} -function getAjv(unicodeRegExp = true): InstanceType { - const instance = unicodeRegExp ? ajvInstance : nonUnicodeAjvInstance; - if (instance) return instance; +const ajvInstances = new Map>(); + +function getAjv(unicodeRegExp = true, stripUnknown = false): InstanceType { + const key = `${String(unicodeRegExp)}:${String(stripUnknown)}`; + const cached = ajvInstances.get(key); + if (cached) return cached; // eslint-disable-next-line @typescript-eslint/no-require-imports const { default: Ajv } = require('ajv') as { default: typeof AjvType }; - const newInstance = new Ajv({ + const instance = new Ajv({ strict: false, ...(unicodeRegExp ? {} : { unicodeRegExp: false }), + ...(stripUnknown ? { removeAdditional: true } : {}), }); - if (unicodeRegExp) ajvInstance = newInstance; - else nonUnicodeAjvInstance = newInstance; - return newInstance; + ajvInstances.set(key, instance); + return instance; } /** @@ -34,6 +39,7 @@ function getAjv(unicodeRegExp = true): InstanceType { export async function parseWithSchema( schema: ZodType | JSONSchema7, data: unknown, + options: ParseOptions = {}, ): Promise { if (isZodSchema(schema)) { const result = await schema.safeParseAsync(data); @@ -41,15 +47,19 @@ export async function parseWithSchema( return { success: false, error: result.error.message }; } - let ajv = getAjv(); + const { stripUnknown = false } = options; + // Ajv strips in place, so clone to leave the caller's object intact as Zod does. + const target = stripUnknown ? structuredClone(data) : data; + + let ajv = getAjv(true, stripUnknown); let validate: ValidateFunction; try { validate = ajv.compile(schema); } catch (error) { if (!(error instanceof SyntaxError)) throw error; - ajv = getAjv(false); + ajv = getAjv(false, stripUnknown); validate = ajv.compile(schema); } - if (validate(data)) return { success: true, data }; + if (validate(target)) return { success: true, data: target }; return { success: false, error: ajv.errorsText(validate.errors) }; }