feat: seven file tools behind the policy and an approval
The file tools are absent from the list unless file access is on, which is the difference between a model that cannot call them and one that is told not to. Each handler is thin: argument checks here, and the operation itself in files.ts. move_path reads `from`, not the shared path helper, which is the bug the tests caught: an empty source path would have been refused by the policy on every move.
This commit is contained in:
+157
-3
@@ -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);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -249,6 +249,10 @@ export async function runAgent(options: AgentOptions): Promise<AgentResult> {
|
||||
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 = {
|
||||
|
||||
+286
-8
@@ -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<string, string> = {
|
||||
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, unknown>): 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<FileResult | ToolResult>,
|
||||
): Promise<ToolResult> {
|
||||
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 };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user