diff --git a/src/permission.ts b/src/permission.ts index 4402ddf..206f918 100644 --- a/src/permission.ts +++ b/src/permission.ts @@ -68,6 +68,18 @@ export function subjectOf(tool: string, input: unknown): string | undefined { case 'list_dir': case 'git_blame': return str('path'); + case 'web_fetch': + return str('url'); + case 'apply_patch': { + // Every path the patch touches, so denying `src/generated/*` catches a patch + // that includes one alongside files it may edit. + const patch = str('patch'); + if (!patch) return undefined; + const paths = [...patch.matchAll(/^\*\*\* (?:Add|Update|Delete) File: (.+)$/gm)].map((m) => m[1]!.trim()); + const moves = [...patch.matchAll(/^\*\*\* Move to: (.+)$/gm)].map((m) => m[1]!.trim()); + const all = [...paths, ...moves]; + return all.length > 0 ? all.join(' ') : undefined; + } case 'read_many_files': { // A batch read is gated on the paths it asks for, so one bad path in // twenty is enough to trigger a rule. @@ -97,11 +109,14 @@ export function subjectOf(tool: string, input: unknown): string | undefined { } /** - * A read_many_files subject is several paths at once, so a rule has to match if - * it matches any of them: denying `*.env` must catch a batch that includes one. + * A subject that is several strings at once is matched if a rule matches any of + * them: denying `*.env` must catch a batch read that includes one, and denying + * `src/generated/*` must catch a patch that touches one among five files. */ +const MULTI = new Set(['read_many_files', 'apply_patch']); + const subjectsFor = (tool: string, subject: string): string[] => - tool === 'read_many_files' ? subject.split(' ') : [subject]; + MULTI.has(tool) ? subject.split(' ') : [subject]; export type Resolved = { decision: Decision; pattern: string | undefined }; @@ -154,7 +169,9 @@ export const DEFAULT_PERMISSIONS: PermissionConfig = { write_file: 'ask', edit_file: 'ask', multi_edit: 'ask', + apply_patch: 'ask', bash: 'ask', + web_fetch: 'ask', }; /** Session, plugin, and read-only tools that never gate. */ diff --git a/src/plugins-builtin.ts b/src/plugins-builtin.ts index b365beb..672ce7a 100644 --- a/src/plugins-builtin.ts +++ b/src/plugins-builtin.ts @@ -66,9 +66,116 @@ export const timePlugin: Plugin = { }, }; -export const BUILTIN_PLUGINS: Plugin[] = [guardPlugin, bellPlugin, timePlugin]; +/** + * Paths that hold credentials. + * + * The permission defaults already refuse to *read* these. This refuses to write + * them, which is a different failure: a model asked to "add the API key to the env + * file" will do exactly that, and a secret committed by an agent is a secret to + * rotate. The user writes their own credentials. + */ +const SECRET_PATHS: { re: RegExp; why: string }[] = [ + // `.env.example` holds placeholders by convention and is the one such file a + // model legitimately writes, so it is excluded here rather than by a later rule. + { re: /(^|[\\/])\.env(?!\.example$)(\.|$)/i, why: 'an env file' }, + { re: /\.(pem|key|p12|pfx|jks|keystore)$/i, why: 'a key or certificate' }, + { re: /(^|[\\/])(id_rsa|id_ed25519|id_ecdsa|id_dsa)(\.pub)?$/i, why: 'an SSH key' }, + { re: /(^|[\\/])\.(npmrc|pypirc|netrc|pgpass)$/i, why: 'a registry or database credential file' }, + { re: /(^|[\\/])(credentials|secrets?)\.(json|ya?ml|toml|ini)$/i, why: 'a credentials file' }, + { re: /(^|[\\/])\.aws[\\/]/i, why: 'an AWS credential directory' }, + { re: /(^|[\\/])\.ssh[\\/]/i, why: 'an SSH directory' }, + { re: /(^|[\\/])\.gnupg[\\/]/i, why: 'a GPG directory' }, +]; -/** Enabled unless the config turns them off. bell is opt-in; a bell per turn is intrusive. */ -export const DEFAULT_ENABLED = ['guard', 'time']; +/** Every path a write tool might carry, including a patch's markers. */ +function writtenPaths(toolName: string, input: unknown): string[] { + const o = (input ?? {}) as Record; -export { DESTRUCTIVE }; + if (toolName === 'apply_patch') { + const patch = typeof o['patch'] === 'string' ? o['patch'] : ''; + return [ + ...[...patch.matchAll(/^\*\*\* (?:Add|Update|Delete) File: (.+)$/gm)].map((m) => m[1]!.trim()), + ...[...patch.matchAll(/^\*\*\* Move to: (.+)$/gm)].map((m) => m[1]!.trim()), + ]; + } + + return typeof o['path'] === 'string' ? [o['path']] : []; +} + +export const secretsPlugin: Plugin = { + name: 'secrets', + description: 'refuses to write credential files', + appendix: + 'The secrets plugin refuses writes to env files, keys, and credential stores. If one needs a value, tell the ' + + 'user which file and which key, and let them write it themselves. Do not work around the refusal by writing ' + + 'the same content somewhere else.', + beforeToolCall: ({ toolName, input }) => { + if (!['write_file', 'edit_file', 'multi_edit', 'apply_patch'].includes(toolName)) return undefined; + + for (const path of writtenPaths(toolName, input)) { + for (const { re, why } of SECRET_PATHS) { + if (re.test(path)) { + return `refusing to write ${path} (${why}). Tell the user what to put there and let them write it.`; + } + } + } + return undefined; + }, +}; + +const FORMATTERS: { file: string; script: string; command: string[] }[] = [ + { file: 'package.json', script: 'format', command: ['bun', 'run', 'format'] }, + { file: 'Cargo.toml', script: '', command: ['cargo', 'fmt'] }, + { file: 'go.mod', script: '', command: ['gofmt', '-w', '.'] }, +]; + +/** + * Runs the project's own formatter once a turn ends, if it has one. + * + * Off by default. It is useful — a diff without formatting noise reviews faster — + * but it writes to files after the approvals for that turn are over, which is a + * boundary worth crossing only on purpose. + * + * It runs the script the project already defines rather than shipping opinions + * about style. No `package.json` `format` script means nothing happens. + */ +export const formatPlugin: Plugin = { + name: 'format', + description: "runs the project's own formatter after each turn", + afterTurn: async () => { + for (const { file, script, command } of FORMATTERS) { + const manifest = Bun.file(file); + if (!(await manifest.exists())) continue; + + if (script) { + try { + const pkg = (await manifest.json()) as { scripts?: Record }; + if (!pkg.scripts?.[script]) continue; + } catch { + continue; + } + } + + try { + const proc = Bun.spawn(command, { stdout: 'ignore', stderr: 'ignore', timeout: 60_000 }); + await proc.exited; + } catch { + // A missing binary is not worth interrupting the turn over. + } + return; + } + }, +}; + +export const BUILTIN_PLUGINS: Plugin[] = [guardPlugin, secretsPlugin, bellPlugin, timePlugin, formatPlugin]; + +/** + * Enabled unless the config turns them off. + * + * `guard` and `secrets` are refusals, so they are on: a user who has to opt into a + * safety check does not have it. `bell` and `format` both act on their own — one + * makes noise, the other writes files — so they are opt-in. + */ +export const DEFAULT_ENABLED = ['guard', 'secrets', 'time']; + +export { DESTRUCTIVE, SECRET_PATHS }; diff --git a/src/tools.ts b/src/tools.ts index 6547e34..91e01ec 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -4,6 +4,7 @@ import { join, resolve } from 'node:path'; import { z } from 'zod'; import { jail, posix, walk } from './ignore'; import { GIT_TOOL_NAMES, gitTools } from './tools-git'; +import { NET_TOOL_NAMES, netTools } from './tools-net'; /** Max chars returned by any single tool. Beyond this the output is truncated. */ const MAX_OUTPUT = 30_000; @@ -85,6 +86,169 @@ export const readManyFilesTool = tool({ }, }); +export type PatchOp = + | { kind: 'add'; path: string; content: string } + | { kind: 'update'; path: string; moveTo?: string; oldString: string; newString: string } + | { kind: 'delete'; path: string }; + +const MARKER = /^\*\*\* (Add|Update|Delete) File: (.+)$/; +const MOVE = /^\*\*\* Move to: (.+)$/; + +/** + * Parses the patch envelope. Exported so the format is testable without a disk. + * + * The shape follows Codex's `apply_patch`, which is worth copying for one reason: + * models have seen it. A bespoke format costs schema description and gets malformed + * calls until the model learns it. + * + * *** Add File: src/new.ts + * +export const a = 1; + * *** Update File: src/old.ts + * *** Move to: src/renamed.ts + * -const a = 1; + * +const a = 2; + * *** Delete File: src/gone.ts + */ +export function parsePatch(patch: string): PatchOp[] { + const lines = patch.replace(/\r\n/g, '\n').split('\n'); + const ops: PatchOp[] = []; + let i = 0; + + while (i < lines.length) { + const line = lines[i]!; + if (line.trim().length === 0) { + i++; + continue; + } + + const marker = MARKER.exec(line); + if (!marker) throw new Error(`patch line ${i + 1} is not a marker or part of a hunk: ${line.slice(0, 60)}`); + + const kind = marker[1]!.toLowerCase() as 'add' | 'update' | 'delete'; + const path = marker[2]!.trim(); + i++; + + if (kind === 'delete') { + ops.push({ kind: 'delete', path }); + continue; + } + + let moveTo: string | undefined; + const move = i < lines.length ? MOVE.exec(lines[i]!) : null; + if (move) { + if (kind === 'add') throw new Error(`${path}: "Move to" is only valid on an Update`); + moveTo = move[1]!.trim(); + i++; + } + + const removed: string[] = []; + const added: string[] = []; + while (i < lines.length && !MARKER.test(lines[i]!)) { + const body = lines[i]!; + if (body.startsWith('+')) added.push(body.slice(1)); + else if (body.startsWith('-')) removed.push(body.slice(1)); + else if (body.trim().length > 0) { + throw new Error(`${path}: hunk line ${i + 1} starts with neither + nor -: ${body.slice(0, 60)}`); + } + i++; + } + + if (kind === 'add') { + if (removed.length > 0) throw new Error(`${path}: an Add cannot remove lines`); + ops.push({ kind: 'add', path, content: added.join('\n') }); + continue; + } + + if (removed.length === 0) throw new Error(`${path}: an Update needs at least one - line to locate the change`); + ops.push({ + kind: 'update', + path, + ...(moveTo ? { moveTo } : {}), + oldString: removed.join('\n'), + newString: added.join('\n'), + }); + } + + if (ops.length === 0) throw new Error('the patch is empty'); + return ops; +} + +export const applyPatchTool = 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 ' + + 'plus its callers. For several edits to one file use multi_edit; for one edit use edit_file.\n' + + 'Format, one marker per file:\n' + + '*** Add File: path then + lines for the whole new file\n' + + '*** Update File: path then - lines to find and + lines to replace them with\n' + + '*** Move to: path directly after an Update marker, to rename\n' + + '*** Delete File: path no hunk\n' + + 'The - lines must match the file byte-for-byte and appear exactly once.', + inputSchema: z.object({ + patch: z.string().describe('The patch envelope, as described above'), + }), + execute: async ({ patch }) => { + const ops = parsePatch(patch); + + // Every operation is resolved and validated against the real files before + // anything is written. A patch that fails on its fourth file must not leave + // the first three applied — that is the only reason to have this tool rather + // than a sequence of edit_file calls. + const writes: { abs: string; content: string }[] = []; + const removals: string[] = []; + const summary: string[] = []; + const seen = new Set(); + + for (const op of ops) { + if (seen.has(op.path)) throw new Error(`${op.path} appears twice in one patch`); + seen.add(op.path); + const abs = jail(op.path); + + if (op.kind === 'delete') { + if (!(await Bun.file(abs).exists())) throw new Error(`cannot delete ${op.path}: no such file`); + removals.push(abs); + summary.push(`deleted ${op.path}`); + continue; + } + + if (op.kind === 'add') { + if (await Bun.file(abs).exists()) throw new Error(`cannot add ${op.path}: it already exists`); + writes.push({ abs, content: op.content.endsWith('\n') ? op.content : `${op.content}\n` }); + summary.push(`added ${op.path}`); + continue; + } + + const file = Bun.file(abs); + if (!(await file.exists())) throw new Error(`cannot update ${op.path}: no such file`); + if (await isBinary(abs)) throw new Error(`cannot update ${op.path}: it is a binary file`); + + const before = await file.text(); + const count = before.split(op.oldString).length - 1; + if (count === 0) throw new Error(`${op.path}: the - lines do not match the file. Nothing was written.`); + if (count > 1) { + throw new Error(`${op.path}: the - lines appear ${count} times. Add context to make them unique.`); + } + + const after = before.replace(op.oldString, op.newString); + if (op.moveTo) { + const target = jail(op.moveTo); + if (await Bun.file(target).exists()) throw new Error(`cannot move ${op.path}: ${op.moveTo} already exists`); + writes.push({ abs: target, content: after }); + removals.push(abs); + summary.push(`moved ${op.path} to ${op.moveTo}`); + } else { + writes.push({ abs, content: after }); + summary.push(`updated ${op.path}`); + } + } + + for (const { abs, content } of writes) await Bun.write(abs, content); + for (const abs of removals) await Bun.file(abs).delete(); + + return `Applied ${ops.length} change${ops.length === 1 ? '' : 's'}:\n${summary.map((s) => `- ${s}`).join('\n')}`; + }, +}); + export const writeFileTool = tool({ description: 'Create a file or overwrite it completely. Prefer edit_file for existing files.', inputSchema: z.object({ @@ -504,11 +668,13 @@ export const tools = { write_file: writeFileTool, edit_file: editFileTool, multi_edit: multiEditTool, + apply_patch: applyPatchTool, list_dir: listDirTool, glob: globTool, grep: grepTool, bash: bashTool, ...gitTools, + ...netTools, }; /** @@ -517,11 +683,16 @@ export const tools = { * 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. */ export const TOOL_SETS = { core: ['read_file', 'write_file', 'edit_file', 'glob', 'grep', 'bash'], - 'edit-plus': ['multi_edit', 'list_dir', 'read_many_files'], + 'edit-plus': ['multi_edit', 'list_dir', 'read_many_files', 'apply_patch'], git: GIT_TOOL_NAMES, + net: NET_TOOL_NAMES, } as const satisfies Record; export type ToolSetName = keyof typeof TOOL_SETS; @@ -530,6 +701,9 @@ export const TOOL_SET_NAMES = Object.keys(TOOL_SETS) as ToolSetName[]; export const isToolSetName = (v: string): v is ToolSetName => (TOOL_SET_NAMES as string[]).includes(v); +/** Sets offered when the config says nothing. `net` is opt-in. */ +export const DEFAULT_TOOL_SETS: ToolSetName[] = ['core', 'edit-plus', 'git']; + /** Which set a tool came from, for `/tools`. Session, plugin, and MCP tools have none. */ export function toolSetOf(name: string): ToolSetName | undefined { return TOOL_SET_NAMES.find((set) => (TOOL_SETS[set] as readonly string[]).includes(name)); @@ -538,14 +712,16 @@ export function toolSetOf(name: string): ToolSetName | undefined { /** * Names to withhold given the enabled sets. A tool belonging to no set is never * withheld: session, plugin, and MCP tools are not part of this budget. + * + * Omitting `toolSets` entirely means the defaults, not everything — `net` has to be + * asked for by name. */ export function disabledToolNames(enabled: readonly ToolSetName[] | undefined): string[] { - if (!enabled) return []; - const live = new Set([...enabled, 'core']); + const live = new Set([...(enabled ?? DEFAULT_TOOL_SETS), 'core']); 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', 'bash'] as const; +export const MUTATING_TOOLS = ['write_file', 'edit_file', 'multi_edit', 'apply_patch', 'bash'] as const; export { jail }; diff --git a/test/plugins-builtin.test.ts b/test/plugins-builtin.test.ts new file mode 100644 index 0000000..8f69399 --- /dev/null +++ b/test/plugins-builtin.test.ts @@ -0,0 +1,142 @@ +import { expect, test } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createHost } from '../src/plugins'; +import { BUILTIN_PLUGINS, DEFAULT_ENABLED, formatPlugin, secretsPlugin } from '../src/plugins-builtin'; + +const cwd = process.cwd(); +const check = (toolName: string, input: unknown) => secretsPlugin.beforeToolCall!({ toolName, input, cwd }); + +test('secrets is on by default, because an opt-in safety check is not one', () => { + expect(DEFAULT_ENABLED).toContain('secrets'); + expect(DEFAULT_ENABLED).toContain('guard'); + // Both of these act on their own rather than refusing, so they are opt-in. + expect(DEFAULT_ENABLED).not.toContain('bell'); + expect(DEFAULT_ENABLED).not.toContain('format'); +}); + +test('every builtin plugin has a unique name and a description', () => { + const names = BUILTIN_PLUGINS.map((p) => p.name); + expect(new Set(names).size).toBe(names.length); + for (const p of BUILTIN_PLUGINS) expect(p.description.length, p.name).toBeGreaterThan(5); +}); + +test('writing an env file is refused', async () => { + for (const path of ['.env', 'config/.env', 'app/.env.production', '.env.local']) { + expect(await check('write_file', { path }), path).toContain('refusing to write'); + } +}); + +test('writing a key or certificate is refused', async () => { + for (const path of ['certs/server.pem', 'app.key', 'store.p12', 'keys/id_rsa', '.ssh/id_ed25519']) { + expect(await check('write_file', { path }), path).toContain('refusing to write'); + } +}); + +test('writing a registry or credential file is refused', async () => { + for (const path of ['.npmrc', '.netrc', 'credentials.json', 'secrets.yaml', '.aws/config']) { + expect(await check('write_file', { path }), path).toContain('refusing to write'); + } +}); + +test('every write tool is covered, including apply_patch', async () => { + for (const tool of ['write_file', 'edit_file', 'multi_edit']) { + expect(await check(tool, { path: '.env' }), tool).toContain('refusing to write'); + } + + // A patch carries its paths in markers, so a guard that only reads `path` is + // bypassed by the one tool that can touch several files at once. + const patch = '*** Update File: src/app.ts\n-a\n+b\n*** Add File: .env\n+SECRET=x'; + expect(await check('apply_patch', { patch })).toContain('refusing to write'); + expect(await check('apply_patch', { patch: '*** Move to: .env.production\n' })).toContain('refusing to write'); +}); + +test('ordinary source files are untouched', async () => { + for (const path of ['src/app.ts', 'README.md', 'docs/env-vars.md', 'test/environment.test.ts', 'keychain.ts']) { + expect(await check('write_file', { path }), path).toBeUndefined(); + } +}); + +test('an example env file may be written, since it holds no secret', async () => { + // The permission defaults allow *reading* .env.example for the same reason. + expect(await check('write_file', { path: '.env.example' })).toBeUndefined(); +}); + +test('reading is not the secrets plugin′s business', async () => { + // Reads are refused by the permission defaults instead, so this must not also + // block them: two mechanisms refusing the same thing means one is dead code. + expect(await check('read_file', { path: '.env' })).toBeUndefined(); + expect(await check('bash', { command: 'cat .env' })).toBeUndefined(); +}); + +test('the appendix tells the model what to do instead', () => { + expect(secretsPlugin.appendix).toContain('let them write it'); +}); + +test('the guard and secrets both run, and the first refusal wins', async () => { + const host = createHost(BUILTIN_PLUGINS.filter((p) => DEFAULT_ENABLED.includes(p.name))); + + expect(await host.guard({ toolName: 'bash', input: { command: 'rm -rf /' }, cwd })).toContain('guard plugin'); + expect(await host.guard({ toolName: 'write_file', input: { path: '.env' }, cwd })).toContain('secrets plugin'); + expect(await host.guard({ toolName: 'write_file', input: { path: 'src/app.ts' }, cwd })).toBeUndefined(); +}); + +function inTempDir(fn: (dir: string) => Promise): Promise { + const orig = process.cwd(); + const dir = mkdtempSync(join(tmpdir(), 'shiro-fmt-')); + process.chdir(dir); + return fn(dir).finally(() => { + process.chdir(orig); + rmSync(dir, { recursive: true, force: true }); + }); +} + +test('format runs the script the project already defines', async () => + inTempDir(async (dir) => { + // A script that touches a file, so the assertion is that the formatter ran + // rather than that some formatter binary happens to be installed. + await Bun.write(join(dir, 'marker.js'), 'require("fs").writeFileSync("formatted.txt", "yes")'); + await Bun.write(join(dir, 'package.json'), JSON.stringify({ scripts: { format: 'node marker.js' } })); + + await formatPlugin.afterTurn!(); + expect(await Bun.file(join(dir, 'formatted.txt')).text()).toBe('yes'); + }), 30_000); + +test('format does nothing when the project defines no formatter', async () => + inTempDir(async (dir) => { + await Bun.write(join(dir, 'package.json'), JSON.stringify({ scripts: { test: 'bun test' } })); + // No throw and no side effect: a project without a format script gets nothing. + await formatPlugin.afterTurn!(); + expect(await Bun.file(join(dir, 'formatted.txt')).exists()).toBe(false); + }), 30_000); + +test('format survives a manifest it cannot parse', async () => + inTempDir(async (dir) => { + await Bun.write(join(dir, 'package.json'), '{ not json'); + await formatPlugin.afterTurn!(); + expect(true).toBe(true); + }), 30_000); + +test('format does nothing in a directory with no manifest at all', async () => + inTempDir(async () => { + await formatPlugin.afterTurn!(); + expect(true).toBe(true); + }), 30_000); + +test('a throwing afterTurn does not stop the other plugins', async () => { + let ran = 0; + const host = createHost([ + { + name: 'broken', + description: 'throws', + afterTurn: () => { + throw new Error('boom'); + }, + }, + { name: 'counter', description: 'counts', afterTurn: () => void ran++ }, + ]); + + await host.afterTurn(); + expect(ran).toBe(1); +}); diff --git a/test/tools.test.ts b/test/tools.test.ts index dd5dcdf..006a77b 100644 --- a/test/tools.test.ts +++ b/test/tools.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { + applyPatchTool, bashTool, editFileTool, globTool, @@ -12,6 +13,7 @@ import { listDirTool, multiEditTool, onBashOutput, + parsePatch, readFileTool, readManyFilesTool, writeFileTool, @@ -229,6 +231,134 @@ test('list_dir scopes to a subdirectory and refuses a file', async () => { expect(run(listDirTool, { path: 'other.ts' })).rejects.toThrow(/Not a directory/); }); +const PATCH_ADD = `*** Add File: src/new.ts ++export const a = 1; ++export const b = 2;`; + +test('parsePatch reads add, update, move, and delete markers', () => { + const ops = parsePatch(`${PATCH_ADD} +*** Update File: src/old.ts +*** Move to: src/renamed.ts +-const port = 8080; ++const port = 9090; +*** Delete File: src/gone.ts`); + + expect(ops).toEqual([ + { kind: 'add', path: 'src/new.ts', content: 'export const a = 1;\nexport const b = 2;' }, + { + kind: 'update', + path: 'src/old.ts', + moveTo: 'src/renamed.ts', + oldString: 'const port = 8080;', + newString: 'const port = 9090;', + }, + { kind: 'delete', path: 'src/gone.ts' }, + ]); +}); + +test('parsePatch rejects a malformed envelope rather than guessing', () => { + expect(() => parsePatch('')).toThrow(/empty/); + expect(() => parsePatch('just some text')).toThrow(/not a marker/); + expect(() => parsePatch('*** Update File: a.ts\n+only additions')).toThrow(/at least one - line/); + expect(() => parsePatch('*** Add File: a.ts\n-removing')).toThrow(/cannot remove lines/); + expect(() => parsePatch('*** Add File: a.ts\n*** Move to: b.ts\n+x')).toThrow(/only valid on an Update/); + expect(() => parsePatch('*** Update File: a.ts\nno prefix here')).toThrow(/neither \+ nor -/); +}); + +test('apply_patch adds, updates, moves, and deletes in one call', async () => { + await Bun.write(join(dir, 'old.ts'), 'const port = 8080;\n'); + await Bun.write(join(dir, 'gone.ts'), 'obsolete\n'); + await Bun.write(join(dir, 'moving.ts'), 'const name = "a";\n'); + + const out = await run(applyPatchTool, { + patch: `*** Add File: fresh.ts ++export const fresh = true; +*** Update File: old.ts +-const port = 8080; ++const port = 9090; +*** Update File: moving.ts +*** Move to: moved.ts +-const name = "a"; ++const name = "b"; +*** Delete File: gone.ts`, + }); + + expect(out).toContain('4 changes'); + expect(await Bun.file(join(dir, 'fresh.ts')).text()).toBe('export const fresh = true;\n'); + expect(await Bun.file(join(dir, 'old.ts')).text()).toBe('const port = 9090;\n'); + expect(await Bun.file(join(dir, 'moved.ts')).text()).toBe('const name = "b";\n'); + expect(await Bun.file(join(dir, 'moving.ts')).exists()).toBe(false); + expect(await Bun.file(join(dir, 'gone.ts')).exists()).toBe(false); +}); + +test('a patch that fails on its third file writes nothing at all', async () => { + await Bun.write(join(dir, 'one.ts'), 'const a = 1;\n'); + await Bun.write(join(dir, 'two.ts'), 'const b = 2;\n'); + + expect( + run(applyPatchTool, { + patch: `*** Update File: one.ts +-const a = 1; ++const a = 10; +*** Update File: two.ts +-const b = 2; ++const b = 20; +*** Update File: three.ts +-const c = 3; ++const c = 30;`, + }), + ).rejects.toThrow(/no such file/); + + await Bun.sleep(20); + // Atomicity is the whole reason this tool exists rather than three edit_file + // calls, so the first two files must be untouched. + expect(await Bun.file(join(dir, 'one.ts')).text()).toBe('const a = 1;\n'); + expect(await Bun.file(join(dir, 'two.ts')).text()).toBe('const b = 2;\n'); +}); + +test('apply_patch refuses an ambiguous match without writing', async () => { + await Bun.write(join(dir, 'dup.ts'), 'x\nx\n'); + expect(run(applyPatchTool, { patch: '*** Update File: dup.ts\n-x\n+y' })).rejects.toThrow(/appear 2 times/); + await Bun.sleep(20); + expect(await Bun.file(join(dir, 'dup.ts')).text()).toBe('x\nx\n'); +}); + +test('apply_patch refuses to add over an existing file', async () => { + await Bun.write(join(dir, 'here.ts'), 'original\n'); + expect(run(applyPatchTool, { patch: '*** Add File: here.ts\n+replacement' })).rejects.toThrow(/already exists/); + await Bun.sleep(20); + expect(await Bun.file(join(dir, 'here.ts')).text()).toBe('original\n'); +}); + +test('apply_patch refuses to move onto an existing file', async () => { + await Bun.write(join(dir, 'from.ts'), 'const a = 1;\n'); + await Bun.write(join(dir, 'to.ts'), 'occupied\n'); + + expect( + run(applyPatchTool, { patch: '*** Update File: from.ts\n*** Move to: to.ts\n-const a = 1;\n+const a = 2;' }), + ).rejects.toThrow(/already exists/); + await Bun.sleep(20); + expect(await Bun.file(join(dir, 'from.ts')).text()).toBe('const a = 1;\n'); + expect(await Bun.file(join(dir, 'to.ts')).text()).toBe('occupied\n'); +}); + +test('apply_patch refuses the same file twice in one patch', async () => { + await Bun.write(join(dir, 'twice.ts'), 'a\nb\n'); + expect( + run(applyPatchTool, { patch: '*** Update File: twice.ts\n-a\n+A\n*** Update File: twice.ts\n-b\n+B' }), + ).rejects.toThrow(/appears twice/); +}); + +test('apply_patch refuses a path outside the workspace', async () => { + expect(run(applyPatchTool, { patch: '*** Add File: ../escape.ts\n+x' })).rejects.toThrow(/escapes workspace/); +}); + +test('an update that removes lines and adds none deletes them', async () => { + await Bun.write(join(dir, 'trim.ts'), 'keep\nremove me\nkeep too\n'); + await run(applyPatchTool, { patch: '*** Update File: trim.ts\n-remove me\n' }); + expect(await Bun.file(join(dir, 'trim.ts')).text()).toBe('keep\n\nkeep too\n'); +}); + test('glob skips gitignored paths and honours includeIgnored', async () => { await Bun.write(join(dir, '.gitignore'), 'dist/\n'); await Bun.write(join(dir, 'dist/app.js'), 'x'); @@ -293,7 +423,9 @@ test('bash streams output to the listener before the command exits', async () => const script = process.platform === 'win32' - ? 'echo first && ping -n 2 127.0.0.1 > nul && echo second' + ? // Unconditional sequencing: a blocked loopback makes `ping` exit 1, and `&&` + // would then skip `echo second` on machines where ICMP is filtered. + 'echo first & ping -n 2 127.0.0.1 > nul & echo second' : 'echo first; sleep 0.4; echo second'; await run(bashTool, { command: script });