From 11c215103c5a87feee3f0bfb8056b8ec15dda593 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 27 Oct 2025 14:18:05 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A4=96=20Fix=20ENOENT=20error=20in=20?= =?UTF-8?q?todo=5Fwrite=20for=20SSH=20runtimes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure temp directory exists before writing todos.json. Previously, writeTodos() assumed the directory already existed, but SSH runtime initialization could fail to create it, resulting in ENOENT errors. Added fs.mkdir() with recursive: true to create the directory if needed. --- src/services/tools/todo.test.ts | 31 +++++++++++++++++++++++++++++++ src/services/tools/todo.ts | 2 ++ 2 files changed, 33 insertions(+) diff --git a/src/services/tools/todo.test.ts b/src/services/tools/todo.test.ts index 206d13a72..bffb4bb26 100644 --- a/src/services/tools/todo.test.ts +++ b/src/services/tools/todo.test.ts @@ -209,6 +209,37 @@ describe("Todo Storage", () => { await setTodosForTempDir(runtimeTempDir, todos); expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos); }); + + it("should create directory if it doesn't exist", async () => { + // Use a non-existent nested directory path + const nonExistentDir = path.join(os.tmpdir(), "todo-nonexistent-test", "nested", "path"); + + try { + const todos: TodoItem[] = [ + { + content: "Test task", + status: "pending", + }, + ]; + + // Should not throw even though directory doesn't exist + await setTodosForTempDir(nonExistentDir, todos); + + // Verify the file was created and is readable + const retrievedTodos = await getTodosForTempDir(nonExistentDir); + expect(retrievedTodos).toEqual(todos); + + // Verify the directory was actually created + const dirStats = await fs.stat(nonExistentDir); + expect(dirStats.isDirectory()).toBe(true); + } finally { + // Clean up the created directory + await fs.rm(path.join(os.tmpdir(), "todo-nonexistent-test"), { + recursive: true, + force: true, + }); + } + }); }); describe("getTodosForTempDir", () => { diff --git a/src/services/tools/todo.ts b/src/services/tools/todo.ts index 8794c46ce..276f334fb 100644 --- a/src/services/tools/todo.ts +++ b/src/services/tools/todo.ts @@ -104,6 +104,8 @@ function validateTodos(todos: TodoItem[]): void { async function writeTodos(tempDir: string, todos: TodoItem[]): Promise { validateTodos(todos); const todoFile = getTodoFilePath(tempDir); + // Ensure directory exists before writing (SSH runtime might not have created it yet) + await fs.mkdir(tempDir, { recursive: true }); await fs.writeFile(todoFile, JSON.stringify(todos, null, 2), "utf-8"); } From 451ae4d8ebc1fa464343853d852ac7721654b4a7 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 27 Oct 2025 14:23:11 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A4=96=20Use=20runtime=20abstraction?= =?UTF-8?q?=20for=20todo=20tool=20file=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace direct fs operations with runtime abstraction to support SSH runtimes. Key changes: - readTodos/writeTodos now use readFileString/writeFileString helpers - Directory creation uses runtime.exec('mkdir -p') instead of fs.mkdir - Test helpers updated to accept Runtime parameter - All tests updated to use createRuntime() for local testing This enables todo persistence to work correctly on both local and SSH runtimes. --- src/services/tools/todo.test.ts | 69 ++++++++++++++++++--------------- src/services/tools/todo.ts | 37 ++++++++++-------- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/services/tools/todo.test.ts b/src/services/tools/todo.test.ts index bffb4bb26..fded1c4da 100644 --- a/src/services/tools/todo.test.ts +++ b/src/services/tools/todo.test.ts @@ -4,13 +4,18 @@ import * as os from "os"; import * as path from "path"; import { clearTodosForTempDir, getTodosForTempDir, setTodosForTempDir } from "./todo"; import type { TodoItem } from "@/types/tools"; +import type { Runtime } from "@/runtime/Runtime"; +import { createRuntime } from "@/runtime/runtimeFactory"; describe("Todo Storage", () => { let runtimeTempDir: string; + let runtime: Runtime; beforeEach(async () => { // Create a temporary directory for each test runtimeTempDir = await fs.mkdtemp(path.join(os.tmpdir(), "todo-test-")); + // Create a local runtime for testing + runtime = createRuntime({ type: "local", srcBaseDir: "/tmp" }); }); afterEach(async () => { @@ -35,9 +40,9 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, todos); + await setTodosForTempDir(runtime, runtimeTempDir, todos); - const storedTodos = await getTodosForTempDir(runtimeTempDir); + const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir); expect(storedTodos).toEqual(todos); }); @@ -54,7 +59,7 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, initialTodos); + await setTodosForTempDir(runtime, runtimeTempDir, initialTodos); // Replace with updated list const updatedTodos: TodoItem[] = [ @@ -72,16 +77,16 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, updatedTodos); + await setTodosForTempDir(runtime, runtimeTempDir, updatedTodos); // Verify list was replaced, not merged - const storedTodos = await getTodosForTempDir(runtimeTempDir); + const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir); expect(storedTodos).toEqual(updatedTodos); }); it("should handle empty todo list", async () => { // Create initial list - await setTodosForTempDir(runtimeTempDir, [ + await setTodosForTempDir(runtime, runtimeTempDir, [ { content: "Task 1", status: "pending", @@ -89,9 +94,9 @@ describe("Todo Storage", () => { ]); // Clear list - await setTodosForTempDir(runtimeTempDir, []); + await setTodosForTempDir(runtime, runtimeTempDir, []); - const storedTodos = await getTodosForTempDir(runtimeTempDir); + const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir); expect(storedTodos).toEqual([]); }); @@ -108,10 +113,10 @@ describe("Todo Storage", () => { { content: "Task 8", status: "pending" }, ]; - await expect(setTodosForTempDir(runtimeTempDir, tooManyTodos)).rejects.toThrow( + await expect(setTodosForTempDir(runtime, runtimeTempDir, tooManyTodos)).rejects.toThrow( /Too many TODOs \(8\/7\)/i ); - await expect(setTodosForTempDir(runtimeTempDir, tooManyTodos)).rejects.toThrow( + await expect(setTodosForTempDir(runtime, runtimeTempDir, tooManyTodos)).rejects.toThrow( /Keep high precision at the center/i ); }); @@ -127,8 +132,8 @@ describe("Todo Storage", () => { { content: "Future work (5 items)", status: "pending" }, ]; - await setTodosForTempDir(runtimeTempDir, maxTodos); - expect(await getTodosForTempDir(runtimeTempDir)).toEqual(maxTodos); + await setTodosForTempDir(runtime, runtimeTempDir, maxTodos); + expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(maxTodos); }); it("should reject multiple in_progress tasks", async () => { @@ -139,7 +144,7 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, validTodos); + await setTodosForTempDir(runtime, runtimeTempDir, validTodos); const invalidTodos: TodoItem[] = [ { @@ -152,12 +157,12 @@ describe("Todo Storage", () => { }, ]; - await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow( + await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow( /only one task can be marked as in_progress/i ); // Original todos should remain unchanged on failure - expect(await getTodosForTempDir(runtimeTempDir)).toEqual(validTodos); + expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(validTodos); }); it("should reject when in_progress tasks appear after pending", async () => { @@ -172,7 +177,7 @@ describe("Todo Storage", () => { }, ]; - await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow( + await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow( /in-progress tasks must appear before pending tasks/i ); }); @@ -189,7 +194,7 @@ describe("Todo Storage", () => { }, ]; - await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow( + await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow( /completed tasks must appear before in-progress or pending tasks/i ); }); @@ -206,8 +211,8 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, todos); - expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos); + await setTodosForTempDir(runtime, runtimeTempDir, todos); + expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(todos); }); it("should create directory if it doesn't exist", async () => { @@ -223,10 +228,10 @@ describe("Todo Storage", () => { ]; // Should not throw even though directory doesn't exist - await setTodosForTempDir(nonExistentDir, todos); + await setTodosForTempDir(runtime, nonExistentDir, todos); // Verify the file was created and is readable - const retrievedTodos = await getTodosForTempDir(nonExistentDir); + const retrievedTodos = await getTodosForTempDir(runtime, nonExistentDir); expect(retrievedTodos).toEqual(todos); // Verify the directory was actually created @@ -244,7 +249,7 @@ describe("Todo Storage", () => { describe("getTodosForTempDir", () => { it("should return empty array when no todos exist", async () => { - const todos = await getTodosForTempDir(runtimeTempDir); + const todos = await getTodosForTempDir(runtime, runtimeTempDir); expect(todos).toEqual([]); }); @@ -260,9 +265,9 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, todos); + await setTodosForTempDir(runtime, runtimeTempDir, todos); - const retrievedTodos = await getTodosForTempDir(runtimeTempDir); + const retrievedTodos = await getTodosForTempDir(runtime, runtimeTempDir); expect(retrievedTodos).toEqual(todos); }); }); @@ -288,12 +293,12 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(tempDir1, todos1); - await setTodosForTempDir(tempDir2, todos2); + await setTodosForTempDir(runtime, tempDir1, todos1); + await setTodosForTempDir(runtime, tempDir2, todos2); // Verify each temp directory has its own todos - const retrievedTodos1 = await getTodosForTempDir(tempDir1); - const retrievedTodos2 = await getTodosForTempDir(tempDir2); + const retrievedTodos1 = await getTodosForTempDir(runtime, tempDir1); + const retrievedTodos2 = await getTodosForTempDir(runtime, tempDir2); expect(retrievedTodos1).toEqual(todos1); expect(retrievedTodos2).toEqual(todos2); @@ -314,11 +319,11 @@ describe("Todo Storage", () => { }, ]; - await setTodosForTempDir(runtimeTempDir, todos); - expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos); + await setTodosForTempDir(runtime, runtimeTempDir, todos); + expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(todos); - await clearTodosForTempDir(runtimeTempDir); - expect(await getTodosForTempDir(runtimeTempDir)).toEqual([]); + await clearTodosForTempDir(runtime, runtimeTempDir); + expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual([]); }); }); }); diff --git a/src/services/tools/todo.ts b/src/services/tools/todo.ts index 276f334fb..77bbd49f3 100644 --- a/src/services/tools/todo.ts +++ b/src/services/tools/todo.ts @@ -1,10 +1,11 @@ import { tool } from "ai"; -import * as fs from "fs/promises"; import * as path from "path"; +import type { Runtime } from "@/runtime/Runtime"; import type { ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; import type { TodoItem } from "@/types/tools"; import { MAX_TODOS } from "@/constants/toolLimits"; +import { readFileString, writeFileString, execBuffered } from "@/utils/runtime/helpers"; /** * Get path to todos.json file in the stream's temporary directory @@ -14,12 +15,12 @@ function getTodoFilePath(tempDir: string): string { } /** - * Read todos from filesystem + * Read todos from filesystem using runtime abstraction */ -async function readTodos(tempDir: string): Promise { +async function readTodos(runtime: Runtime, tempDir: string): Promise { const todoFile = getTodoFilePath(tempDir); try { - const content = await fs.readFile(todoFile, "utf-8"); + const content = await readFileString(runtime, todoFile); return JSON.parse(content) as TodoItem[]; } catch { // File doesn't exist yet or is invalid @@ -99,14 +100,14 @@ function validateTodos(todos: TodoItem[]): void { } /** - * Write todos to filesystem + * Write todos to filesystem using runtime abstraction */ -async function writeTodos(tempDir: string, todos: TodoItem[]): Promise { +async function writeTodos(runtime: Runtime, tempDir: string, todos: TodoItem[]): Promise { validateTodos(todos); const todoFile = getTodoFilePath(tempDir); // Ensure directory exists before writing (SSH runtime might not have created it yet) - await fs.mkdir(tempDir, { recursive: true }); - await fs.writeFile(todoFile, JSON.stringify(todos, null, 2), "utf-8"); + await execBuffered(runtime, `mkdir -p "${tempDir}"`, { cwd: "/", timeout: 10 }); + await writeFileString(runtime, todoFile, JSON.stringify(todos, null, 2)); } /** @@ -118,7 +119,7 @@ export const createTodoWriteTool: ToolFactory = (config) => { description: TOOL_DEFINITIONS.todo_write.description, inputSchema: TOOL_DEFINITIONS.todo_write.schema, execute: async ({ todos }) => { - await writeTodos(config.runtimeTempDir, todos); + await writeTodos(config.runtime, config.runtimeTempDir, todos); return { success: true as const, count: todos.length, @@ -136,7 +137,7 @@ export const createTodoReadTool: ToolFactory = (config) => { description: TOOL_DEFINITIONS.todo_read.description, inputSchema: TOOL_DEFINITIONS.todo_read.schema, execute: async () => { - const todos = await readTodos(config.runtimeTempDir); + const todos = await readTodos(config.runtime, config.runtimeTempDir); return { todos, }; @@ -147,24 +148,28 @@ export const createTodoReadTool: ToolFactory = (config) => { /** * Set todos for a temp directory (useful for testing) */ -export async function setTodosForTempDir(tempDir: string, todos: TodoItem[]): Promise { - await writeTodos(tempDir, todos); +export async function setTodosForTempDir( + runtime: Runtime, + tempDir: string, + todos: TodoItem[] +): Promise { + await writeTodos(runtime, tempDir, todos); } /** * Get todos for a temp directory (useful for testing) */ -export async function getTodosForTempDir(tempDir: string): Promise { - return readTodos(tempDir); +export async function getTodosForTempDir(runtime: Runtime, tempDir: string): Promise { + return readTodos(runtime, tempDir); } /** * Clear todos for a temp directory (useful for testing and cleanup) */ -export async function clearTodosForTempDir(tempDir: string): Promise { +export async function clearTodosForTempDir(runtime: Runtime, tempDir: string): Promise { const todoFile = getTodoFilePath(tempDir); try { - await fs.unlink(todoFile); + await execBuffered(runtime, `rm -f "${todoFile}"`, { cwd: "/", timeout: 10 }); } catch { // File doesn't exist, nothing to clear }