fix: add type validation for tool parameters (#45)

Co-authored-by: Letta <noreply@letta.com>

src/tests/tools/ls.test.ts

@@ -51,4 +51,58 @@ describe("LS tool", () => {
       /missing required parameter.*path/,
     );
   });
+
+  test("filters files using ignore parameter (array)", async () => {
+    testDir = new TestDirectory();
+    testDir.createFile("file1.txt", "");
+    testDir.createFile("file2.log", "");
+    testDir.createFile("important.txt", "");
+    testDir.createDir("node_modules");
+
+    const result = await ls({
+      path: testDir.path,
+      ignore: ["*.log", "node_modules"],
+    });
+
+    expect(result.content[0].text).toContain("file1.txt");
+    expect(result.content[0].text).toContain("important.txt");
+    expect(result.content[0].text).not.toContain("file2.log");
+    expect(result.content[0].text).not.toContain("node_modules");
+  });
+
+  test("throws error when ignore is a string instead of array", async () => {
+    testDir = new TestDirectory();
+    testDir.createFile("file.txt", "");
+
+    await expect(
+      ls({
+        path: testDir.path,
+        ignore: '["*.log", "node_modules"]' as unknown as string[],
+      }),
+    ).rejects.toThrow(/must be an array/);
+  });
+
+  test("throws error when ignore is a number", async () => {
+    testDir = new TestDirectory();
+    testDir.createFile("file.txt", "");
+
+    await expect(
+      ls({
+        path: testDir.path,
+        ignore: 123 as unknown as string[],
+      }),
+    ).rejects.toThrow(/must be an array/);
+  });
+
+  test("throws error when ignore is an object", async () => {
+    testDir = new TestDirectory();
+    testDir.createFile("file.txt", "");
+
+    await expect(
+      ls({
+        path: testDir.path,
+        ignore: { pattern: "*.log" } as unknown as string[],
+      }),
+    ).rejects.toThrow(/must be an array/);
+  });
 });

src/tests/tools/validation.test.ts

@@ -0,0 +1,188 @@
+import { describe, expect, test } from "bun:test";
+import {
+  validateParamTypes,
+  validateRequiredParams,
+} from "../../tools/impl/validation";
+
+describe("validateRequiredParams", () => {
+  test("passes when all required params are present", () => {
+    expect(() => {
+      validateRequiredParams(
+        { path: "/test", name: "file" },
+        ["path", "name"],
+        "TestTool",
+      );
+    }).not.toThrow();
+  });
+
+  test("throws when required param is missing", () => {
+    expect(() => {
+      validateRequiredParams({ name: "file" }, ["path", "name"], "TestTool");
+    }).toThrow(/missing required parameter.*path/);
+  });
+
+  test("throws with multiple missing params", () => {
+    expect(() => {
+      validateRequiredParams({}, ["path", "name"], "TestTool");
+    }).toThrow(/missing required parameters.*path.*name/);
+  });
+});
+
+describe("validateParamTypes", () => {
+  test("passes when string param has correct type", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        path: { type: "string" },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ path: "/test" }, schema, "TestTool");
+    }).not.toThrow();
+  });
+
+  test("passes when array param has correct type", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        ignore: {
+          type: "array",
+          items: { type: "string" },
+        },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ ignore: ["*.log"] }, schema, "TestTool");
+    }).not.toThrow();
+  });
+
+  test("throws when array param is given as string", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        ignore: {
+          type: "array",
+          items: { type: "string" },
+        },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ ignore: '["*.log"]' }, schema, "TestTool");
+    }).toThrow(/must be an array.*received string/);
+  });
+
+  test("throws when string param is given as number", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        path: { type: "string" },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ path: 123 }, schema, "TestTool");
+    }).toThrow(/must be a string.*received integer/);
+  });
+
+  test("throws when object param is given as string", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        config: { type: "object" },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ config: '{"key": "value"}' }, schema, "TestTool");
+    }).toThrow(/must be an object.*received string/);
+  });
+
+  test("throws when boolean param is given as string", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        enabled: { type: "boolean" },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ enabled: "true" }, schema, "TestTool");
+    }).toThrow(/must be a boolean.*received string/);
+  });
+
+  test("allows optional params to be undefined", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        path: { type: "string" },
+        ignore: { type: "array" },
+      },
+      required: ["path"],
+    };
+
+    expect(() => {
+      validateParamTypes({ path: "/test" }, schema, "TestTool");
+    }).not.toThrow();
+  });
+
+  test("throws with clear error message indicating expected vs received type", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        ignore: {
+          type: "array",
+          items: { type: "string" },
+        },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes({ ignore: "not-an-array" }, schema, "TestTool");
+    }).toThrow(
+      /TestTool: Parameter 'ignore' must be an array, received string/,
+    );
+  });
+
+  test("validates array item types", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        items: {
+          type: "array",
+          items: { type: "string" },
+        },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes(
+        { items: ["valid", 123, "string"] },
+        schema,
+        "TestTool",
+      );
+    }).toThrow(/items\[1\].*must be a string.*received integer/);
+  });
+
+  test("passes when array items have correct types", () => {
+    const schema = {
+      type: "object",
+      properties: {
+        items: {
+          type: "array",
+          items: { type: "string" },
+        },
+      },
+    };
+
+    expect(() => {
+      validateParamTypes(
+        { items: ["all", "strings", "here"] },
+        schema,
+        "TestTool",
+      );
+    }).not.toThrow();
+  });
+});

src/tools/impl/LS.ts

@@ -1,7 +1,8 @@
 import { readdir, stat } from "node:fs/promises";
 import { join, resolve } from "node:path";
 import picomatch from "picomatch";
-import { validateRequiredParams } from "./validation.js";
+import LSSchema from "../schemas/LS.json";
+import { validateParamTypes, validateRequiredParams } from "./validation.js";
 
 interface LSArgs {
   path: string;
@@ -17,6 +18,7 @@ export async function ls(
   args: LSArgs,
 ): Promise<{ content: Array<{ type: string; text: string }> }> {
   validateRequiredParams(args, ["path"], "LS");
+  validateParamTypes(args, LSSchema, "LS");
   const { path: inputPath, ignore = [] } = args;
   const dirPath = resolve(inputPath);
   try {

src/tools/impl/validation.ts

@@ -12,3 +12,77 @@ export function validateRequiredParams<T extends object>(
     );
   }
 }
+
+interface JsonSchema {
+  type?: string;
+  properties?: Record<string, JsonSchema>;
+  items?: JsonSchema;
+  required?: string[];
+  [key: string]: unknown;
+}
+
+/**
+ * Validates that parameter values match their expected types from the JSON schema.
+ * Throws a clear error if types don't match.
+ */
+export function validateParamTypes(
+  args: Record<string, unknown>,
+  schema: JsonSchema,
+  toolName: string,
+): void {
+  if (!schema.properties) return;
+
+  for (const [paramName, paramSchema] of Object.entries(schema.properties)) {
+    const value = args[paramName];
+
+    // Skip undefined optional parameters
+    if (value === undefined) continue;
+
+    const expectedType = paramSchema.type;
+    if (!expectedType) continue;
+
+    const actualType = getJsonSchemaType(value);
+
+    if (actualType !== expectedType) {
+      const article = ["array", "object", "integer"].includes(expectedType)
+        ? "an"
+        : "a";
+      throw new Error(
+        `${toolName}: Parameter '${paramName}' must be ${article} ${expectedType}, received ${actualType}`,
+      );
+    }
+
+    // Additional validation for arrays to ensure they contain the right element types
+    if (expectedType === "array" && Array.isArray(value) && paramSchema.items) {
+      const itemType = paramSchema.items.type;
+      if (itemType) {
+        for (let i = 0; i < value.length; i++) {
+          const itemActualType = getJsonSchemaType(value[i]);
+          if (itemActualType !== itemType) {
+            const article = ["array", "object", "integer"].includes(itemType)
+              ? "an"
+              : "a";
+            throw new Error(
+              `${toolName}: Parameter '${paramName}[${i}]' must be ${article} ${itemType}, received ${itemActualType}`,
+            );
+          }
+        }
+      }
+    }
+  }
+}
+
+/**
+ * Gets the JSON Schema type name for a JavaScript value.
+ */
+function getJsonSchemaType(value: unknown): string {
+  if (value === null) return "null";
+  if (Array.isArray(value)) return "array";
+  if (typeof value === "object") return "object";
+  if (typeof value === "boolean") return "boolean";
+  if (typeof value === "number") {
+    return Number.isInteger(value) ? "integer" : "number";
+  }
+  if (typeof value === "string") return "string";
+  return "unknown";
+}