Merge pull request #1284 from tt-a1i/fix/boolean-string-coercion

fix(core): coerce string boolean values in schema validation

Author: Mingholy

packages/core/src/tools/shell.test.ts

@@ -169,6 +169,44 @@ describe('ShellTool', () => {
       });
       expect(invocation.getDescription()).not.toContain('[background]');
     });
+
+    describe('is_background parameter coercion', () => {
+      it('should accept string "true" as boolean true', () => {
+        const invocation = shellTool.build({
+          command: 'npm run dev',
+          is_background: 'true' as unknown as boolean,
+        });
+        expect(invocation).toBeDefined();
+        expect(invocation.getDescription()).toContain('[background]');
+      });
+
+      it('should accept string "false" as boolean false', () => {
+        const invocation = shellTool.build({
+          command: 'npm run build',
+          is_background: 'false' as unknown as boolean,
+        });
+        expect(invocation).toBeDefined();
+        expect(invocation.getDescription()).not.toContain('[background]');
+      });
+
+      it('should accept string "True" as boolean true', () => {
+        const invocation = shellTool.build({
+          command: 'npm run dev',
+          is_background: 'True' as unknown as boolean,
+        });
+        expect(invocation).toBeDefined();
+        expect(invocation.getDescription()).toContain('[background]');
+      });
+
+      it('should accept string "False" as boolean false', () => {
+        const invocation = shellTool.build({
+          command: 'npm run build',
+          is_background: 'False' as unknown as boolean,
+        });
+        expect(invocation).toBeDefined();
+        expect(invocation.getDescription()).not.toContain('[background]');
+      });
+    });
   });
 
   describe('execute', () => {

packages/core/src/utils/schemaValidator.test.ts

@@ -122,4 +122,91 @@ describe('SchemaValidator', () => {
     };
     expect(SchemaValidator.validate(schema, params)).not.toBeNull();
   });
+
+  describe('boolean string coercion', () => {
+    const booleanSchema = {
+      type: 'object',
+      properties: {
+        is_background: {
+          type: 'boolean',
+        },
+      },
+      required: ['is_background'],
+    };
+
+    it('should coerce string "true" to boolean true', () => {
+      const params = { is_background: 'true' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(true);
+    });
+
+    it('should coerce string "True" to boolean true', () => {
+      const params = { is_background: 'True' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(true);
+    });
+
+    it('should coerce string "TRUE" to boolean true', () => {
+      const params = { is_background: 'TRUE' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(true);
+    });
+
+    it('should coerce string "false" to boolean false', () => {
+      const params = { is_background: 'false' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(false);
+    });
+
+    it('should coerce string "False" to boolean false', () => {
+      const params = { is_background: 'False' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(false);
+    });
+
+    it('should coerce string "FALSE" to boolean false', () => {
+      const params = { is_background: 'FALSE' };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(false);
+    });
+
+    it('should handle nested objects with string booleans', () => {
+      const nestedSchema = {
+        type: 'object',
+        properties: {
+          options: {
+            type: 'object',
+            properties: {
+              enabled: { type: 'boolean' },
+            },
+          },
+        },
+      };
+      const params = { options: { enabled: 'true' } };
+      expect(SchemaValidator.validate(nestedSchema, params)).toBeNull();
+      expect((params.options as unknown as { enabled: boolean }).enabled).toBe(
+        true,
+      );
+    });
+
+    it('should not affect non-boolean strings', () => {
+      const mixedSchema = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+          is_active: { type: 'boolean' },
+        },
+      };
+      const params = { name: 'trueman', is_active: 'true' };
+      expect(SchemaValidator.validate(mixedSchema, params)).toBeNull();
+      expect(params.name).toBe('trueman');
+      expect(params.is_active).toBe(true);
+    });
+
+    it('should pass through actual boolean values unchanged', () => {
+      const params = { is_background: true };
+      expect(SchemaValidator.validate(booleanSchema, params)).toBeNull();
+      expect(params.is_background).toBe(true);
+    });
+  });
 });

packages/core/src/utils/schemaValidator.ts

@@ -41,14 +41,12 @@ export class SchemaValidator {
       return 'Value of params must be an object';
     }
     const validate = ajValidator.compile(schema);
-    const valid = validate(data);
+    let valid = validate(data);
     if (!valid && validate.errors) {
-      // Find any True or False values and lowercase them
-      fixBooleanCasing(data as Record<string, unknown>);
-
-      const validate = ajValidator.compile(schema);
-      const valid = validate(data);
+      // Coerce string boolean values ("true"/"false") to actual booleans
+      fixBooleanValues(data as Record<string, unknown>);
 
+      valid = validate(data);
       if (!valid && validate.errors) {
         return ajValidator.errorsText(validate.errors, { dataVar: 'params' });
       }
@@ -57,13 +55,29 @@ export class SchemaValidator {
   }
 }
 
-function fixBooleanCasing(data: Record<string, unknown>) {
+/**
+ * Coerces string boolean values to actual booleans.
+ * This handles cases where LLMs return "true"/"false" strings instead of boolean values,
+ * which is common with self-hosted LLMs.
+ *
+ * Converts:
+ * - "true", "True", "TRUE" -> true
+ * - "false", "False", "FALSE" -> false
+ */
+function fixBooleanValues(data: Record<string, unknown>) {
   for (const key of Object.keys(data)) {
     if (!(key in data)) continue;
+    const value = data[key];
 
-    if (typeof data[key] === 'object') {
-      fixBooleanCasing(data[key] as Record<string, unknown>);
-    } else if (data[key] === 'True') data[key] = 'true';
-    else if (data[key] === 'False') data[key] = 'false';
+    if (typeof value === 'object' && value !== null) {
+      fixBooleanValues(value as Record<string, unknown>);
+    } else if (typeof value === 'string') {
+      const lower = value.toLowerCase();
+      if (lower === 'true') {
+        data[key] = true;
+      } else if (lower === 'false') {
+        data[key] = false;
+      }
+    }
   }
 }