mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 21:49:36 +02:00
fix(core): Ignore undeclared fields in agent resume payloads (backport to release-candidate/2.34.x) (#35705)
Co-authored-by: José Braulio González Valido <jose.gonzalez@n8n.io>
This commit is contained in:
parent
970da944b9
commit
a944f62402
2
.github/WORKFLOWS.md
vendored
2
.github/WORKFLOWS.md
vendored
|
|
@ -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
|
||||
|
|
|
|||
1
.github/workflows/ci-instance-ai-evals.yml
vendored
1
.github/workflows/ci-instance-ai-evals.yml
vendored
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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' });
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -9,22 +9,27 @@ export type ParseResult<T = unknown> =
|
|||
| { success: true; data: T }
|
||||
| { success: false; error: string };
|
||||
|
||||
let ajvInstance: InstanceType<typeof AjvType> | undefined;
|
||||
let nonUnicodeAjvInstance: InstanceType<typeof AjvType> | 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<typeof AjvType> {
|
||||
const instance = unicodeRegExp ? ajvInstance : nonUnicodeAjvInstance;
|
||||
if (instance) return instance;
|
||||
const ajvInstances = new Map<string, InstanceType<typeof AjvType>>();
|
||||
|
||||
function getAjv(unicodeRegExp = true, stripUnknown = false): InstanceType<typeof AjvType> {
|
||||
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<typeof AjvType> {
|
|||
export async function parseWithSchema(
|
||||
schema: ZodType | JSONSchema7,
|
||||
data: unknown,
|
||||
options: ParseOptions = {},
|
||||
): Promise<ParseResult> {
|
||||
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) };
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user