Skip to content

Commit 451ae4d

Browse files
ammar-agentammar-agent
committed
🤖 Use runtime abstraction for todo tool file operations
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.
1 parent 11c2151 commit 451ae4d

File tree

2 files changed

+58
-48
lines changed

2 files changed

+58
-48
lines changed

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

Lines changed: 37 additions & 32 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,8 +211,8 @@ 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);
211216
});
212217

213218
it("should create directory if it doesn't exist", async () => {
@@ -223,10 +228,10 @@ describe("Todo Storage", () => {
223228
];
224229

225230
// Should not throw even though directory doesn't exist
226-
await setTodosForTempDir(nonExistentDir, todos);
231+
await setTodosForTempDir(runtime, nonExistentDir, todos);
227232

228233
// Verify the file was created and is readable
229-
const retrievedTodos = await getTodosForTempDir(nonExistentDir);
234+
const retrievedTodos = await getTodosForTempDir(runtime, nonExistentDir);
230235
expect(retrievedTodos).toEqual(todos);
231236

232237
// Verify the directory was actually created
@@ -244,7 +249,7 @@ describe("Todo Storage", () => {
244249

245250
describe("getTodosForTempDir", () => {
246251
it("should return empty array when no todos exist", async () => {
247-
const todos = await getTodosForTempDir(runtimeTempDir);
252+
const todos = await getTodosForTempDir(runtime, runtimeTempDir);
248253
expect(todos).toEqual([]);
249254
});
250255

@@ -260,9 +265,9 @@ describe("Todo Storage", () => {
260265
},
261266
];
262267

263-
await setTodosForTempDir(runtimeTempDir, todos);
268+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
264269

265-
const retrievedTodos = await getTodosForTempDir(runtimeTempDir);
270+
const retrievedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
266271
expect(retrievedTodos).toEqual(todos);
267272
});
268273
});
@@ -288,12 +293,12 @@ describe("Todo Storage", () => {
288293
},
289294
];
290295

291-
await setTodosForTempDir(tempDir1, todos1);
292-
await setTodosForTempDir(tempDir2, todos2);
296+
await setTodosForTempDir(runtime, tempDir1, todos1);
297+
await setTodosForTempDir(runtime, tempDir2, todos2);
293298

294299
// Verify each temp directory has its own todos
295-
const retrievedTodos1 = await getTodosForTempDir(tempDir1);
296-
const retrievedTodos2 = await getTodosForTempDir(tempDir2);
300+
const retrievedTodos1 = await getTodosForTempDir(runtime, tempDir1);
301+
const retrievedTodos2 = await getTodosForTempDir(runtime, tempDir2);
297302

298303
expect(retrievedTodos1).toEqual(todos1);
299304
expect(retrievedTodos2).toEqual(todos2);
@@ -314,11 +319,11 @@ describe("Todo Storage", () => {
314319
},
315320
];
316321

317-
await setTodosForTempDir(runtimeTempDir, todos);
318-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual(todos);
322+
await setTodosForTempDir(runtime, runtimeTempDir, todos);
323+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual(todos);
319324

320-
await clearTodosForTempDir(runtimeTempDir);
321-
expect(await getTodosForTempDir(runtimeTempDir)).toEqual([]);
325+
await clearTodosForTempDir(runtime, runtimeTempDir);
326+
expect(await getTodosForTempDir(runtime, runtimeTempDir)).toEqual([]);
322327
});
323328
});
324329
});

‎src/services/tools/todo.ts‎

Lines changed: 21 additions & 16 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,14 +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);
107108
// Ensure directory exists before writing (SSH runtime might not have created it yet)
108-
await fs.mkdir(tempDir, { recursive: true });
109-
await fs.writeFile(todoFile, JSON.stringify(todos, null, 2), "utf-8");
109+
await execBuffered(runtime, `mkdir -p "${tempDir}"`, { cwd: "/", timeout: 10 });
110+
await writeFileString(runtime, todoFile, JSON.stringify(todos, null, 2));
110111
}
111112

112113
/**
@@ -118,7 +119,7 @@ export const createTodoWriteTool: ToolFactory = (config) => {
118119
description: TOOL_DEFINITIONS.todo_write.description,
119120
inputSchema: TOOL_DEFINITIONS.todo_write.schema,
120121
execute: async ({ todos }) => {
121-
await writeTodos(config.runtimeTempDir, todos);
122+
await writeTodos(config.runtime, config.runtimeTempDir, todos);
122123
return {
123124
success: true as const,
124125
count: todos.length,
@@ -136,7 +137,7 @@ export const createTodoReadTool: ToolFactory = (config) => {
136137
description: TOOL_DEFINITIONS.todo_read.description,
137138
inputSchema: TOOL_DEFINITIONS.todo_read.schema,
138139
execute: async () => {
139-
const todos = await readTodos(config.runtimeTempDir);
140+
const todos = await readTodos(config.runtime, config.runtimeTempDir);
140141
return {
141142
todos,
142143
};
@@ -147,24 +148,28 @@ export const createTodoReadTool: ToolFactory = (config) => {
147148
/**
148149
* Set todos for a temp directory (useful for testing)
149150
*/
150-
export async function setTodosForTempDir(tempDir: string, todos: TodoItem[]): Promise<void> {
151-
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);
152157
}
153158

154159
/**
155160
* Get todos for a temp directory (useful for testing)
156161
*/
157-
export async function getTodosForTempDir(tempDir: string): Promise<TodoItem[]> {
158-
return readTodos(tempDir);
162+
export async function getTodosForTempDir(runtime: Runtime, tempDir: string): Promise<TodoItem[]> {
163+
return readTodos(runtime, tempDir);
159164
}
160165

161166
/**
162167
* Clear todos for a temp directory (useful for testing and cleanup)
163168
*/
164-
export async function clearTodosForTempDir(tempDir: string): Promise<void> {
169+
export async function clearTodosForTempDir(runtime: Runtime, tempDir: string): Promise<void> {
165170
const todoFile = getTodoFilePath(tempDir);
166171
try {
167-
await fs.unlink(todoFile);
172+
await execBuffered(runtime, `rm -f "${todoFile}"`, { cwd: "/", timeout: 10 });
168173
} catch {
169174
// File doesn't exist, nothing to clear
170175
}

0 commit comments

Comments
 (0)