Skip to content

Commit 141f03b

Browse files
ammar-agentammar-agent
authored
🤖 Fix ENOENT error in todo_write for SSH runtimes (#449)
## Problem SSH runtime agents encounter ENOENT errors when calling `todo_write`: ``` Error: ENOENT: no such file or directory, open '/root/.cmux-tmp/b29673ad/todos.json' ``` The root cause was that the todo tool was using Node's `fs` module directly, which only works on the local filesystem. For SSH runtimes, file operations must go through the runtime abstraction. ## Solution Refactored todo tool to use runtime abstraction for all file operations: - `readTodos`/`writeTodos` now use `readFileString`/`writeFileString` helpers from `@/utils/runtime/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({ type: 'local' })` ## Testing - Added test case that verifies directory creation when path doesn't exist - All 14 existing tests pass with runtime abstraction - Tests verify both local runtime behavior and proper isolation _Generated with `cmux`_
1 parent 842609c commit 141f03b

File tree

2 files changed

+88
-45
lines changed

2 files changed

+88
-45
lines changed

‎src/services/tools/todo.test.ts‎

Lines changed: 66 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import * as os from "os";
44
import * as path from "path";
55
import { clearTodosForTempDir, getTodosForTempDir, setTodosForTempDir } from "./todo";
66
import type { TodoItem } from "@/types/tools";
7+
import type { Runtime } from "@/runtime/Runtime";
8+
import { createRuntime } from "@/runtime/runtimeFactory";
79

810
describe("Todo Storage", () => {
911
let runtimeTempDir: string;
12+
let runtime: Runtime;
1013

1114
beforeEach(async () => {
1215
// Create a temporary directory for each test
1316
runtimeTempDir = await fs.mkdtemp(path.join(os.tmpdir(), "todo-test-"));
17+
// Create a local runtime for testing
18+
runtime = createRuntime({ type: "local", srcBaseDir: "/tmp" });
1419
});
1520

1621
afterEach(async () => {
@@ -35,9 +40,9 @@ describe("Todo Storage", () => {
3540
},
3641
];
3742

38-
await setTodosForTempDir(runtimeTempDir, todos);
43+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
3944

40-
const storedTodos = await getTodosForTempDir(runtimeTempDir);
45+
const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
4146
expect(storedTodos).toEqual(todos);
4247
});
4348

@@ -54,7 +59,7 @@ describe("Todo Storage", () => {
5459
},
5560
];
5661

57-
await setTodosForTempDir(runtimeTempDir, initialTodos);
62+
await setTodosForTempDir(runtime, runtimeTempDir, initialTodos);
5863

5964
// Replace with updated list
6065
const updatedTodos: TodoItem[] = [
@@ -72,26 +77,26 @@ describe("Todo Storage", () => {
7277
},
7378
];
7479

75-
await setTodosForTempDir(runtimeTempDir, updatedTodos);
80+
await setTodosForTempDir(runtime, runtimeTempDir, updatedTodos);
7681

7782
// Verify list was replaced, not merged
78-
const storedTodos = await getTodosForTempDir(runtimeTempDir);
83+
const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
7984
expect(storedTodos).toEqual(updatedTodos);
8085
});
8186

8287
it("should handle empty todo list", async () => {
8388
// Create initial list
84-
await setTodosForTempDir(runtimeTempDir, [
89+
await setTodosForTempDir(runtime, runtimeTempDir, [
8590
{
8691
content: "Task 1",
8792
status: "pending",
8893
},
8994
]);
9095

9196
// Clear list
92-
await setTodosForTempDir(runtimeTempDir, []);
97+
await setTodosForTempDir(runtime, runtimeTempDir, []);
9398

94-
const storedTodos = await getTodosForTempDir(runtimeTempDir);
99+
const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
95100
expect(storedTodos).toEqual([]);
96101
});
97102

@@ -108,10 +113,10 @@ describe("Todo Storage", () => {
108113
{ content: "Task 8", status: "pending" },
109114
];
110115

111-
await expect(setTodosForTempDir(runtimeTempDir, tooManyTodos)).rejects.toThrow(
116+
await expect(setTodosForTempDir(runtime, runtimeTempDir, tooManyTodos)).rejects.toThrow(
112117
/Too many TODOs \(8\/7\)/i
113118
);
114-
await expect(setTodosForTempDir(runtimeTempDir, tooManyTodos)).rejects.toThrow(
119+
await expect(setTodosForTempDir(runtime, runtimeTempDir, tooManyTodos)).rejects.toThrow(
115120
/Keep high precision at the center/i
116121
);
117122
});
@@ -127,8 +132,8 @@ describe("Todo Storage", () => {
127132
{ content: "Future work (5 items)", status: "pending" },
128133
];
129134

130-
await setTodosForTempDir(runtimeTempDir, maxTodos);
131-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual(maxTodos);
135+
await setTodosForTempDir(runtime, runtimeTempDir, maxTodos);
136+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(maxTodos);
132137
});
133138

134139
it("should reject multiple in_progress tasks", async () => {
@@ -139,7 +144,7 @@ describe("Todo Storage", () => {
139144
},
140145
];
141146

142-
await setTodosForTempDir(runtimeTempDir, validTodos);
147+
await setTodosForTempDir(runtime, runtimeTempDir, validTodos);
143148

144149
const invalidTodos: TodoItem[] = [
145150
{
@@ -152,12 +157,12 @@ describe("Todo Storage", () => {
152157
},
153158
];
154159

155-
await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow(
160+
await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow(
156161
/only one task can be marked as in_progress/i
157162
);
158163

159164
// Original todos should remain unchanged on failure
160-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual(validTodos);
165+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(validTodos);
161166
});
162167

163168
it("should reject when in_progress tasks appear after pending", async () => {
@@ -172,7 +177,7 @@ describe("Todo Storage", () => {
172177
},
173178
];
174179

175-
await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow(
180+
await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow(
176181
/in-progress tasks must appear before pending tasks/i
177182
);
178183
});
@@ -189,7 +194,7 @@ describe("Todo Storage", () => {
189194
},
190195
];
191196

192-
await expect(setTodosForTempDir(runtimeTempDir, invalidTodos)).rejects.toThrow(
197+
await expect(setTodosForTempDir(runtime, runtimeTempDir, invalidTodos)).rejects.toThrow(
193198
/completed tasks must appear before in-progress or pending tasks/i
194199
);
195200
});
@@ -206,14 +211,45 @@ describe("Todo Storage", () => {
206211
},
207212
];
208213

209-
await setTodosForTempDir(runtimeTempDir, todos);
210-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos);
214+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
215+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(todos);
216+
});
217+
218+
it("should create directory if it doesn't exist", async () => {
219+
// Use a non-existent nested directory path
220+
const nonExistentDir = path.join(os.tmpdir(), "todo-nonexistent-test", "nested", "path");
221+
222+
try {
223+
const todos: TodoItem[] = [
224+
{
225+
content: "Test task",
226+
status: "pending",
227+
},
228+
];
229+
230+
// Should not throw even though directory doesn't exist
231+
await setTodosForTempDir(runtime, nonExistentDir, todos);
232+
233+
// Verify the file was created and is readable
234+
const retrievedTodos = await getTodosForTempDir(runtime, nonExistentDir);
235+
expect(retrievedTodos).toEqual(todos);
236+
237+
// Verify the directory was actually created
238+
const dirStats = await fs.stat(nonExistentDir);
239+
expect(dirStats.isDirectory()).toBe(true);
240+
} finally {
241+
// Clean up the created directory
242+
await fs.rm(path.join(os.tmpdir(), "todo-nonexistent-test"), {
243+
recursive: true,
244+
force: true,
245+
});
246+
}
211247
});
212248
});
213249

214250
describe("getTodosForTempDir", () => {
215251
it("should return empty array when no todos exist", async () => {
216-
const todos = await getTodosForTempDir(runtimeTempDir);
252+
const todos = await getTodosForTempDir(runtime, runtimeTempDir);
217253
expect(todos).toEqual([]);
218254
});
219255

@@ -229,9 +265,9 @@ describe("Todo Storage", () => {
229265
},
230266
];
231267

232-
await setTodosForTempDir(runtimeTempDir, todos);
268+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
233269

234-
const retrievedTodos = await getTodosForTempDir(runtimeTempDir);
270+
const retrievedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
235271
expect(retrievedTodos).toEqual(todos);
236272
});
237273
});
@@ -257,12 +293,12 @@ describe("Todo Storage", () => {
257293
},
258294
];
259295

260-
await setTodosForTempDir(tempDir1, todos1);
261-
await setTodosForTempDir(tempDir2, todos2);
296+
await setTodosForTempDir(runtime, tempDir1, todos1);
297+
await setTodosForTempDir(runtime, tempDir2, todos2);
262298

263299
// Verify each temp directory has its own todos
264-
const retrievedTodos1 = await getTodosForTempDir(tempDir1);
265-
const retrievedTodos2 = await getTodosForTempDir(tempDir2);
300+
const retrievedTodos1 = await getTodosForTempDir(runtime, tempDir1);
301+
const retrievedTodos2 = await getTodosForTempDir(runtime, tempDir2);
266302

267303
expect(retrievedTodos1).toEqual(todos1);
268304
expect(retrievedTodos2).toEqual(todos2);
@@ -283,11 +319,11 @@ describe("Todo Storage", () => {
283319
},
284320
];
285321

286-
await setTodosForTempDir(runtimeTempDir, todos);
287-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos);
322+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
323+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(todos);
288324

289-
await clearTodosForTempDir(runtimeTempDir);
290-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual([]);
325+
await clearTodosForTempDir(runtime, runtimeTempDir);
326+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual([]);
291327
});
292328
});
293329
});

‎src/services/tools/todo.ts‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { tool } from "ai";
2-
import * as fs from "fs/promises";
32
import * as path from "path";
3+
import type { Runtime } from "@/runtime/Runtime";
44
import type { ToolFactory } from "@/utils/tools/tools";
55
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";
66
import type { TodoItem } from "@/types/tools";
77
import { MAX_TODOS } from "@/constants/toolLimits";
8+
import { readFileString, writeFileString, execBuffered } from "@/utils/runtime/helpers";
89

910
/**
1011
* Get path to todos.json file in the stream's temporary directory
@@ -14,12 +15,12 @@ function getTodoFilePath(tempDir: string): string {
1415
}
1516

1617
/**
17-
* Read todos from filesystem
18+
* Read todos from filesystem using runtime abstraction
1819
*/
19-
async function readTodos(tempDir: string): Promise<TodoItem[]> {
20+
async function readTodos(runtime: Runtime, tempDir: string): Promise<TodoItem[]> {
2021
const todoFile = getTodoFilePath(tempDir);
2122
try {
22-
const content = await fs.readFile(todoFile, "utf-8");
23+
const content = await readFileString(runtime, todoFile);
2324
return JSON.parse(content) as TodoItem[];
2425
} catch {
2526
// File doesn't exist yet or is invalid
@@ -99,12 +100,14 @@ function validateTodos(todos: TodoItem[]): void {
99100
}
100101

101102
/**
102-
* Write todos to filesystem
103+
* Write todos to filesystem using runtime abstraction
103104
*/
104-
async function writeTodos(tempDir: string, todos: TodoItem[]): Promise<void> {
105+
async function writeTodos(runtime: Runtime, tempDir: string, todos: TodoItem[]): Promise<void> {
105106
validateTodos(todos);
106107
const todoFile = getTodoFilePath(tempDir);
107-
await fs.writeFile(todoFile, JSON.stringify(todos, null, 2), "utf-8");
108+
// Ensure directory exists before writing (SSH runtime might not have created it yet)
109+
await execBuffered(runtime, `mkdir -p "${tempDir}"`, { cwd: "/", timeout: 10 });
110+
await writeFileString(runtime, todoFile, JSON.stringify(todos, null, 2));
108111
}
109112

110113
/**
@@ -116,7 +119,7 @@ export const createTodoWriteTool: ToolFactory = (config) => {
116119
description: TOOL_DEFINITIONS.todo_write.description,
117120
inputSchema: TOOL_DEFINITIONS.todo_write.schema,
118121
execute: async ({ todos }) => {
119-
await writeTodos(config.runtimeTempDir, todos);
122+
await writeTodos(config.runtime, config.runtimeTempDir, todos);
120123
return {
121124
success: true as const,
122125
count: todos.length,
@@ -134,7 +137,7 @@ export const createTodoReadTool: ToolFactory = (config) => {
134137
description: TOOL_DEFINITIONS.todo_read.description,
135138
inputSchema: TOOL_DEFINITIONS.todo_read.schema,
136139
execute: async () => {
137-
const todos = await readTodos(config.runtimeTempDir);
140+
const todos = await readTodos(config.runtime, config.runtimeTempDir);
138141
return {
139142
todos,
140143
};
@@ -145,24 +148,28 @@ export const createTodoReadTool: ToolFactory = (config) => {
145148
/**
146149
* Set todos for a temp directory (useful for testing)
147150
*/
148-
export async function setTodosForTempDir(tempDir: string, todos: TodoItem[]): Promise<void> {
149-
await writeTodos(tempDir, todos);
151+
export async function setTodosForTempDir(
152+
runtime: Runtime,
153+
tempDir: string,
154+
todos: TodoItem[]
155+
): Promise<void> {
156+
await writeTodos(runtime, tempDir, todos);
150157
}
151158

152159
/**
153160
* Get todos for a temp directory (useful for testing)
154161
*/
155-
export async function getTodosForTempDir(tempDir: string): Promise<TodoItem[]> {
156-
return readTodos(tempDir);
162+
export async function getTodosForTempDir(runtime: Runtime, tempDir: string): Promise<TodoItem[]> {
163+
return readTodos(runtime, tempDir);
157164
}
158165

159166
/**
160167
* Clear todos for a temp directory (useful for testing and cleanup)
161168
*/
162-
export async function clearTodosForTempDir(tempDir: string): Promise<void> {
169+
export async function clearTodosForTempDir(runtime: Runtime, tempDir: string): Promise<void> {
163170
const todoFile = getTodoFilePath(tempDir);
164171
try {
165-
await fs.unlink(todoFile);
172+
await execBuffered(runtime, `rm -f "${todoFile}"`, { cwd: "/", timeout: 10 });
166173
} catch {
167174
// File doesn't exist, nothing to clear
168175
}

0 commit comments

Comments
 (0)