diff --git a/src/__tests__/tools.test.ts b/src/__tests__/tools.test.ts index b5f7602..209188d 100644 --- a/src/__tests__/tools.test.ts +++ b/src/__tests__/tools.test.ts @@ -1,6 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdir, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import * as path from "node:path"; import { createSourceList } from "../lib/citations"; -import { TOOL_DEFINITIONS, WEB_TOOL_NAMES, executeTool, toolsFor } from "../lib/tools"; +import { createPolicy, type ApproveFn } from "../lib/files"; +import { TOOL_DEFINITIONS, WEB_TOOL_NAMES, executeTool, toolsFor, type ToolContext } from "../lib/tools"; /** * Direct coverage for the tool layer. @@ -40,10 +44,12 @@ afterEach(() => { }); describe("tool registration", () => { - it("exposes all four tools to the model", () => { + it("exposes the four always-available tools to the model", () => { + // The file tools are in the list too, but they are gated by a preference, so the + // unconditional ones are what this pins. const names = TOOL_DEFINITIONS.map((tool) => tool.function.name); - expect(names).toEqual(["get_datetime", "get_location", "web_search", "web_fetch"]); + expect(names.slice(0, 4)).toEqual(["get_datetime", "get_location", "web_search", "web_fetch"]); }); it("marks both web tools as cloud-dependent", () => { @@ -469,3 +475,151 @@ describe("dispatch", () => { expect(fetchMock).not.toHaveBeenCalled(); }); }); + +describe("the file tools", () => { + let home: string; + + beforeEach(async () => { + // Resolved, because mkdtemp on macOS hands back a path under /var, which is a + // symlink: the policy would then be built on one path and check another. + home = await realpath(await mkdtemp(path.join(tmpdir(), "lam-tools-"))); + await mkdir(path.join(home, "Downloads"), { recursive: true }); + await writeFile(path.join(home, "Downloads", "notes.md"), "alpha\nbeta\n"); + }); + + afterEach(async () => { + await rm(home, { recursive: true, force: true }); + }); + + const context = (approve: ApproveFn = async () => true): ToolContext => ({ + host: "h", + apiKey: "k", + files: { policy: createPolicy({ home }), approve }, + }); + + it("is absent from the list unless file access is on", () => { + expect(toolsFor({ webEnabled: true }).some((tool) => tool.function.name === "read_file")).toBe(false); + expect( + toolsFor({ webEnabled: true, fileEnabled: true }).some((tool) => tool.function.name === "read_file"), + ).toBe(true); + }); + + it("leaves the web tools alone when file access is off", () => { + const names = toolsFor({ webEnabled: true }).map((tool) => tool.function.name); + expect(names).toContain("web_search"); + expect(names).not.toContain("delete_path"); + }); + + it("reads a file through the policy", async () => { + const result = await executeTool( + "read_file", + { path: path.join(home, "Downloads", "notes.md") }, + context(), + ); + expect(result.isError).toBe(false); + expect(result.content).toContain("alpha"); + }); + + it("writes through the approval, and reports a decline as an error", async () => { + const written = await executeTool( + "write_file", + { path: path.join(home, "Downloads", "new.md"), content: "x\n" }, + context(), + ); + expect(written.isError).toBe(false); + + const refused = await executeTool( + "write_file", + { path: path.join(home, "Downloads", "other.md"), content: "x\n" }, + context(async () => false), + ); + expect(refused.isError).toBe(true); + expect(refused.content).toContain("declined"); + }); + + it("returns a policy refusal as an error the model can read", async () => { + const result = await executeTool("read_file", { path: "/etc/passwd" }, context()); + expect(result.isError).toBe(true); + expect(result.content).toContain("outside the paths"); + }); + + it("lists a folder", async () => { + const result = await executeTool("list_directory", { path: home }, context()); + expect(result.isError).toBe(false); + expect(result.content).toContain("Downloads/"); + }); + + it("searches for a file by name", async () => { + const result = await executeTool("search_files", { path: home, pattern: "*.md" }, context()); + expect(result.isError).toBe(false); + expect(result.content).toContain("notes.md"); + }); + + it("edits a file", async () => { + const result = await executeTool( + "edit_file", + { + path: path.join(home, "Downloads", "notes.md"), + old_string: "alpha", + new_string: "ALPHA", + }, + context(), + ); + expect(result.isError).toBe(false); + expect(result.content).toContain("1 occurrence"); + }); + + it("deletes and moves", async () => { + const moved = await executeTool( + "move_path", + { + from: path.join(home, "Downloads", "notes.md"), + to: path.join(home, "Downloads", "moved.md"), + }, + context(), + ); + expect(moved.isError).toBe(false); + + const deleted = await executeTool( + "delete_path", + { path: path.join(home, "Downloads", "moved.md") }, + context(), + ); + expect(deleted.isError).toBe(false); + expect(deleted.content).toContain("Deleted"); + }); + + it("says a tool is unavailable when the context carries no policy", async () => { + const result = await executeTool("read_file", { path: "~" }, { host: "h", apiKey: "k" }); + expect(result.isError).toBe(true); + expect(result.content).toContain("not available"); + }); + + it("requires its arguments", async () => { + const read = await executeTool("read_file", {}, context()); + expect(read.isError).toBe(true); + expect(read.content).toContain("path"); + + const search = await executeTool("search_files", { path: home }, context()); + expect(search.isError).toBe(true); + expect(search.content).toContain("pattern"); + }); + + it("labels every file tool for the activity line", () => { + const tools = toolsFor({ webEnabled: true, fileEnabled: true }); + for (const name of [ + "list_directory", + "search_files", + "read_file", + "write_file", + "edit_file", + "delete_path", + "move_path", + ]) { + expect( + tools.some((tool) => tool.function.name === name), + name, + ).toBe(true); + } + }); +}); diff --git a/src/lib/agent.ts b/src/lib/agent.ts index d6d2e8a..10da8e3 100644 --- a/src/lib/agent.ts +++ b/src/lib/agent.ts @@ -249,6 +249,10 @@ export async function runAgent(options: AgentOptions): Promise { now: options.toolContext.now, signal: options.signal, sources, + // The policy and the ledger travel with the turn, so a second write to a path + // the user already approved does not ask again, and a new question starts + // asking from the beginning. + files: options.toolContext.files, }); const record: ToolCallRecord = { diff --git a/src/lib/tools.ts b/src/lib/tools.ts index a091195..4339667 100644 --- a/src/lib/tools.ts +++ b/src/lib/tools.ts @@ -1,4 +1,16 @@ import { createSourceList, type SourceList } from "./citations"; +import { + deletePath, + editTextFile, + listDirectory, + movePath, + readTextFile, + searchFiles, + writeTextFile, + type ApproveFn, + type FileResult, + type PathPolicy, +} from "./files"; import { formatNow, getLocationContext, weekdayName } from "./location"; import { webFetch, webSearch } from "./ollama"; import type { GeoLookup } from "./geo"; @@ -8,11 +20,12 @@ import { errorMessage } from "./utils"; /** * The tools Lam can call. * - * All four are read-only. That is a deliberate boundary for this version: an - * assistant that can silently act on the user's machine needs a confirmation - * story first, and this release is chat-only. Anything with a side effect - * (opening URLs, clipboard access, launching apps) belongs behind an approval - * step that does not exist yet. + * All are read-only except the file tools, which are the one family here that can + * change the machine. They are bounded in three ways at once: they are absent from + * the tool list unless the user switches file access on, the policy in `files.ts` + * decides what they may touch, and every change is approved in a dialog before it + * happens. Nothing here opens a URL, touches the clipboard or launches a program: + * those would be side effects with no approval story of their own. */ export interface ToolContext { @@ -44,6 +57,14 @@ export interface ToolContext { * list is made on the spot when none is supplied. */ sources?: SourceList; + /** + * The file tools' policy and approval step, when file access is switched on. + * + * Both arrive together because neither is usable alone: the policy decides what may + * be touched and the approval decides whether it happens. Absent means the tools are + * not offered at all, and a call that arrives anyway is refused. + */ + files?: { policy: PathPolicy; approve: ApproveFn }; } /** @@ -123,26 +144,180 @@ export const TOOL_DEFINITIONS: ToolDefinition[] = [ }, }, }, + { + type: "function", + function: { + name: "list_directory", + description: + "List what is in a folder on the user's machine: names, which entries are directories, their sizes and how long ago they changed. Use it before reading a file to get the exact name, and to see what a folder holds.", + parameters: { + type: "object", + properties: { + path: { + type: "string", + description: + "Absolute path, or one starting with ~. A relative path is read as relative to the user's home directory.", + }, + hidden: { type: "boolean", description: "Include dotfiles. Defaults to false." }, + }, + required: ["path"], + }, + }, + }, + { + type: "function", + function: { + name: "search_files", + description: + "Find files by name under a folder, optionally only those whose contents contain a string. Use it when you do not know where something is. Credential files are never returned.", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "Folder to search, absolute or starting with ~." }, + pattern: { + type: "string", + description: 'Glob for the file name, such as "*.md" or "report*". Use "*" for everything.', + }, + contains: { + type: "string", + description: "Only files whose contents contain this text, case-insensitively.", + }, + }, + required: ["path", "pattern"], + }, + }, + }, + { + type: "function", + function: { + name: "read_file", + description: + "Read a text file. Lines come back numbered, and a long file comes back in parts: use offset and limit to continue from where the last read stopped rather than asking for the whole thing. Binary files are refused.", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "File to read, absolute or starting with ~." }, + offset: { type: "integer", description: "First line to return, counting from 1." }, + limit: { type: "integer", description: "How many lines to return. Defaults to 200." }, + }, + required: ["path"], + }, + }, + }, + { + type: "function", + function: { + name: "write_file", + description: + "Create a file, or replace one entirely, with the content you supply. The user approves every change in a dialog before it happens, and a refusal is final: do not retry it, say what you could not do instead. Prefer edit_file for a small change to an existing file.", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "File to write, absolute or starting with ~." }, + content: { type: "string", description: "The complete new contents of the file." }, + }, + required: ["path", "content"], + }, + }, + }, + { + type: "function", + function: { + name: "edit_file", + description: + "Replace an exact string in an existing file, leaving the rest of it alone. old_string must appear exactly once, or pass all to replace every occurrence. Include enough surrounding text to make it unique, and never guess at the contents: read the file first.", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "File to edit, absolute or starting with ~." }, + old_string: { + type: "string", + description: "The exact text to replace, copied from the file.", + }, + new_string: { type: "string", description: "The text to put in its place." }, + all: { + type: "boolean", + description: "Replace every occurrence rather than requiring one.", + }, + }, + required: ["path", "old_string", "new_string"], + }, + }, + }, + { + type: "function", + function: { + name: "delete_path", + description: + "Delete one file, or one empty folder. A folder with anything in it is refused, because Lam never deletes recursively. The user approves every deletion and it cannot be undone.", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "File or empty folder to delete." }, + }, + required: ["path"], + }, + }, + }, + { + type: "function", + function: { + name: "move_path", + description: + "Move or rename a file or folder. Both ends must be somewhere Lam may change, the destination must not already exist, and the user approves every move.", + parameters: { + type: "object", + properties: { + from: { type: "string", description: "Current path." }, + to: { type: "string", description: "New path, which must not exist yet." }, + }, + required: ["from", "to"], + }, + }, + }, ]; /** Tools that need Ollama Cloud. A local Ollama server does not implement these endpoints. */ export const WEB_TOOL_NAMES = new Set(["web_search", "web_fetch"]); +/** Tools that read or change files, offered only when the user has switched file access on. */ +export const FILE_TOOL_NAMES = new Set([ + "list_directory", + "search_files", + "read_file", + "write_file", + "edit_file", + "delete_path", + "move_path", +]); + /** Human-readable labels for the activity line in the UI. */ export const TOOL_LABELS: Record = { get_datetime: "Checking the time", get_location: "Checking your location", web_search: "Searching the web", web_fetch: "Reading a page", + list_directory: "Listing a folder", + search_files: "Searching your files", + read_file: "Reading a file", + write_file: "Writing a file", + edit_file: "Editing a file", + delete_path: "Deleting a file", + move_path: "Moving a file", }; export function toolLabel(name: string): string { return TOOL_LABELS[name] ?? `Running ${name}`; } -/** Filters out the web tools when the user has turned them off. */ -export function toolsFor(options: { webEnabled: boolean }): ToolDefinition[] { - return TOOL_DEFINITIONS.filter((tool) => options.webEnabled || !WEB_TOOL_NAMES.has(tool.function.name)); +/** Filters out the tools the user has switched off. */ +export function toolsFor(options: { webEnabled: boolean; fileEnabled?: boolean }): ToolDefinition[] { + return TOOL_DEFINITIONS.filter((tool) => { + const name = tool.function.name; + if (WEB_TOOL_NAMES.has(name)) return options.webEnabled; + if (FILE_TOOL_NAMES.has(name)) return options.fileEnabled ?? false; + return true; + }); } function asString(value: unknown): string | undefined { @@ -161,6 +336,42 @@ function isWebTool(name: string): boolean { return WEB_TOOL_NAMES.has(name); } +/** The path argument every file tool needs, as the model wrote it. */ +function requirePath(args: Record): string { + return typeof args.path === "string" ? args.path : ""; +} + +function missing(tool: string, argument: string): ToolResult { + return { content: `${tool} requires a "${argument}".`, isError: true }; +} + +/** + * Runs one file operation, once the context is known to have a policy. + * + * The missing-policy branch is not dead code: `toolsFor` keeps these tools out of the + * list when file access is off, so a call only arrives here if a model invents one, and + * then the honest answer is that the tool is not available. + */ +async function runFileTool( + context: ToolContext, + run: (files: { policy: PathPolicy; approve: ApproveFn }) => Promise, +): Promise { + const files = context.files; + if (!files) { + return { + content: "File tools are not available in this session. The user has not switched file access on.", + isError: true, + }; + } + + const result = await run(files); + // A missing argument is answered here rather than by the policy, so the callback is + // allowed to return either shape and this is where the two meet. + if ("isError" in result) return result; + + return { content: result.content, isError: !result.ok }; +} + /** * Runs one tool call and always resolves. * @@ -188,6 +399,73 @@ export async function executeTool( case "web_fetch": return await runWebFetch(args, context); + case "list_directory": + return await runFileTool(context, (files) => + listDirectory(requirePath(args), files.policy, { hidden: args.hidden === true }), + ); + + case "search_files": + return await runFileTool(context, (files) => { + const pattern = asString(args.pattern); + if (!pattern) return Promise.resolve(missing("search_files", "pattern")); + return searchFiles( + { path: requirePath(args), pattern, contains: asString(args.contains) }, + files.policy, + ); + }); + + case "read_file": + return await runFileTool(context, (files) => + readTextFile( + { + path: requirePath(args), + offset: asCount(args.offset, 1, Number.MAX_SAFE_INTEGER), + limit: asCount(args.limit, 1, 5000), + }, + files.policy, + ), + ); + + case "write_file": + return await runFileTool(context, (files) => + writeTextFile( + { + path: requirePath(args), + content: typeof args.content === "string" ? args.content : "", + }, + files.policy, + files.approve, + ), + ); + + case "edit_file": + return await runFileTool(context, (files) => { + const oldString = typeof args.old_string === "string" ? args.old_string : undefined; + const newString = typeof args.new_string === "string" ? args.new_string : ""; + if (oldString === undefined) return Promise.resolve(missing("edit_file", "old_string")); + return editTextFile( + { path: requirePath(args), oldString, newString, all: args.all === true }, + files.policy, + files.approve, + ); + }); + + case "delete_path": + return await runFileTool(context, (files) => + deletePath(requirePath(args), files.policy, files.approve), + ); + + case "move_path": + return await runFileTool(context, (files) => { + // `from` rather than `path`: this is the one file tool with two paths, and + // reading the shared helper here would have sent an empty source path. + const from = asString(args.from); + const to = asString(args.to); + if (!from) return Promise.resolve(missing("move_path", "from")); + if (!to) return Promise.resolve(missing("move_path", "to")); + return movePath({ from, to }, files.policy, files.approve); + }); + default: return { content: `Unknown tool "${name}".`, isError: true }; }