Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 66 additions & 30 deletions src/services/tools/todo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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);
});

Expand All @@ -54,7 +59,7 @@ describe("Todo Storage", () => {
},
];

await setTodosForTempDir(runtimeTempDir, initialTodos);
await setTodosForTempDir(runtime, runtimeTempDir, initialTodos);

// Replace with updated list
const updatedTodos: TodoItem[] = [
Expand All @@ -72,26 +77,26 @@ 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",
},
]);

// Clear list
await setTodosForTempDir(runtimeTempDir, []);
await setTodosForTempDir(runtime, runtimeTempDir, []);

const storedTodos = await getTodosForTempDir(runtimeTempDir);
const storedTodos = await getTodosForTempDir(runtime, runtimeTempDir);
expect(storedTodos).toEqual([]);
});

Expand All @@ -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
);
});
Expand All @@ -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 () => {
Expand All @@ -139,7 +144,7 @@ describe("Todo Storage", () => {
},
];

await setTodosForTempDir(runtimeTempDir, validTodos);
await setTodosForTempDir(runtime, runtimeTempDir, validTodos);

const invalidTodos: TodoItem[] = [
{
Expand All @@ -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 () => {
Expand All @@ -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
);
});
Expand All @@ -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
);
});
Expand All @@ -206,14 +211,45 @@ 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 () => {
// 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(runtime, nonExistentDir, todos);

// Verify the file was created and is readable
const retrievedTodos = await getTodosForTempDir(runtime, 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", () => {
it("should return empty array when no todos exist", async () => {
const todos = await getTodosForTempDir(runtimeTempDir);
const todos = await getTodosForTempDir(runtime, runtimeTempDir);
expect(todos).toEqual([]);
});

Expand All @@ -229,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);
});
});
Expand All @@ -257,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);
Expand All @@ -283,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([]);
});
});
});
37 changes: 22 additions & 15 deletions src/services/tools/todo.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<TodoItem[]> {
async function readTodos(runtime: Runtime, tempDir: string): Promise<TodoItem[]> {
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
Expand Down Expand Up @@ -99,12 +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<void> {
async function writeTodos(runtime: Runtime, tempDir: string, todos: TodoItem[]): Promise<void> {
validateTodos(todos);
const todoFile = getTodoFilePath(tempDir);
await fs.writeFile(todoFile, JSON.stringify(todos, null, 2), "utf-8");
// Ensure directory exists before writing (SSH runtime might not have created it yet)
await execBuffered(runtime, `mkdir -p "${tempDir}"`, { cwd: "/", timeout: 10 });
await writeFileString(runtime, todoFile, JSON.stringify(todos, null, 2));
}

/**
Expand All @@ -116,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,
Expand All @@ -134,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,
};
Expand All @@ -145,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<void> {
await writeTodos(tempDir, todos);
export async function setTodosForTempDir(
runtime: Runtime,
tempDir: string,
todos: TodoItem[]
): Promise<void> {
await writeTodos(runtime, tempDir, todos);
}

/**
* Get todos for a temp directory (useful for testing)
*/
export async function getTodosForTempDir(tempDir: string): Promise<TodoItem[]> {
return readTodos(tempDir);
export async function getTodosForTempDir(runtime: Runtime, tempDir: string): Promise<TodoItem[]> {
return readTodos(runtime, tempDir);
}

/**
* Clear todos for a temp directory (useful for testing and cleanup)
*/
export async function clearTodosForTempDir(tempDir: string): Promise<void> {
export async function clearTodosForTempDir(runtime: Runtime, tempDir: string): Promise<void> {
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
}
Expand Down