From bb774f8c70c15295d14bf0edd3a7481c5a076e26 Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Thu, 7 Aug 2025 20:35:35 +0200 Subject: [PATCH] fix: update parameter validation tests to match new validation format - Updated 15 failing tests to expect new validation error format - Tests now expect 'tool_name: Validation failed' format instead of 'Missing required parameters' - Fixed type conversion expectations - new validation requires actual numbers, not strings - Updated tests for minimum value constraints (e.g., limit >= 1) - All 52 parameter validation tests now passing Tests were failing in CI because they expected the old error message format but the new validation system uses a more structured format with detailed field-level error messages. --- data/nodes.db | Bin 12296192 -> 12296192 bytes tests/unit/mcp/parameter-validation.test.ts | 144 ++++++++++---------- 2 files changed, 69 insertions(+), 75 deletions(-) diff --git a/data/nodes.db b/data/nodes.db index bdd9a1de2bc1c82c8a30a89bfa00bb27f1d82638..ca64d1358bca013e94fb7cdc41809eaccee75609 100644 GIT binary patch delta 844 zcmWmA)l!rJ0EXdz*~M1YKtb#dR1^!bySp*4ySrCWFj2%7bq&^55OftzxCpPq@d7;X z;hTA8zT-F6@Fz9KovgO;AZS?^1mR>51gSxg{<|zYHx#cfOb?Y$4QJMrhH68hWO+kM zMo~eyq#zt3g~lHtl_oT$8O>=yOIp#IHlz`yE$wJe2h!vz z-t?g_{pimC1~Q1jWHW@J3?ql(j3AehjAArn7@I5z=T&}66y`^Z8}bqjsUL{5FQY;mL{63b^i6PU;(CNqVpOk+ATn8_?=Gl#j%V?GO5$RZZAgrzKF zIV)JnDps?GwG^<9^=x1xg=}IoTiD7rwzGqs>|!^2*vmflbAW>!;xI=z$}x^}f|H!$ zG-o)=InHx|7#F!j5tq3_F(q8(8rQkOO>R-jZSGLUUCOz~eID?TM?B^UPpRM;&v`+d tm%QRNZ>S`}Ti)@W4}9bkpZUU9s;K50HPrH*A0+w7FY1z!SpC25{{f|D9b5na delta 707 zcmWmA)l!xL7)9al=NAhD1G~G$7Q4G!F|b>g-HNqr1+hhJ>_#z8xB+j)@e+L9GtbOE zUWHYEG7Cdd%LZjhlJzr5(xpj~WF|?@@8VtqQe~eCa#AJZ(#^kIO;w~)r6pAv%}(d1 z&*i66AtU_%)k0>d9%_V|p;o9JvO=9uH`ELDLw0Bo8iq!pamWcxLetPJG!HF8%g`#c z4sAl)&@QwO9YV*@DRd59Lf6nObPqj3&(JIM4t+x3kQ@4i{$W5E7zTyGVMrKSnV-&k z{x&;G{9m=G@;y{mW)}?qTi>d@@~UBv%0K31%WxSXBW09~mN7C`#>se@AQNShOqMA! zRi?>wnISV}mdutpGFRrwd|4n1WsxkFC9+hONxm$X6|z!RNr9}EHL_OL$$Hr!8)cJh zmMyYXw#jzcAv*=xCA(#h?3I18Uk=DYIV6YWh#Zw;a$HVGp`4Uca$3&FSy9f(dAT4L z<&s>MD{@t?Ns$!Gbt#b>a#L=}ZMh?N<(}M^Qh6X{@=zYhV|gM^<(WK}a(N*y<(0ga hH}Y2A$$R-AALWyLmM>BvU*(%r%6Iu8Kg;=*`wzC;4 { describe('search_nodes', () => { it('should require query parameter', async () => { await expect(server.testExecuteTool('search_nodes', {})) - .rejects.toThrow('Missing required parameters for search_nodes: query'); + .rejects.toThrow('search_nodes: Validation failed:\n • query: query is required'); }); it('should succeed with valid query', async () => { @@ -194,29 +194,28 @@ describe('Parameter Validation', () => { expect(result).toEqual({ results: [] }); }); - it('should convert limit to number and use default on invalid value', async () => { - const result = await server.testExecuteTool('search_nodes', { + it('should reject invalid limit value', async () => { + await expect(server.testExecuteTool('search_nodes', { query: 'http', limit: 'invalid' - }); - expect(result).toEqual({ results: [] }); + })).rejects.toThrow('search_nodes: Validation failed:\n • limit: limit must be a number, got string'); }); }); describe('validate_node_operation', () => { it('should require nodeType and config parameters', async () => { await expect(server.testExecuteTool('validate_node_operation', {})) - .rejects.toThrow('Missing required parameters for validate_node_operation: nodeType, config'); + .rejects.toThrow('validate_node_operation: Validation failed:\n • nodeType: nodeType is required\n • config: config is required'); }); it('should require nodeType parameter when config is provided', async () => { await expect(server.testExecuteTool('validate_node_operation', { config: {} })) - .rejects.toThrow('Missing required parameters for validate_node_operation: nodeType'); + .rejects.toThrow('validate_node_operation: Validation failed:\n • nodeType: nodeType is required'); }); it('should require config parameter when nodeType is provided', async () => { await expect(server.testExecuteTool('validate_node_operation', { nodeType: 'nodes-base.httpRequest' })) - .rejects.toThrow('Missing required parameters for validate_node_operation: config'); + .rejects.toThrow('validate_node_operation: Validation failed:\n • config: config is required'); }); it('should succeed with valid parameters', async () => { @@ -255,7 +254,7 @@ describe('Parameter Validation', () => { describe('list_node_templates', () => { it('should require nodeTypes parameter', async () => { await expect(server.testExecuteTool('list_node_templates', {})) - .rejects.toThrow('Missing required parameters for list_node_templates: nodeTypes'); + .rejects.toThrow('list_node_templates: Validation failed:\n • nodeTypes: nodeTypes is required'); }); it('should succeed with valid nodeTypes array', async () => { @@ -290,26 +289,18 @@ describe('Parameter Validation', () => { }); describe('limit parameter conversion', () => { - it('should convert string numbers to numbers', async () => { - const mockSearchNodes = vi.spyOn(server as any, 'searchNodes'); - - await server.testExecuteTool('search_nodes', { + it('should reject string limit values', async () => { + await expect(server.testExecuteTool('search_nodes', { query: 'test', limit: '15' - }); - - expect(mockSearchNodes).toHaveBeenCalledWith('test', 15, { mode: undefined }); + })).rejects.toThrow('search_nodes: Validation failed:\n • limit: limit must be a number, got string'); }); - it('should use default when limit is invalid string', async () => { - const mockSearchNodes = vi.spyOn(server as any, 'searchNodes'); - - await server.testExecuteTool('search_nodes', { + it('should reject invalid string limit values', async () => { + await expect(server.testExecuteTool('search_nodes', { query: 'test', limit: 'invalid' - }); - - expect(mockSearchNodes).toHaveBeenCalledWith('test', 20, { mode: undefined }); + })).rejects.toThrow('search_nodes: Validation failed:\n • limit: limit must be a number, got string'); }); it('should use default when limit is undefined', async () => { @@ -322,15 +313,11 @@ describe('Parameter Validation', () => { expect(mockSearchNodes).toHaveBeenCalledWith('test', 20, { mode: undefined }); }); - it('should handle zero as valid limit', async () => { - const mockSearchNodes = vi.spyOn(server as any, 'searchNodes'); - - await server.testExecuteTool('search_nodes', { + it('should reject zero as limit due to minimum constraint', async () => { + await expect(server.testExecuteTool('search_nodes', { query: 'test', limit: 0 - }); - - expect(mockSearchNodes).toHaveBeenCalledWith('test', 20, { mode: undefined }); // 0 converts to falsy, uses default + })).rejects.toThrow('search_nodes: Validation failed:\n • limit: limit must be at least 1, got 0'); }); }); @@ -361,26 +348,18 @@ describe('Parameter Validation', () => { }); describe('templateLimit parameter conversion', () => { - it('should convert string numbers to numbers', async () => { - const mockListNodeTemplates = vi.spyOn(server as any, 'listNodeTemplates'); - - await server.testExecuteTool('list_node_templates', { + it('should reject string limit values', async () => { + await expect(server.testExecuteTool('list_node_templates', { nodeTypes: ['nodes-base.httpRequest'], limit: '5' - }); - - expect(mockListNodeTemplates).toHaveBeenCalledWith(['nodes-base.httpRequest'], 5); + })).rejects.toThrow('list_node_templates: Validation failed:\n • limit: limit must be a number, got string'); }); - it('should use default when templateLimit is invalid', async () => { - const mockListNodeTemplates = vi.spyOn(server as any, 'listNodeTemplates'); - - await server.testExecuteTool('list_node_templates', { + it('should reject invalid string limit values', async () => { + await expect(server.testExecuteTool('list_node_templates', { nodeTypes: ['nodes-base.httpRequest'], limit: 'invalid' - }); - - expect(mockListNodeTemplates).toHaveBeenCalledWith(['nodes-base.httpRequest'], 10); + })).rejects.toThrow('list_node_templates: Validation failed:\n • limit: limit must be a number, got string'); }); }); @@ -452,7 +431,7 @@ describe('Parameter Validation', () => { it('should list all missing parameters', () => { expect(() => { server.testValidateToolParams('validate_node_operation', { profile: 'strict' }, ['nodeType', 'config']); - }).toThrow('Missing required parameters for validate_node_operation: nodeType, config'); + }).toThrow('validate_node_operation: Validation failed:\n • nodeType: nodeType is required\n • config: config is required'); }); it('should include helpful guidance', () => { @@ -475,10 +454,10 @@ describe('Parameter Validation', () => { .rejects.toThrow('Missing required parameters for get_node_info: nodeType'); await expect(server.testExecuteTool('search_nodes', {})) - .rejects.toThrow('Missing required parameters for search_nodes: query'); + .rejects.toThrow('search_nodes: Validation failed:\n • query: query is required'); await expect(server.testExecuteTool('validate_node_operation', { nodeType: 'test' })) - .rejects.toThrow('Missing required parameters for validate_node_operation: config'); + .rejects.toThrow('validate_node_operation: Validation failed:\n • config: config is required'); }); it('should handle edge cases in parameter validation gracefully', async () => { @@ -492,24 +471,34 @@ describe('Parameter Validation', () => { }); it('should provide consistent error format across all tools', async () => { - const toolsWithRequiredParams = [ - { name: 'get_node_info', args: {}, missing: 'nodeType' }, - { name: 'search_nodes', args: {}, missing: 'query' }, - { name: 'get_node_documentation', args: {}, missing: 'nodeType' }, - { name: 'get_node_essentials', args: {}, missing: 'nodeType' }, - { name: 'search_node_properties', args: {}, missing: 'nodeType, query' }, - { name: 'get_node_for_task', args: {}, missing: 'task' }, - { name: 'validate_node_operation', args: {}, missing: 'nodeType, config' }, - { name: 'validate_node_minimal', args: {}, missing: 'nodeType, config' }, - { name: 'get_property_dependencies', args: {}, missing: 'nodeType' }, - { name: 'get_node_as_tool_info', args: {}, missing: 'nodeType' }, - { name: 'list_node_templates', args: {}, missing: 'nodeTypes' }, - { name: 'get_template', args: {}, missing: 'templateId' }, + // Tools using legacy validation + const legacyValidationTools = [ + { name: 'get_node_info', args: {}, expected: 'Missing required parameters for get_node_info: nodeType' }, + { name: 'get_node_documentation', args: {}, expected: 'Missing required parameters for get_node_documentation: nodeType' }, + { name: 'get_node_essentials', args: {}, expected: 'Missing required parameters for get_node_essentials: nodeType' }, + { name: 'search_node_properties', args: {}, expected: 'Missing required parameters for search_node_properties: nodeType, query' }, + { name: 'get_node_for_task', args: {}, expected: 'Missing required parameters for get_node_for_task: task' }, + { name: 'get_property_dependencies', args: {}, expected: 'Missing required parameters for get_property_dependencies: nodeType' }, + { name: 'get_node_as_tool_info', args: {}, expected: 'Missing required parameters for get_node_as_tool_info: nodeType' }, + { name: 'get_template', args: {}, expected: 'Missing required parameters for get_template: templateId' }, ]; - for (const tool of toolsWithRequiredParams) { + for (const tool of legacyValidationTools) { await expect(server.testExecuteTool(tool.name, tool.args)) - .rejects.toThrow(`Missing required parameters for ${tool.name}: ${tool.missing}`); + .rejects.toThrow(tool.expected); + } + + // Tools using new schema validation + const schemaValidationTools = [ + { name: 'search_nodes', args: {}, expected: 'search_nodes: Validation failed:\n • query: query is required' }, + { name: 'validate_node_operation', args: {}, expected: 'validate_node_operation: Validation failed:\n • nodeType: nodeType is required\n • config: config is required' }, + { name: 'validate_node_minimal', args: {}, expected: 'validate_node_minimal: Validation failed:\n • nodeType: nodeType is required\n • config: config is required' }, + { name: 'list_node_templates', args: {}, expected: 'list_node_templates: Validation failed:\n • nodeTypes: nodeTypes is required' }, + ]; + + for (const tool of schemaValidationTools) { + await expect(server.testExecuteTool(tool.name, tool.args)) + .rejects.toThrow(tool.expected); } }); @@ -540,23 +529,28 @@ describe('Parameter Validation', () => { })); const n8nToolsWithRequiredParams = [ - { name: 'n8n_create_workflow', args: {}, missing: 'name, nodes, connections' }, - { name: 'n8n_get_workflow', args: {}, missing: 'id' }, - { name: 'n8n_get_workflow_details', args: {}, missing: 'id' }, - { name: 'n8n_get_workflow_structure', args: {}, missing: 'id' }, - { name: 'n8n_get_workflow_minimal', args: {}, missing: 'id' }, - { name: 'n8n_update_full_workflow', args: {}, missing: 'id' }, - { name: 'n8n_update_partial_workflow', args: {}, missing: 'id, operations' }, - { name: 'n8n_delete_workflow', args: {}, missing: 'id' }, - { name: 'n8n_validate_workflow', args: {}, missing: 'id' }, - { name: 'n8n_trigger_webhook_workflow', args: {}, missing: 'webhookUrl' }, - { name: 'n8n_get_execution', args: {}, missing: 'id' }, - { name: 'n8n_delete_execution', args: {}, missing: 'id' }, + { name: 'n8n_create_workflow', args: {}, expected: 'n8n_create_workflow: Validation failed:\n • name: name is required\n • nodes: nodes is required\n • connections: connections is required' }, + { name: 'n8n_get_workflow', args: {}, expected: 'n8n_get_workflow: Validation failed:\n • id: id is required' }, + { name: 'n8n_get_workflow_details', args: {}, expected: 'n8n_get_workflow_details: Validation failed:\n • id: id is required' }, + { name: 'n8n_get_workflow_structure', args: {}, expected: 'n8n_get_workflow_structure: Validation failed:\n • id: id is required' }, + { name: 'n8n_get_workflow_minimal', args: {}, expected: 'n8n_get_workflow_minimal: Validation failed:\n • id: id is required' }, + { name: 'n8n_update_full_workflow', args: {}, expected: 'n8n_update_full_workflow: Validation failed:\n • id: id is required' }, + { name: 'n8n_delete_workflow', args: {}, expected: 'n8n_delete_workflow: Validation failed:\n • id: id is required' }, + { name: 'n8n_validate_workflow', args: {}, expected: 'n8n_validate_workflow: Validation failed:\n • id: id is required' }, + { name: 'n8n_get_execution', args: {}, expected: 'n8n_get_execution: Validation failed:\n • id: id is required' }, + { name: 'n8n_delete_execution', args: {}, expected: 'n8n_delete_execution: Validation failed:\n • id: id is required' }, ]; + // n8n_update_partial_workflow and n8n_trigger_webhook_workflow use legacy validation + await expect(server.testExecuteTool('n8n_update_partial_workflow', {})) + .rejects.toThrow('Missing required parameters for n8n_update_partial_workflow: id, operations'); + + await expect(server.testExecuteTool('n8n_trigger_webhook_workflow', {})) + .rejects.toThrow('Missing required parameters for n8n_trigger_webhook_workflow: webhookUrl'); + for (const tool of n8nToolsWithRequiredParams) { await expect(server.testExecuteTool(tool.name, tool.args)) - .rejects.toThrow(`Missing required parameters for ${tool.name}: ${tool.missing}`); + .rejects.toThrow(tool.expected); } }); });