From 626450eb069f78ad6d2007b8e0f22dc083867418 Mon Sep 17 00:00:00 2001 From: asepharyana Date: Wed, 9 Sep 2026 00:29:23 +0700 Subject: [PATCH] TODO Next: derive TOOL_SETS and MUTATING_TOOLS from tool definitions Wrap every builtin tool with withMeta({set, mutating}) at its definition site (src/tool-utils.ts). TOOL_SETS and MUTATING_TOOLS are now derived via setsFrom/mutatingNames rather than hand-lists, so a new write cannot be added ungated by forgetting a parallel list. Permission defaults now cover the 5 extra mutating line-edit tools. Test suite covers coverage, derived-equality, and mutating consistency (test/tool-derive.test.ts). --- TODO.md | 6 +-- docs/development.md | 26 +++++------ src/permission.ts | 10 +++++ src/tool-utils.ts | 34 +++++++++++++++ src/tools-extra.ts | 81 +++++++++++++++++----------------- src/tools-git.ts | 25 +++++------ src/tools-net.ts | 5 ++- src/tools.ts | 93 ++++++++++++++++------------------------ test/tool-derive.test.ts | 50 +++++++++++++++++++++ 9 files changed, 205 insertions(+), 125 deletions(-) create mode 100644 src/tool-utils.ts create mode 100644 test/tool-derive.test.ts diff --git a/TODO.md b/TODO.md index 15e7499..37e3b52 100644 --- a/TODO.md +++ b/TODO.md @@ -52,9 +52,9 @@ names only the servers. A hundred servers then cost almost nothing until one is in the other is a silently ungated write, which is the worst kind of bug this codebase can have. -- [ ] Mark each tool as mutating where it is defined, not in a list beside it -- [ ] `TOOL_SETS` covers every registered tool, checked rather than assumed -- [ ] Test: a tool in no set, or a mutating tool outside `MUTATING_TOOLS`, fails the suite +- [x] Mark each tool as mutating where it is defined, not in a list beside it (`src/tool-utils.ts` `withMeta({ set, mutating })`, each tool file wraps its `tool({` definitions — 41 tools across `tools.ts`/`tools-extra.ts`/`tools-git.ts`/`tools-net.ts`) +- [x] `TOOL_SETS` covers every registered tool, checked rather than assumed (`setsFrom(tools)` derives from `_meta`, `TOOL_SETS`/`MUTATING_TOOLS` are derived, `DEFAULT_PERMISSIONS` covers all mutating) +- [x] Test: a tool in no set, or a mutating tool outside `MUTATING_TOOLS`, fails the suite (`test/tool-derive.test.ts`: 5 tests — `_meta` present, `TOOL_SETS` derived + exact-once + coverage, `MUTATING_TOOLS` derived, permissions coverage, mutating consistency) ### Subagent parallelism diff --git a/docs/development.md b/docs/development.md index e903c54..e5a6e68 100644 --- a/docs/development.md +++ b/docs/development.md @@ -84,19 +84,21 @@ mock-verification test: ## Adding a tool -1. Define it in `src/tools.ts` with a `zod` schema. Descriptions are read by the model, so - write them as guidance, not as documentation. -2. Add it to the `tools` object. -3. Add it to a set in `TOOL_SETS`. A tool in no set can never be gated off. -4. If it mutates anything, add it to `MUTATING_TOOLS` so it requires approval. -5. Add a line to `TOOL_DOCS` in `src/prompt.ts` saying *when* to reach for it. -6. If it is read-only, add it to `READ_ONLY` in `src/agents.ts` so `plan` and `review` can use +1. Define it with a `zod` schema and wrap it with `withMeta({ set, mutating }, tool({…}))` + at the definition site. `set` is `core | edit-plus | nav | extra | git | net`, `mutating` + is whether it writes or executes. Example: `src/tools-extra.ts` (`extra`), `src/tools-git.ts` + (`git`), `src/tools-net.ts` (`net`), `src/tools.ts` (everything else). Descriptions are + read by the model, so write them as guidance, not as documentation. +2. Add it to the `tools` object (or `extraTools`/`gitTools`/`netTools` — they are merged in + `src/tools.ts`). +3. Add its subject to `subjectOf` in `src/permission.ts` if the approval prompt should match + on a field (path, command, …). Add a `DEFAULT_PERMISSIONS` entry for mutating tools. +4. Add a line to `TOOL_DOCS` in `src/prompt.ts` saying *when* to reach for it. +5. If it is read-only, add it to `READ_ONLY` in `src/agents.ts` so `plan` and `review` can use it. -7. Test the behaviour in a temp directory, including the failure path. - -Steps 3 and 4 are two hand-maintained lists of tool names, which is a known weakness: a tool -added to one and forgotten in the other is a silently ungated write. Deriving both from the -tool definitions is on [TODO.md](../TODO.md). +6. Test the behaviour in a temp directory, including the failure path. `test/tool-derive.test.ts` + fails if a builtin tool has no `_meta` or lives in no set, or a mutating tool is outside + `MUTATING_TOOLS` — both are derived from the definitions, not hand-lists. Every tool costs roughly 550 characters of schema on every request. Nineteen built-in tools is past where selection accuracy starts to matter, which is why sets exist and why a new tool diff --git a/src/permission.ts b/src/permission.ts index e652975..36284ed 100644 --- a/src/permission.ts +++ b/src/permission.ts @@ -66,6 +66,11 @@ export function subjectOf(tool: string, input: unknown): string | undefined { case 'edit_file': case 'multi_edit': case 'delete_file': + case 'insert_lines': + case 'delete_lines': + case 'replace_lines': + case 'append_file': + case 'prepend_file': case 'list_dir': case 'git_blame': return str('path'); @@ -181,6 +186,11 @@ export const DEFAULT_PERMISSIONS: PermissionConfig = { apply_patch: 'ask', move_file: 'ask', delete_file: 'ask', + insert_lines: 'ask', + delete_lines: 'ask', + replace_lines: 'ask', + append_file: 'ask', + prepend_file: 'ask', bash: 'ask', web_fetch: 'ask', }; diff --git a/src/tool-utils.ts b/src/tool-utils.ts new file mode 100644 index 0000000..76090d6 --- /dev/null +++ b/src/tool-utils.ts @@ -0,0 +1,34 @@ +export type ToolSetName = 'core' | 'edit-plus' | 'nav' | 'extra' | 'git' | 'net'; + +export type ToolMeta = { set: ToolSetName; mutating: boolean }; + +/** + * Tags a tool with its set and mutating flag at the definition site. + * `_meta` is consumed only to derive TOOL_SETS and MUTATING_TOOLS. + */ +export function withMeta(meta: ToolMeta, t: T): T & { _meta: ToolMeta } { + return Object.assign(t as object, { _meta: meta }) as T & { _meta: ToolMeta }; +} + +export function mutatingNames(tools: Record): string[] { + const out = Object.entries(tools) + .filter(([, t]) => t._meta?.mutating) + .map(([name]) => name); + out.sort(); + return out; +} + +export function setsFrom(tools: Record): Record { + const out: Record = { core: [], 'edit-plus': [], nav: [], extra: [], git: [], net: [] }; + for (const [name, t] of Object.entries(tools)) { + const set = t._meta?.set; + if (set) out[set].push(name); + } + for (const k of Object.keys(out) as ToolSetName[]) out[k].sort(); + return out as Record; +} + +export function assertAllToolsInSets(tools: Record, sets: Record): string[] { + const covered = new Set((Object.values(sets) as unknown as string[][]).flat()); + return Object.keys(tools).filter((name) => !covered.has(name)); +} diff --git a/src/tools-extra.ts b/src/tools-extra.ts index 566cb00..ab8a77d 100644 --- a/src/tools-extra.ts +++ b/src/tools-extra.ts @@ -3,6 +3,7 @@ import { stat } from 'node:fs/promises'; import { resolve } from 'node:path'; import { z } from 'zod'; import { jail, posix, walk } from './ignore'; +import { withMeta } from './tool-utils'; import { git } from './tools-git'; /** @@ -36,7 +37,7 @@ async function readLines(path: string): Promise<{ abs: string; lines: string[] } // edit // --------------------------------------------------------------------------- -export const insertLinesTool = tool({ +export const insertLinesTool = withMeta({ set: 'extra', mutating: true }, tool({ description: 'Insert lines at a 1-based position in a file, pushing the rest down. Cheaper and safer than a rewrite for adding a block in the middle.', inputSchema: z.object({ @@ -51,9 +52,9 @@ export const insertLinesTool = tool({ await Bun.write(abs, cur.join('\n')); return `Inserted ${lines(text).length} line(s) at ${path}:${line}`; }, -}); +})); -export const deleteLinesTool = tool({ +export const deleteLinesTool = withMeta({ set: 'extra', mutating: true }, tool({ description: 'Delete an inclusive range of lines from a file. Refuses to delete the whole file; use delete_file for that.', inputSchema: z.object({ path: z.string(), @@ -69,9 +70,9 @@ export const deleteLinesTool = tool({ await Bun.write(abs, cur.join('\n')); return `Deleted lines ${start}-${end} from ${path}`; }, -}); +})); -export const replaceLinesTool = tool({ +export const replaceLinesTool = withMeta({ set: 'extra', mutating: true }, tool({ description: 'Replace an inclusive range of lines with new text, in one write.', inputSchema: z.object({ path: z.string(), @@ -87,9 +88,9 @@ export const replaceLinesTool = tool({ await Bun.write(abs, cur.join('\n')); return `Replaced lines ${start}-${end} in ${path}`; }, -}); +})); -export const appendFileTool = tool({ +export const appendFileTool = withMeta({ set: 'extra', mutating: true }, tool({ description: 'Append text to the end of a file without reading the whole thing into the edit.', inputSchema: z.object({ path: z.string(), text: z.string() }), execute: async ({ path, text }) => { @@ -97,9 +98,9 @@ export const appendFileTool = tool({ await Bun.write(abs, `${cur.join('\n').replace(/\n?$/, '\n')}${text.replace(/\n?$/, '')}\n`); return `Appended ${lines(text).length} line(s) to ${path}`; }, -}); +})); -export const prependFileTool = tool({ +export const prependFileTool = withMeta({ set: 'extra', mutating: true }, tool({ description: 'Prepend text to the start of a file, e.g. a license header or an import block.', inputSchema: z.object({ path: z.string(), text: z.string() }), execute: async ({ path, text }) => { @@ -107,9 +108,9 @@ export const prependFileTool = tool({ await Bun.write(abs, `${text.replace(/\n?$/, '\n')}${cur.join('\n')}`); return `Prepended ${lines(text).length} line(s) to ${path}`; }, -}); +})); -export const countLinesTool = tool({ +export const countLinesTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Count lines in one file, or per file across a glob. A quick size read before deciding to open something large.', inputSchema: z.object({ path: z.string().optional().describe('One file. Omit to use pattern instead'), @@ -133,7 +134,7 @@ export const countLinesTool = tool({ } return out.length ? cap(out.join('\n')) : 'No matching text files.'; }, -}); +})); // --------------------------------------------------------------------------- // inspect @@ -141,7 +142,7 @@ export const countLinesTool = tool({ const MAX_TREE = 400; -export const treeTool = tool({ +export const treeTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Indented directory tree from a path, honouring .gitignore, with directories first. Faster to scan than list_dir for a broad shape.', inputSchema: z.object({ @@ -166,9 +167,9 @@ export const treeTool = tool({ const out = rows.map((r) => `${' '.repeat(r.depth - 1)}${r.rel.split('/').at(-1)}${r.dir ? '/' : ''}`); return out.length ? cap((prefix ? `${prefix.replace(/\/$/, '')}/\n` : './\n') + out.join('\n')) : `Nothing under ${path}.`; }, -}); +})); -export const fileInfoTool = tool({ +export const fileInfoTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Metadata for one file: size, line count, modified time, and whether it is text or binary.', inputSchema: z.object({ path: z.string() }), execute: async ({ path }) => { @@ -185,9 +186,9 @@ export const fileInfoTool = tool({ const linesN = binary ? undefined : (await Bun.file(abs).text()).split('\n').length; return `${path}: ${entry.size} bytes${linesN === undefined ? '' : `, ${linesN} lines`}, ${binary ? 'binary' : 'text'}, modified ${entry.mtime.toISOString()}`; }, -}); +})); -export const findFilesTool = tool({ +export const findFilesTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Find files whose *name* contains a substring (not a glob), e.g. "auth" or ".test.". Honours .gitignore.', inputSchema: z.object({ name: z.string().describe('Substring to match against the filename'), @@ -202,9 +203,9 @@ export const findFilesTool = tool({ } return hits.length ? cap(hits.join('\n')) : `No files matching "${name}".`; }, -}); +})); -export const recentFilesTool = tool({ +export const recentFilesTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Files modified most recently, newest first. Orient in a tree you did not write, or find what a tool just touched.', inputSchema: z.object({ limit: z.number().int().min(1).optional().describe('Default 20') }), execute: async ({ limit = 20 }) => { @@ -221,9 +222,9 @@ export const recentFilesTool = tool({ const out = seen.slice(0, limit).map((s) => `${new Date(s.mtime).toISOString().slice(0, 19).replace('T', ' ')} ${s.rel}`); return out.length ? cap(out.join('\n')) : 'No files found.'; }, -}); +})); -export const changedFilesTool = tool({ +export const changedFilesTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Files git reports as modified, staged, or untracked — the working-tree delta at a glance, without a full status.', inputSchema: z.object({}), execute: async () => { @@ -235,7 +236,7 @@ export const changedFilesTool = tool({ .map((l) => `${l.slice(0, 2).trim() || ' '} ${posix(l.slice(3))}`); return out.length ? cap(out.join('\n')) : 'Working tree clean.'; }, -}); +})); // --------------------------------------------------------------------------- // git ext (read-only, argv-spawned) @@ -247,7 +248,7 @@ const gitRun = async (args: string[], empty: string): Promise => { return cap(result.stdout.trim() || empty); }; -export const gitLogFileTool = tool({ +export const gitLogFileTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Commits that touched one file, newest first, with hash, date, and subject.', inputSchema: z.object({ path: z.string(), @@ -255,9 +256,9 @@ export const gitLogFileTool = tool({ }), execute: async ({ path, limit = 15 }) => gitRun(['log', `--max-count=${limit}`, '--pretty=format:%h %ad %s', '--date=short', '--', posix(path)], 'No history for that file.'), -}); +})); -export const gitDiffCommitsTool = tool({ +export const gitDiffCommitsTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Diff between two refs (branches, tags, or commits), optionally limited to one path.', inputSchema: z.object({ from: z.string().describe('Base ref'), @@ -266,34 +267,34 @@ export const gitDiffCommitsTool = tool({ }), execute: async ({ from, to, path }) => gitRun(['diff', `${from}...${to}`, ...(path ? ['--', posix(path)] : [])], `No differences between ${from} and ${to}.`), -}); +})); -export const gitShowFileTool = tool({ +export const gitShowFileTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'The contents of a file at a ref, e.g. what auth.ts looked like at HEAD~3 or on main.', inputSchema: z.object({ ref: z.string().describe('Branch, tag, or commit'), path: z.string(), }), execute: async ({ ref, path }) => gitRun(['show', `${ref}:${posix(path)}`], `No ${path} at ${ref}.`), -}); +})); -export const gitCurrentBranchTool = tool({ +export const gitCurrentBranchTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'The current branch, plus its upstream and ahead/behind count when one is set.', inputSchema: z.object({}), execute: async () => gitRun(['status', '--short', '--branch'], 'no commits yet'), -}); +})); -export const gitChangedInRefTool = tool({ +export const gitChangedInRefTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Files changed between a ref and the working tree, name only.', inputSchema: z.object({ ref: z.string().describe('Compare the working tree against this ref, e.g. main') }), execute: async ({ ref }) => gitRun(['diff', '--name-only', ref], `No changes against ${ref}.`), -}); +})); // --------------------------------------------------------------------------- // code // --------------------------------------------------------------------------- -export const outlineTool = tool({ +export const outlineTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Top-level declarations of a source file — functions, classes, types, exports — as a compact structural map. Read this before opening a large file.', inputSchema: z.object({ path: z.string() }), @@ -308,9 +309,9 @@ export const outlineTool = tool({ }); return out.length ? cap(out.join('\n')) : `No top-level declarations found in ${path}.`; }, -}); +})); -export const readSymbolTool = tool({ +export const readSymbolTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'The full body of one top-level definition (function, class, type) from a file, by name.', inputSchema: z.object({ path: z.string(), @@ -335,9 +336,9 @@ export const readSymbolTool = tool({ } return cap(src.slice(start, end + 1).map((l, i) => `${start + i + 1}: ${l}`).join('\n')); }, -}); +})); -export const envInfoTool = tool({ +export const envInfoTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Platform, shell, runtimes, and package managers present, so commands are written for what is actually installed.', inputSchema: z.object({}), execute: async () => { @@ -365,9 +366,9 @@ export const envInfoTool = tool({ } return rows.join('\n'); }, -}); +})); -export const countTokensTool = tool({ +export const countTokensTool = withMeta({ set: 'extra', mutating: false }, tool({ description: 'Estimate the token cost of a file or a string before sending it to the model (~4 chars per token).', inputSchema: z.object({ path: z.string().optional().describe('A file to measure'), @@ -385,7 +386,7 @@ export const countTokensTool = tool({ const chars = content.length; return `${path ?? 'input'}: ${chars} chars, ~${Math.round(chars / 4)} tokens`; }, -}); +})); /** The 20, registered by name for the tools map and the `extra` tool set. */ export const extraTools = { diff --git a/src/tools-git.ts b/src/tools-git.ts index 6926a22..3e19b4d 100644 --- a/src/tools-git.ts +++ b/src/tools-git.ts @@ -1,4 +1,5 @@ import { tool } from 'ai'; +import { withMeta } from './tool-utils'; import { z } from 'zod'; const MAX_OUTPUT = 30_000; @@ -58,7 +59,7 @@ const STATUS_LABEL: Record = { '!': 'ignored', }; -export const gitStatusTool = tool({ +export const gitStatusTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'Working tree status: current branch, and which files are staged, modified, or untracked. ' + 'Use it before proposing a commit, and to see what you have changed so far.', @@ -87,9 +88,9 @@ export const gitStatusTool = tool({ return cap([`On ${branch.stdout.trim()}, ${lines.length} changed:`, ...described].join('\n')); }, -}); +})); -export const gitDiffTool = tool({ +export const gitDiffTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'Unified diff of uncommitted changes. Pass staged to see what is staged instead, or a path to narrow it. ' + 'Use it to review your own edits before claiming they are done.', @@ -103,9 +104,9 @@ export const gitDiffTool = tool({ if (path) args.push('--', path); return run(args, staged ? 'Nothing staged.' : 'No uncommitted changes.'); }, -}); +})); -export const gitLogTool = tool({ +export const gitLogTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'Recent commits, newest first: short hash, date, author, subject. Pass a path to see only commits touching it. ' + 'Use it to find when something changed and who changed it.', @@ -118,9 +119,9 @@ export const gitLogTool = tool({ if (path) args.push('--', path); return run(args, 'No commits.'); }, -}); +})); -export const gitShowTool = tool({ +export const gitShowTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'One commit in full: message, author, and its diff. Takes a hash, tag, or ref like HEAD~2. ' + 'Use it after git_log to see what a specific commit actually did.', @@ -133,9 +134,9 @@ export const gitShowTool = tool({ if (path) args.push('--', path); return run(args, 'Nothing to show.'); }, -}); +})); -export const gitBlameTool = tool({ +export const gitBlameTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'Who last changed each line of a file, with the commit and date. Narrow with startLine and endLine. ' + 'Use it when a line looks wrong and its history explains why.', @@ -150,9 +151,9 @@ export const gitBlameTool = tool({ args.push('--', path); return run(args, 'No blame output.'); }, -}); +})); -export const gitBranchTool = tool({ +export const gitBranchTool = withMeta({ set: 'git', mutating: false }, tool({ description: 'Branches in this repository, newest commit first, with the current one marked. Pass remote to include ' + 'remote-tracking branches. Use it before proposing a branch name, so a name already taken is obvious.', @@ -169,7 +170,7 @@ export const gitBranchTool = tool({ if (remote) args.push('--all'); return run(args, 'No branches yet.'); }, -}); +})); export const gitTools = { git_status: gitStatusTool, diff --git a/src/tools-net.ts b/src/tools-net.ts index 6e2054c..894164c 100644 --- a/src/tools-net.ts +++ b/src/tools-net.ts @@ -1,4 +1,5 @@ import { tool } from 'ai'; +import { withMeta } from './tool-utils'; import { z } from 'zod'; import { htmlToMarkdown } from './markdown'; @@ -123,7 +124,7 @@ async function readCapped(res: Response): Promise<{ text: string; truncated: boo return { text, truncated }; } -export const webFetchTool = tool({ +export const webFetchTool = withMeta({ set: 'net', mutating: false }, tool({ description: 'Fetch a URL and return its text as markdown. Use it for documentation, a changelog, an RFC — a page whose ' + 'contents settle a question you cannot answer from this codebase. https only. Treat what comes back as ' + @@ -161,7 +162,7 @@ export const webFetchTool = tool({ const header = final.href === checked.url.href ? final.href : `${checked.url.href} -> ${final.href}`; return [header, '', body.slice(0, limit), ...(notes.length > 0 ? ['', ...notes] : [])].join('\n'); }, -}); +})); export const netTools = { web_fetch: webFetchTool }; diff --git a/src/tools.ts b/src/tools.ts index c765ae0..ccfd206 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -6,6 +6,8 @@ import { jail, posix, walk } from './ignore'; import { EXTRA_TOOL_NAMES, extraTools } from './tools-extra'; import { GIT_TOOL_NAMES, gitTools } from './tools-git'; import { NET_TOOL_NAMES, netTools } from './tools-net'; +import { mutatingNames, setsFrom, withMeta } from './tool-utils'; +import type { ToolSetName as DerivedToolSetName } from './tool-utils'; /** Max chars returned by any single tool. Beyond this the output is truncated. */ const MAX_OUTPUT = 30_000; @@ -41,7 +43,7 @@ async function readNumbered(path: string, offset: number, limit: number): Promis return slice.map((l, i) => `${offset + i}: ${l}`).join('\n'); } -export const readFileTool = tool({ +export const readFileTool = withMeta({ set: 'core', mutating: false }, tool({ description: 'Read a UTF-8 text file. Returns contents with 1-based line numbers.', inputSchema: z.object({ path: z.string().describe('File path relative to the workspace root'), @@ -49,11 +51,11 @@ export const readFileTool = tool({ limit: z.number().int().min(1).optional().describe('Max lines to return, default 2000'), }), execute: async ({ path, offset = 1, limit = 2000 }) => cap(await readNumbered(path, offset, limit)), -}); +})); const MAX_BATCH_FILES = 20; -export const readManyFilesTool = tool({ +export const readManyFilesTool = withMeta({ set: 'edit-plus', mutating: false }, tool({ description: 'Read several text files in one call. Use it when you already know which files you need — one round trip ' + 'instead of one per file. Each file may set its own offset and limit. A path that cannot be read is reported ' + @@ -85,7 +87,7 @@ export const readManyFilesTool = tool({ ); return cap(blocks.join('\n\n')); }, -}); +})); export type PatchOp = | { kind: 'add'; path: string; content: string } @@ -174,7 +176,7 @@ export function parsePatch(patch: string): PatchOp[] { return ops; } -export const applyPatchTool = tool({ +export const applyPatchTool = withMeta({ set: 'edit-plus', mutating: true }, tool({ description: 'Apply one patch across several files: add, update, move, and delete in a single call. All or nothing — if any ' + 'part fails, nothing is written. Use it when a change spans files that must land together, such as a rename ' + @@ -248,7 +250,7 @@ export const applyPatchTool = tool({ return `Applied ${ops.length} change${ops.length === 1 ? '' : 's'}:\n${summary.map((s) => `- ${s}`).join('\n')}`; }, -}); +})); /** * A rewrite that collapses whitespace: similar character count, a fraction of the lines. @@ -264,7 +266,7 @@ function collapsedRewrite(before: string, after: string): boolean { return after.split('\n').length < before.split('\n').length / 2; } -export const writeFileTool = tool({ +export const writeFileTool = withMeta({ set: 'core', mutating: true }, tool({ description: 'Create a file or overwrite it completely. Prefer edit_file for existing files.', inputSchema: z.object({ path: z.string(), @@ -284,9 +286,9 @@ export const writeFileTool = tool({ } return `Wrote ${content.length} chars to ${path}`; }, -}); +})); -export const editFileTool = tool({ +export const editFileTool = withMeta({ set: 'core', mutating: true }, tool({ description: 'Replace an exact string in a file. oldString must appear exactly once unless replaceAll is true. Include surrounding context to make oldString unique.', inputSchema: z.object({ @@ -312,9 +314,9 @@ export const editFileTool = tool({ await Bun.write(abs, after); return `Replaced ${replaceAll ? count : 1} occurrence(s) in ${path}`; }, -}); +})); -export const multiEditTool = tool({ +export const multiEditTool = withMeta({ set: 'edit-plus', mutating: true }, tool({ description: 'Apply several exact-string edits to one file in a single call. Each edit sees the result of the previous one. ' + 'All or nothing: if any oldString fails to match, or matches more than once without replaceAll, nothing is ' + @@ -367,9 +369,9 @@ export const multiEditTool = tool({ await Bun.write(abs, text); return `Applied ${edits.length} edit(s) to ${path} (${applied.join(', ')})`; }, -}); +})); -export const globTool = tool({ +export const globTool = withMeta({ set: 'core', mutating: false }, tool({ description: 'Find files by glob pattern, e.g. "src/**/*.ts". Skips anything .gitignore excludes. Returns paths relative to the workspace root.', inputSchema: z.object({ @@ -387,11 +389,11 @@ export const globTool = tool({ } return hits.length ? hits.join('\n') : 'No files matched.'; }, -}); +})); const MAX_TREE_ENTRIES = 300; -export const listDirTool = tool({ +export const listDirTool = withMeta({ set: 'edit-plus', mutating: false }, tool({ description: 'Directory tree, honouring .gitignore. Use it first to orient yourself in an unfamiliar project instead of ' + 'guessing at glob patterns. Directories end with /, files show their size.', @@ -430,7 +432,7 @@ export const listDirTool = tool({ const capped = rows.length >= MAX_TREE_ENTRIES ? `\n... [${MAX_TREE_ENTRIES}-entry limit reached]` : ''; return cap(`${label}\n${rows.map((r) => r.line).join('\n')}${capped}`); }, -}); +})); async function isDir(abs: string): Promise { try { @@ -536,7 +538,7 @@ async function grepInJs({ pattern, include = '**/*', ignoreCase, includeIgnored return hits.length ? cap(hits.join('\n')) : 'No matches.'; } -export const grepTool = tool({ +export const grepTool = withMeta({ set: 'core', mutating: false }, tool({ description: 'Search file contents with a regular expression. Skips binaries and anything .gitignore excludes. Returns path:line:text hits.', inputSchema: z.object({ @@ -546,7 +548,7 @@ export const grepTool = tool({ includeIgnored: z.boolean().optional().describe('Also search files git ignores'), }), execute: async (args) => (await grepWithRipgrep(args)) ?? (await grepInJs(args)), -}); +})); export type BashOutput = { toolCallId: string; chunk: string }; @@ -622,7 +624,7 @@ export function interruptBash(): string[] { return killed; } -export const bashTool = tool({ +export const bashTool = withMeta({ set: 'core', mutating: true }, tool({ description: 'Run a shell command in the workspace root. Use for builds, tests, git, and package managers. ' + 'Output streams live and the user can interrupt a command with ctrl-c without ending the turn.', @@ -684,9 +686,9 @@ export const bashTool = tool({ running.delete(toolCallId); } }, -}); +})); -export const moveFileTool = tool({ +export const moveFileTool = withMeta({ set: 'edit-plus', mutating: true }, tool({ description: 'Move or rename one file. Creates the target directory. Refuses if the source is missing or the target ' + 'already exists, so a rename cannot silently overwrite work. For a rename plus its callers in one step, ' + @@ -708,9 +710,9 @@ export const moveFileTool = tool({ await file.delete(); return `Moved ${from} to ${to}`; }, -}); +})); -export const deleteFileTool = tool({ +export const deleteFileTool = withMeta({ set: 'edit-plus', mutating: true }, tool({ description: 'Delete one file. Refuses a directory: removing a tree is what the guard plugin blocks in bash, and it is ' + 'not something to do implicitly. Delete the files you mean, one call each.', @@ -733,7 +735,7 @@ export const deleteFileTool = tool({ await Bun.file(abs).delete(); return `Deleted ${path} (${entry.size} bytes)`; }, -}); +})); /** * Definition patterns for `find_symbol`, keyed loosely by language. @@ -756,7 +758,7 @@ const escapeRe = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const MAX_SYMBOL_HITS = 40; -export const findSymbolTool = tool({ +export const findSymbolTool = withMeta({ set: 'nav', mutating: false }, tool({ description: 'Locate where a function, class, type, or constant is *defined*, across JS/TS, Python, Go, and Rust. ' + 'Returns path:line hits. Faster and more precise than grep for "where is X declared", because it matches ' + @@ -795,7 +797,7 @@ export const findSymbolTool = tool({ } return hits.length ? cap(hits.join('\n')) : `No definition of "${trimmed}" found.`; }, -}); +})); /** * A dotted-path lookup into a JSON document, so a large manifest, lockfile, or @@ -803,7 +805,7 @@ export const findSymbolTool = tool({ * `a.b.0.c` walks objects and arrays; a missing segment reports the path that * resolved, so a wrong key is diagnosable rather than a bare "undefined". */ -export const jsonQueryTool = tool({ +export const jsonQueryTool = withMeta({ set: 'nav', mutating: false }, tool({ description: 'Read one value out of a JSON file by dotted path (e.g. "scripts.build" or "dependencies.react"). ' + 'Use it on large manifests and configs instead of reading the whole file into context.', @@ -841,7 +843,7 @@ export const jsonQueryTool = tool({ const rendered = typeof node === 'string' ? node : JSON.stringify(node, null, 2); return cap(`${query} = ${rendered}`); }, -}); +})); export const tools = { read_file: readFileTool, @@ -864,26 +866,13 @@ export const tools = { }; /** - * Tool sets, so a set can be switched off before the schema cost grows. - * - * Measured at ~550 chars of JSON schema per tool on every request, and selection - * accuracy falls as the list grows, so this is both a cost and a quality knob. - * `core` is not listable here: without read, edit, and bash the agent is not an agent. - * - * `net` is the exception that is off unless asked for. Every other tool stays inside - * the workspace; `web_fetch` reaches the internet and brings a stranger's text back - * into the context, which is a decision rather than a default. + * Tool sets, derived from each tool's `_meta` at its definition site. + * A tool added in one file and forgotten in a list is impossible — the set + * follows the definition, not a parallel hand-list. */ -export const TOOL_SETS = { - core: ['read_file', 'write_file', 'edit_file', 'glob', 'grep', 'bash'], - 'edit-plus': ['multi_edit', 'list_dir', 'read_many_files', 'apply_patch', 'move_file', 'delete_file'], - nav: ['find_symbol', 'json_query'], - extra: EXTRA_TOOL_NAMES, - git: GIT_TOOL_NAMES, - net: NET_TOOL_NAMES, -} as const satisfies Record; +export const TOOL_SETS: Record = setsFrom(tools); -export type ToolSetName = keyof typeof TOOL_SETS; +export type ToolSetName = DerivedToolSetName; export const TOOL_SET_NAMES = Object.keys(TOOL_SETS) as ToolSetName[]; @@ -909,15 +898,7 @@ export function disabledToolNames(enabled: readonly ToolSetName[] | undefined): return TOOL_SET_NAMES.filter((set) => !live.has(set)).flatMap((set) => [...TOOL_SETS[set]]); } -/** Tools that mutate the workspace or run arbitrary code always ask the user first. */ -export const MUTATING_TOOLS = [ - 'write_file', - 'edit_file', - 'multi_edit', - 'apply_patch', - 'move_file', - 'delete_file', - 'bash', -] as const; +/** Tools that mutate the workspace or run arbitrary code always ask the user first. Derived from `_meta` so a new write cannot be added without being gated. */ +export const MUTATING_TOOLS: readonly string[] = mutatingNames(tools); export { jail }; diff --git a/test/tool-derive.test.ts b/test/tool-derive.test.ts new file mode 100644 index 0000000..1ce51a4 --- /dev/null +++ b/test/tool-derive.test.ts @@ -0,0 +1,50 @@ +import { expect, test } from 'bun:test'; +import { DEFAULT_PERMISSIONS } from '../src/permission'; +import { tools, TOOL_SETS, MUTATING_TOOLS, TOOL_SET_NAMES } from '../src/tools'; +import { assertAllToolsInSets, mutatingNames, setsFrom, type ToolMeta } from '../src/tool-utils'; + +test('every builtin tool carries _meta at its definition site', () => { + for (const [name, t] of Object.entries(tools as Record)) { + const meta = t._meta; + expect(meta, `${name} has no _meta — wrap it with withMeta at the definition site`).toBeDefined(); + expect(TOOL_SET_NAMES as readonly string[]).toContain(meta!.set); + expect(typeof meta!.mutating).toBe('boolean'); + } +}); + +test('TOOL_SETS is derived from _meta and covers every builtin tool exactly once', () => { + const derived = setsFrom(tools as Record); + expect(TOOL_SETS).toEqual(derived); + + const missing = assertAllToolsInSets(tools, TOOL_SETS); + expect(missing).toEqual([]); + + // no tool belongs to two sets + const all = Object.values(TOOL_SETS).flat(); + expect(new Set(all).size).toBe(all.length); + expect(all.length).toBe(Object.keys(tools).length); + + // every name in a set is a real tool + for (const name of all) expect(tools).toHaveProperty(name); +}); + +test('MUTATING_TOOLS is exactly the set of tools with _meta.mutating', () => { + const derived = mutatingNames(tools as Record); + expect([...MUTATING_TOOLS].sort()).toEqual(derived); +}); + +test('every mutating tool has a DEFAULT_PERMISSIONS entry (otherwise it would run ungated)', () => { + for (const name of MUTATING_TOOLS) { + expect(DEFAULT_PERMISSIONS[name], `${name} mutates but has no DEFAULT_PERMISSIONS entry`).toBeDefined(); + } +}); + +test('a mutating tool outside MUTATING_TOOLS would be caught: the derive is the single source of truth', () => { + // This is the invariant the hand-list used to break: a tool with mutating:true + // must appear in MUTATING_TOOLS, and a tool without it must not. + for (const [name, t] of Object.entries(tools as Record)) { + const isMutating = (t._meta?.mutating ?? false); + const inList = (MUTATING_TOOLS as readonly string[]).includes(name); + expect(inList, `${name}: _meta.mutating=${isMutating} but MUTATING_TOOLS includes=${inList}`).toBe(isMutating); + } +});