From 724c24933ce98ff4551e06f3a3c5c455ffad9fd7 Mon Sep 17 00:00:00 2001 From: Peter Stewart Date: Thu, 18 Sep 2025 15:04:27 +1000 Subject: [PATCH] Enable tool call type coersion (#477) * feat: enable tool call type coercion * fix: tests for type coercion --------- Co-authored-by: Mingholy --- packages/core/src/tools/glob.test.ts | 21 +++++++--------- .../core/src/tools/read-many-files.test.ts | 6 ++--- packages/core/src/tools/write-file.test.ts | 8 +++---- packages/core/src/utils/schemaValidator.ts | 24 +++++++++++++++++-- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index 3a911a57..5aba2b2b 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -62,6 +62,9 @@ describe('GlobTool', () => { // Ensure a noticeable difference in modification time await new Promise((resolve) => setTimeout(resolve, 50)); await fs.writeFile(path.join(tempRootDir, 'newer.sortme'), 'newer_content'); + + // For type coercion testing + await fs.mkdir(path.join(tempRootDir, '123')); }); afterEach(async () => { @@ -279,26 +282,20 @@ describe('GlobTool', () => { ); }); - it('should return error if path is provided but is not a string (schema validation)', () => { + it('should pass if path is provided but is not a string (type coercion)', () => { const params = { pattern: '*.ts', path: 123, - }; - // @ts-expect-error - We're intentionally creating invalid params for testing - expect(globTool.validateToolParams(params)).toBe( - 'params/path must be string', - ); + } as unknown as GlobToolParams; // Force incorrect type + expect(globTool.validateToolParams(params)).toBeNull(); }); - it('should return error if case_sensitive is provided but is not a boolean (schema validation)', () => { + it('should pass if case_sensitive is provided but is not a boolean (type coercion)', () => { const params = { pattern: '*.ts', case_sensitive: 'true', - }; - // @ts-expect-error - We're intentionally creating invalid params for testing - expect(globTool.validateToolParams(params)).toBe( - 'params/case_sensitive must be boolean', - ); + } as unknown as GlobToolParams; // Force incorrect type + expect(globTool.validateToolParams(params)).toBeNull(); }); it("should return error if search path resolves outside the tool's root directory", () => { diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index 551e4aeb..a6186fcb 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -191,14 +191,12 @@ describe('ReadManyFilesTool', () => { ); }); - it('should throw error if include array contains non-string elements', () => { + it('should coerce non-string elements in include array', () => { const params = { paths: ['file1.txt'], include: ['*.ts', 123] as string[], }; - expect(() => tool.build(params)).toThrow( - 'params/include/1 must be string', - ); + expect(() => tool.build(params)).toBeDefined(); }); it('should throw error if exclude array contains non-string elements', () => { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index c7058044..c4ed3755 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -220,14 +220,12 @@ describe('WriteFileTool', () => { ); }); - it('should throw an error if the content is null', () => { - const dirAsFilePath = path.join(rootDir, 'a_directory'); - fs.mkdirSync(dirAsFilePath); + it('should coerce null content into an empty string', () => { const params = { - file_path: dirAsFilePath, + file_path: path.join(rootDir, 'test.txt'), content: null, } as unknown as WriteFileToolParams; // Intentionally non-conforming - expect(() => tool.build(params)).toThrow('params/content must be string'); + expect(() => tool.build(params)).toBeDefined(); }); it('should throw error if the file_path is empty', () => { diff --git a/packages/core/src/utils/schemaValidator.ts b/packages/core/src/utils/schemaValidator.ts index 9465896e..3b009fc8 100644 --- a/packages/core/src/utils/schemaValidator.ts +++ b/packages/core/src/utils/schemaValidator.ts @@ -9,11 +9,12 @@ import * as addFormats from 'ajv-formats'; // Ajv's ESM/CJS interop: use 'any' for compatibility as recommended by Ajv docs // eslint-disable-next-line @typescript-eslint/no-explicit-any const AjvClass = (AjvPkg as any).default || AjvPkg; -const ajValidator = new AjvClass(); +const ajValidator = new AjvClass({ coerceTypes: true }); // eslint-disable-next-line @typescript-eslint/no-explicit-any const addFormatsFunc = (addFormats as any).default || addFormats; addFormatsFunc(ajValidator); + /** * Simple utility to validate objects against JSON Schemas */ @@ -32,8 +33,27 @@ export class SchemaValidator { const validate = ajValidator.compile(schema); const valid = validate(data); if (!valid && validate.errors) { - return ajValidator.errorsText(validate.errors, { dataVar: 'params' }); + // Find any True or False values and lowercase them + fixBooleanCasing(data as Record); + + const validate = ajValidator.compile(schema); + const valid = validate(data); + + if (!valid && validate.errors) { + return ajValidator.errorsText(validate.errors, { dataVar: 'params' }); + } } return null; } } + +function fixBooleanCasing(data: Record) { + for (const key of Object.keys(data)) { + if (!(key in data)) continue; + + if (typeof data[key] === 'object') { + fixBooleanCasing(data[key] as Record); + } else if (data[key] === 'True') data[key] = 'true'; + else if (data[key] === 'False') data[key] = 'false'; + } +}