fix: add parameter validation to all tools to prevent undefined insertions (#14)

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

.gitignore

@@ -4,6 +4,7 @@ bin/
 letta.js
 letta.js.map
 .DS_Store
+.letta
 
 # Logs
 logs

src/tests/tools/bash.test.ts

@@ -78,4 +78,10 @@ describe("Bash tool", () => {
     expect(result.content).toBeDefined();
     expect(result.content[0].text).toBeDefined();
   });
+
+  test("throws error when command is missing", async () => {
+    await expect(bash({} as any)).rejects.toThrow(
+      /missing required parameter.*command/,
+    );
+  });
 });

src/tests/tools/edit.test.ts

@@ -65,4 +65,50 @@ describe("Edit tool", () => {
     expect(readFileSync(file, "utf-8")).toBe("qux bar qux baz qux");
     expect(result.replacements).toBe(3);
   });
+
+  test("throws error when file_path is missing", async () => {
+    await expect(
+      edit({
+        old_string: "foo",
+        new_string: "bar",
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*file_path/);
+  });
+
+  test("throws error when old_string is missing", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "Hello, World!");
+
+    await expect(
+      edit({
+        file_path: file,
+        new_string: "bar",
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*old_string/);
+  });
+
+  test("throws error when new_string is missing", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "Hello, World!");
+
+    await expect(
+      edit({
+        file_path: file,
+        old_string: "foo",
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*new_string/);
+  });
+
+  test("throws error when using typo'd parameter name (new_str instead of new_string)", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "Hello, World!");
+
+    await expect(
+      edit({
+        file_path: file,
+        old_string: "World",
+        new_str: "Bun",
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*new_string/);
+  });
 });

src/tests/tools/grep.test.ts

@@ -56,4 +56,10 @@ describe("Grep tool", () => {
       }
     }
   });
+
+  test("throws error when pattern is missing", async () => {
+    await expect(grep({} as any)).rejects.toThrow(
+      /missing required parameter.*pattern/,
+    );
+  });
 });

src/tests/tools/ls.test.ts

@@ -45,4 +45,10 @@ describe("LS tool", () => {
 
     await expect(ls({ path: file })).rejects.toThrow(/Not a directory/);
   });
+
+  test("throws error when path is missing", async () => {
+    await expect(ls({} as any)).rejects.toThrow(
+      /missing required parameter.*path/,
+    );
+  });
 });

src/tests/tools/multiedit.test.ts

@@ -40,4 +40,59 @@ describe("MultiEdit tool", () => {
     expect(readFileSync(file, "utf-8")).toBe("xxx yyy");
     expect(result.edits_applied).toBe(2);
   });
+
+  test("throws error when file_path is missing", async () => {
+    await expect(
+      multi_edit({
+        edits: [{ old_string: "foo", new_string: "bar" }],
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*file_path/);
+  });
+
+  test("throws error when edits is missing", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "foo bar");
+
+    await expect(
+      multi_edit({
+        file_path: file,
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*edits/);
+  });
+
+  test("throws error when an edit is missing old_string", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "foo bar");
+
+    await expect(
+      multi_edit({
+        file_path: file,
+        edits: [{ new_string: "bar" } as any],
+      }),
+    ).rejects.toThrow(/missing required parameter.*old_string/);
+  });
+
+  test("throws error when an edit is missing new_string", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "foo bar");
+
+    await expect(
+      multi_edit({
+        file_path: file,
+        edits: [{ old_string: "foo" } as any],
+      }),
+    ).rejects.toThrow(/missing required parameter.*new_string/);
+  });
+
+  test("throws error when using typo'd parameter in edit (new_str instead of new_string)", async () => {
+    testDir = new TestDirectory();
+    const file = testDir.createFile("test.txt", "foo bar");
+
+    await expect(
+      multi_edit({
+        file_path: file,
+        edits: [{ old_string: "foo", new_str: "baz" } as any],
+      }),
+    ).rejects.toThrow(/missing required parameter.*new_string/);
+  });
 });

src/tests/tools/read.test.ts

@@ -102,4 +102,10 @@ export default box;
     expect(result.content).toContain("│ Header  │");
     expect(result.content).toContain("TypeScript file");
   });
+
+  test("throws error when file_path is missing", async () => {
+    await expect(read({} as any)).rejects.toThrow(
+      /missing required parameter.*file_path/,
+    );
+  });
 });

src/tests/tools/write.test.ts

@@ -48,4 +48,23 @@ describe("Write tool", () => {
 
     expect(readFileSync(filePath, "utf-8")).toBe(content);
   });
+
+  test("throws error when file_path is missing", async () => {
+    await expect(
+      write({
+        content: "Hello",
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*file_path/);
+  });
+
+  test("throws error when content is missing", async () => {
+    testDir = new TestDirectory();
+    const filePath = testDir.resolve("test.txt");
+
+    await expect(
+      write({
+        file_path: filePath,
+      } as any),
+    ).rejects.toThrow(/missing required parameter.*content/);
+  });
 });

src/tools/impl/Bash.ts

@@ -2,6 +2,7 @@ import type { ExecOptions } from "node:child_process";
 import { exec, spawn } from "node:child_process";
 import { promisify } from "node:util";
 import { backgroundProcesses, getNextBashId } from "./process_manager.js";
+import { validateRequiredParams } from "./validation.js";
 
 const execAsync = promisify(exec);
 
@@ -21,6 +22,7 @@ interface BashResult {
 }
 
 export async function bash(args: BashArgs): Promise<BashResult> {
+  validateRequiredParams(args, ["command"], "Bash");
   const {
     command,
     timeout = 120000,

src/tools/impl/BashOutput.ts

@@ -1,4 +1,5 @@
 import { backgroundProcesses } from "./process_manager.js";
+import { validateRequiredParams } from "./validation.js";
 
 interface BashOutputArgs {
   bash_id: string;
@@ -11,6 +12,7 @@ interface BashOutputResult {
 export async function bash_output(
   args: BashOutputArgs,
 ): Promise<BashOutputResult> {
+  validateRequiredParams(args, ["bash_id"], "BashOutput");
   const { bash_id, filter } = args;
   const proc = backgroundProcesses.get(bash_id);
   if (!proc)

src/tools/impl/Edit.ts

@@ -1,5 +1,6 @@
 import { promises as fs } from "node:fs";
 import * as path from "node:path";
+import { validateRequiredParams } from "./validation.js";
 
 interface EditArgs {
   file_path: string;
@@ -13,6 +14,11 @@ interface EditResult {
 }
 
 export async function edit(args: EditArgs): Promise<EditResult> {
+  validateRequiredParams(
+    args,
+    ["file_path", "old_string", "new_string"],
+    "Edit",
+  );
   const { file_path, old_string, new_string, replace_all = false } = args;
   if (!path.isAbsolute(file_path))
     throw new Error(`File path must be absolute, got: ${file_path}`);

src/tools/impl/ExitPlanMode.ts

@@ -3,6 +3,8 @@
  * Exits plan mode by presenting the plan to the user for approval
  */
 
+import { validateRequiredParams } from "./validation.js";
+
 interface ExitPlanModeArgs {
   plan: string;
 }
@@ -10,6 +12,7 @@ interface ExitPlanModeArgs {
 export async function exit_plan_mode(
   args: ExitPlanModeArgs,
 ): Promise<{ message: string }> {
+  validateRequiredParams(args, ["plan"], "ExitPlanMode");
   const { plan: _plan } = args;
 
   // Return confirmation message that plan was approved

src/tools/impl/Glob.ts

@@ -1,6 +1,7 @@
 import { promises as fs } from "node:fs";
 import * as path from "node:path";
 import picomatch from "picomatch";
+import { validateRequiredParams } from "./validation.js";
 
 interface GlobArgs {
   pattern: string;
@@ -32,6 +33,7 @@ async function walkDirectory(dir: string): Promise<string[]> {
 }
 
 export async function glob(args: GlobArgs): Promise<GlobResult> {
+  validateRequiredParams(args, ["pattern"], "Glob");
   const { pattern, path: searchPath } = args;
   const userCwd = process.env.USER_CWD || process.cwd();
   let baseDir: string;

src/tools/impl/Grep.ts

@@ -3,6 +3,7 @@ import { createRequire } from "node:module";
 import * as path from "node:path";
 import { fileURLToPath } from "node:url";
 import { promisify } from "node:util";
+import { validateRequiredParams } from "./validation.js";
 
 const execFileAsync = promisify(execFile);
 
@@ -40,6 +41,7 @@ interface GrepResult {
 }
 
 export async function grep(args: GrepArgs): Promise<GrepResult> {
+  validateRequiredParams(args, ["pattern"], "Grep");
   const {
     pattern,
     path: searchPath,

src/tools/impl/KillBash.ts

@@ -1,4 +1,5 @@
 import { backgroundProcesses } from "./process_manager.js";
+import { validateRequiredParams } from "./validation.js";
 
 interface KillBashArgs {
   shell_id: string;
@@ -8,6 +9,7 @@ interface KillBashResult {
 }
 
 export async function kill_bash(args: KillBashArgs): Promise<KillBashResult> {
+  validateRequiredParams(args, ["shell_id"], "KillBash");
   const { shell_id } = args;
   const proc = backgroundProcesses.get(shell_id);
   if (!proc) return { killed: false };

src/tools/impl/LS.ts

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

src/tools/impl/MultiEdit.ts

@@ -1,5 +1,6 @@
 import { promises as fs } from "node:fs";
 import * as path from "node:path";
+import { validateRequiredParams } from "./validation.js";
 
 interface Edit {
   old_string: string;
@@ -18,11 +19,17 @@ interface MultiEditResult {
 export async function multi_edit(
   args: MultiEditArgs,
 ): Promise<MultiEditResult> {
+  validateRequiredParams(args, ["file_path", "edits"], "MultiEdit");
   const { file_path, edits } = args;
   if (!path.isAbsolute(file_path))
     throw new Error(`File path must be absolute, got: ${file_path}`);
   if (!edits || edits.length === 0) throw new Error("No edits provided");
   for (let i = 0; i < edits.length; i++) {
+    validateRequiredParams(
+      edits[i] as Record<string, unknown>,
+      ["old_string", "new_string"],
+      `MultiEdit (edit ${i + 1})`,
+    );
     if (edits[i].old_string === edits[i].new_string)
       throw new Error(
         `Edit ${i + 1}: No changes to make: old_string and new_string are exactly the same.`,

src/tools/impl/Read.ts

@@ -1,5 +1,6 @@
 import { promises as fs } from "node:fs";
 import * as path from "node:path";
+import { validateRequiredParams } from "./validation.js";
 
 interface ReadArgs {
   file_path: string;
@@ -79,6 +80,7 @@ function formatWithLineNumbers(
 }
 
 export async function read(args: ReadArgs): Promise<ReadResult> {
+  validateRequiredParams(args, ["file_path"], "Read");
   const { file_path, offset, limit } = args;
   if (!path.isAbsolute(file_path))
     throw new Error(`File path must be absolute, got: ${file_path}`);

src/tools/impl/TodoWrite.ts

@@ -1,3 +1,5 @@
+import { validateRequiredParams } from "./validation.js";
+
 interface TodoItem {
   content: string;
   status: "pending" | "in_progress" | "completed";
@@ -14,6 +16,7 @@ interface TodoWriteResult {
 export async function todo_write(
   args: TodoWriteArgs,
 ): Promise<TodoWriteResult> {
+  validateRequiredParams(args, ["todos"], "TodoWrite");
   if (!args.todos || !Array.isArray(args.todos))
     throw new Error("todos must be an array");
   for (const todo of args.todos) {

src/tools/impl/Write.ts

@@ -1,5 +1,6 @@
 import { promises as fs } from "node:fs";
 import * as path from "node:path";
+import { validateRequiredParams } from "./validation.js";
 
 interface WriteArgs {
   file_path: string;
@@ -10,6 +11,7 @@ interface WriteResult {
 }
 
 export async function write(args: WriteArgs): Promise<WriteResult> {
+  validateRequiredParams(args, ["file_path", "content"], "Write");
   const { file_path, content } = args;
   if (!path.isAbsolute(file_path))
     throw new Error(`File path must be absolute, got: ${file_path}`);

src/tools/impl/validation.ts

@@ -0,0 +1,14 @@
+export function validateRequiredParams(
+  args: Record<string, unknown>,
+  required: string[],
+  toolName: string,
+): void {
+  const missing = required.filter((key) => args[key] === undefined);
+  if (missing.length > 0) {
+    const received = Object.keys(args).join(", ");
+    throw new Error(
+      `${toolName} tool missing required parameter${missing.length > 1 ? "s" : ""}: ${missing.join(", ")}. ` +
+        `Received parameters: ${received}`,
+    );
+  }
+}