From 397c4355f3275d321432c5f15b9ad16394657259 Mon Sep 17 00:00:00 2001 From: Muhammad Zakir Ramadhan <61570975+zakirkun@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:32:32 +0700 Subject: [PATCH] Document the new tools, plugins, skills, MCP panel, and item repair Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- README.md | 13 ++++--- docs/architecture.md | 25 ++++++++++++- docs/configuration.md | 2 +- docs/development.md | 7 +++- docs/mcp.md | 86 +++++++++++++++++++++++++++++++++++++++++++ docs/memory.md | 35 ++++++++++++++++++ docs/permissions.md | 5 ++- docs/plugins.md | 42 ++++++++++++++++++--- docs/skills.md | 15 +++++++- docs/tools.md | 69 +++++++++++++++++++++++++++++++--- 10 files changed, 273 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 1c465a7..5d440dc 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,9 @@ from the models that endpoint actually reports. Settings land in shiro-neko 0.1.0-beta.4 openai/gpt-5 session 0193ab2c agent: default thinking: medium cwd: /home/you/project -skills: commit, debug, refactor, review, test, verify -plugins: guard, time -approvals: ask for write_file, edit_file, multi_edit, apply_patch, bash, web_fetch, mcp__* +skills: commit, debug, migrate, perf, refactor, review, security, test, verify +plugins: guard, secrets, protect, time +approvals: ask for write_file, edit_file, multi_edit, apply_patch, move_file, delete_file, bash, web_fetch, mcp__* /help for commands > why does the pagination test fail? @@ -100,7 +100,8 @@ stops at the same approval prompt as yours. Progress streams to a panel. **Extensible from the prompt.** `/registry` browses external skills and plugins and installs them with one confirmation. A skill is shown in full before its text joins your system prompt; -a plugin is a manifest of refusal rules, never code. +a plugin is a manifest of refusal rules, never code. `/mcp add` walks you through a local or +remote MCP server — kind, name, command or URL, headers — and writes it to your config. **Remembers between sessions.** Decisions, working commands, and traps go into per-project memory that is injected at the start of every future session. @@ -112,7 +113,7 @@ record of what it already ran instead of repeating it. **Runs headless.** `shiro -p "review this diff" --json` for scripts and CI. -**Keeps the tool list affordable.** Sixteen built-in tools, grouped into sets. Each costs +**Keeps the tool list affordable.** Nineteen built-in tools, grouped into sets. Each costs about 550 characters of schema on every request, so `{ "toolSets": [] }` trims back to the six core ones and a disabled set reaches neither the wire nor the prompt. @@ -144,7 +145,7 @@ Type `/` and a menu appears, narrowing as you type. ``` /help /agent [name] /think [level] /provider /models /model -/skills /plugins /registry [search|add|remove] /init /context +/skills /plugins /registry [search|add|remove] /mcp [add|remove] /init /context /todos /notes /memory /tools /compact /cost /sessions /resume /save /clear /exit ``` diff --git a/docs/architecture.md b/docs/architecture.md index 2d10809..e187081 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -212,6 +212,18 @@ an assistant `tool-call` and the `tool` message answering it. What reaches the w reverse pairing is deliberately left alone: a call still awaiting its result is exactly what a suspended approval looks like, and dropping it would break resume. +**An item the provider no longer holds.** A reference resolves only while the item is still in +provider storage, which a resumed session or an endpoint fallback cannot count on: + +``` +404 Item with id 'msg_…' not found. +``` + +Nothing about the same history can succeed on retry, so `pruneToFit` strips every provider +`itemId` from what it sends, and `Session.run` answers that 404 by rewriting its own history +inline and running the request again — once per turn, and only when the rejection arrived before +any output, since delivered text cannot be unsent. + The pruning ladder drops reasoning first and then keeps the widest recent tool tail that fits. The SDK carries that returned message view into later steps, and the session reports compaction once per turn rather than once per step. @@ -234,6 +246,7 @@ the reasoning. | `session.ts` | the loop, approvals, compaction, event stream | | `tools.ts` | file and shell tools, tool sets, ripgrep bridge, bash streaming and interrupt | | `tools-git.ts` | read-only git tools, spawned with a fixed argv | +| `commit.ts` | `git_commit_message`, a nested model call over the staged diff | | `tools-net.ts` | `web_fetch`, private-address and redirect checks | | `ignore.ts` | gitignore-aware walker, path jail | | `complete.ts` | `@path` token extraction, ranking, insertion | @@ -251,20 +264,28 @@ the reasoning. | `prune.ts` | provider-item and tool-pairing repair | | `markdown.ts` | parser, no dependency | | `store.ts` | sessions, prompt history | +| `farewell.ts` | the exit message and its resume commands | | `config.ts` | resolution, model construction | | `providers.ts` | presets, `/models` fetch | | `pricing.ts` | USD rates | | `commands.ts` | slash registry, parsing, menu matching | | `headless.ts` | `-p` mode | | `cli.tsx` | argv, wiring, lifecycle | -| `ui/*` | Ink components | +| `ui/App.tsx` | state, the turn loop, slash-command routing | +| `ui/transcript.ts` | line types, tool argument and result formatting | +| `ui/buses.ts` | notice and subagent channels, subagent view folding | +| `ui/Approval.tsx` | the approval bridge and its prompt | +| `ui/Pickers.tsx` | command menu, shared list picker, install confirm | +| `ui/panel-bodies.ts` | `/tools`, `/cost`, `/context`, `/todos` bodies | +| `ui/Panels.tsx` | presentational panels and the status bar | +| `ui/*` | remaining Ink components | Every module is pure of the UI except `ui/`, and `ui/` never touches the SDK. The seam is the `AgentEvent` stream. ## Testing -538 tests became 647 as the suites grew; no mocking framework. `MockLanguageModelV4` from +538 tests became 713 as the suites grew; no mocking framework. `MockLanguageModelV4` from `ai/test` drives the loop; `ink-testing-library` drives the UI with real keystrokes; MCP is tested against a real stdio server subprocess; provider wire formats and the registry are tested against a local HTTP server; the interrupt path spawns a real subprocess and asserts it diff --git a/docs/configuration.md b/docs/configuration.md index 496bd89..a6587e8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -42,7 +42,7 @@ Written by `/provider`, editable by hand. Every field is optional. | `agent` | default variant: `default`, `quick`, `deep`, `plan`, `review` | | `thinking` | default level: `off`, `low`, `medium`, `high`, `max` | | `maxRetries` | retries per model call for transient failures. Default 3 | -| `plugins` | which builtin plugins to enable. Omit for `["guard", "time"]` | +| `plugins` | which builtin plugins to enable. Omit for `["guard", "secrets", "protect", "time"]` | | `toolSets` | optional tool sets beyond `core`: `edit-plus`, `git`, and `net`. Omit for the defaults; `net` is opt-in. See [tools](tools.md) | | `permission` | which calls run, ask, or are refused, matched per command or path. See [permissions](permissions.md) | | `registryUrl` | index for `/registry`. Omit for the default. See [registry](registry.md) | diff --git a/docs/development.md b/docs/development.md index 3628ac1..37f8faa 100644 --- a/docs/development.md +++ b/docs/development.md @@ -16,7 +16,7 @@ faster and the fallback path is exercised without it. ```bash bun run shiro # run from source bun run typecheck # tsc --noEmit -bun test # 647 tests +bun test # 713 tests bun run build # single binary for this platform -> dist/shiro bun run release # all five platforms -> dist/release + SHA256SUMS bun run install:local # build, then copy onto PATH @@ -71,6 +71,9 @@ mock-verification test: request body - `pruneMessages` leaving a tool result without its tool call — same, and it took a stub endpoint that rejected the pairing to prove the fix +- A provider item the server had dropped — visible only as a 404 from a stub endpoint that + refused any `item_reference`, and only fixable by comparing the two request bodies the + session sent - Compaction blanking the model's memory of its own tool calls — invisible in any single request, and visible only as "the loop ran to its step limit". Caught by asserting the loop terminated because the model chose to, not that the messages had a particular shape @@ -95,7 +98,7 @@ Steps 3 and 4 are two hand-maintained lists of tool names, which is a known weak 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). -Every tool costs roughly 550 characters of schema on every request. Sixteen built-in tools is +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 needs to earn its place — see [ROADMAP.md](../ROADMAP.md) for what has been declined and why. One set, `net`, is opt-in rather than on: `web_fetch` is the one tool that leaves the machine. diff --git a/docs/mcp.md b/docs/mcp.md index 8af150a..0393da1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1,10 +1,96 @@ # MCP +Model Context Protocol servers contribute tools to the agent. Two transports: a local +command over stdio, and a remote http or sse endpoint. + +## Adding one from the prompt + +``` +/mcp list what is configured, with the tool count each contributed +/mcp add wizard: local or remote, then the fields that kind needs +/mcp remove +``` + +`/mcp add` asks for the kind first, because the two need different fields — a command and +its arguments against a URL and its headers — and a single form with half of it inapplicable +is worse than two short ones. + +``` +Add an MCP server +none configured yet +> local a command on this machine, over stdio + remote an http or sse endpoint +``` + +The name is validated as it is typed. Tools register as `mcp____`, so a name +with a space or a double underscore produces a tool the model cannot address and two servers +whose namespaces can collide — both are refused in place rather than at connect time. A name +already in the config is refused too. + +For a local server the wizard then asks for the command and its arguments; arguments split on +spaces and keep quoted runs together, so `--root "/home/my folder"` arrives as one argument. +For a remote one it asks for the URL — http or https only — and optional headers as +`KEY: value, OTHER: value`. + +Both write straight to `config.json` and merge with whatever is already there. **A new server +connects on the next start**, not mid-session: connecting during a turn would change the tool +list under a request that is already running. + +`/mcp` shows the state of each configured server, which is what makes a typo visible: + +``` +mcp servers +/mcp add to add one + +- `filesystem` (local) - 11 tools + npx -y @modelcontextprotocol/server-filesystem . +- `api` (remote) - failed: fetch failed + https://example.com/mcp + +configured in /home/you/.shiro-neko/config.json +``` + +## The config file + +The wizard writes this; it is equally editable by hand. + +```json +{ + "mcpServers": { + "filesystem": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] + }, + "api": { + "url": "https://example.com/mcp", + "headers": { "Authorization": "Bearer sk-..." } + } + } +} +``` + +| Field | Kind | Meaning | +|---|---|---| +| `command` | local | the executable to spawn | +| `args` | local | its arguments | +| `env` | local | extra environment variables | +| `cwd` | local | working directory | +| `url` | remote | the MCP endpoint | +| `type` | remote | `http` (default) or `sse` | +| `headers` | remote | sent with every request, for auth | + +`--no-mcp` skips every server for one run, which is the first thing to try when the agent is +behaving oddly and a server is in play. + [Model Context Protocol](https://modelcontextprotocol.io) servers contribute tools. Configure them in `~/.shiro-neko/config.json` and they appear alongside the builtins. ## Configuration +Everything the wizard writes is equally editable by hand, and a hand-written entry that +`/mcp add` would have rejected still connects — the validation is on the input path, not a +schema check at load. + ```json { "mcpServers": { diff --git a/docs/memory.md b/docs/memory.md index 24565f7..554bd8d 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -125,6 +125,21 @@ shiro -c # newest session for this directory shiro -r 0193ab2c # by id or unique prefix ``` +Both are printed as shiro exits, so the id is on screen rather than in a directory you +have to go looking through: + +``` +Good bye. +Saved 8 messages: "why does the pagination test fail?" + +Resume it with: + shiro -c newest session in this directory + shiro -r 0193ab2c this session by id +``` + +A session with no messages was never written, so it says so instead of naming a command +that would find nothing. + ``` /sessions list the last 15 /resume @@ -208,6 +223,26 @@ answering it: alone deliberately: a tool call still waiting for its result is what a suspended approval looks like, and dropping it would break `/resume`. +**An item the provider no longer holds.** An `item_reference` only resolves while the item is +still in provider storage. A session resumed the next day, or one that fell back from +`/v1/chat/completions` to `/v1/responses` mid-turn, can carry references to items that are gone: + +``` +404 Item with id 'msg_…' not found. +``` + +Retrying that history fails identically every time, so there is nothing to wait for. Two things +answer it. Compaction now strips every provider `itemId` from the history it sends, so a pruned +turn is always inline; and a 404 naming a missing item rewrites the session's own history inline +and runs the request again — once per turn, reported as: + +``` +the provider no longer had part of this session stored. Re-sent the history inline and carried on. +``` + +Only a rejection *before* any output is repaired. Once text is on screen it cannot be unsent, and +a retry would say it all a second time. + ### What compaction still does not do It tells the model the history was pruned but not what was in it. A decision from forty messages diff --git a/docs/permissions.md b/docs/permissions.md index 36dd3be..dceb5c5 100644 --- a/docs/permissions.md +++ b/docs/permissions.md @@ -33,7 +33,8 @@ remain are the ones worth reading. | Tool | Matched against | |---|---| | `bash` | the command, e.g. `git status --porcelain` | -| `read_file` `write_file` `edit_file` `multi_edit` `list_dir` | the path | +| `read_file` `write_file` `edit_file` `multi_edit` `delete_file` `list_dir` | the path | +| `move_file` | both ends; one match is enough | | `apply_patch` | every file marker path in the patch | | `web_fetch` | the URL | | `read_many_files` | every path in the batch; one match is enough | @@ -98,7 +99,7 @@ With no `permission` config: | `glob` `grep` `list_dir` | `allow` | | the git tools | `allow` — they cannot mutate anything | | `task`, and every session tool | `allow` — they touch the agent's own state | -| `write_file` `edit_file` `multi_edit` `apply_patch` `bash` `web_fetch` | `ask` | +| `write_file` `edit_file` `multi_edit` `apply_patch` `move_file` `delete_file` `bash` `web_fetch` | `ask` | | anything else, including every `mcp__*` tool | `ask` | Credentials are denied on read rather than gated, because there is no recovery. A model that diff --git a/docs/plugins.md b/docs/plugins.md index 8239ff7..d49d263 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -19,7 +19,7 @@ the agent can read. That is a sandbox problem, not a loader problem — see ## Enabling ```json -{ "plugins": ["guard", "time"] } +{ "plugins": ["guard", "secrets", "protect", "time"] } ``` That is also the default when the field is absent, and it lists **builtin** plugins only. @@ -95,6 +95,34 @@ a `write_file` overwriting something important is an approval question, not a gu The guard is the last line before a command runs; `ctrl-c` is the one after. A pattern the guard does not know about is still interruptible by hand — see [tools](tools.md#bash). +### `protect` (default on) + +Refuses writes to files whose contents belong to a tool rather than to anyone editing them by +hand. This is a different failure from a secret: the repository looks fine and behaves wrongly, +and the breakage surfaces somewhere else entirely. + +| Refused | Why | +|---|---| +| `.git/**` | git's own object store | +| `bun.lock`, `package-lock.json`, `pnpm-lock.yaml`, `yarn.lock`, `Cargo.lock`, `go.sum`, `poetry.lock`, `uv.lock`, `composer.lock`, `Gemfile.lock` | the package manager owns it | +| `node_modules/**` | an installed dependency | +| `vendor/**`, `target/debug/**`, `target/release/**` | vendored or build directory | +| `dist/**`, `build/**`, `out/**`, `.next/**`, `.nuxt/**`, `.svelte-kit/**`, `coverage/**` | generated output | +| `.venv/**`, `.tox/**`, `.mypy_cache/**`, `.ruff_cache/**`, `.turbo/**` | tool caches | + +``` +refusing to write bun.lock (a lockfile the package manager owns). Regenerate it with the +tool that owns it rather than editing it. +``` + +The message says what to do instead, which matters: a model told only "no" writes the same +content somewhere else. A lockfile is regenerated by `bun install`; build output is regenerated +by the build. + +Both separators match, so `node_modules\react\index.js` is refused on Windows too. Lookalike +names are not: `src/gitignore-parser.ts`, `docs/dist-layout.md`, and `distributed/queue.ts` all +write normally. + ### `time` (default on) Adds `current_time`, returning ISO 8601 plus the local string. Auto-approved; it reads @@ -106,7 +134,7 @@ Writes `\u0007` to stderr when a turn ends. Off by default — a bell after ever intrusive, but it is genuinely useful when a turn takes minutes. ```json -{ "plugins": ["guard", "time", "bell"] } +{ "plugins": ["guard", "secrets", "protect", "time", "bell"] } ``` ## Writing one @@ -125,7 +153,8 @@ export const noSecretsPlugin: Plugin = { 'The no-secrets plugin refuses writes to .env and credential files. Ask the user to ' + 'add secrets themselves rather than working around it.', beforeToolCall: ({ toolName, input }) => { - if (!['write_file', 'edit_file', 'multi_edit', 'apply_patch'].includes(toolName)) return undefined; + const WRITE_TOOLS = ['write_file', 'edit_file', 'multi_edit', 'apply_patch', 'move_file', 'delete_file']; + if (!WRITE_TOOLS.includes(toolName)) return undefined; const path = String((input as { path?: unknown } | null)?.path ?? ''); if (/(^|\/)\.env|credentials|\.pem$/.test(path)) { return `refusing to write ${path}; add secrets yourself`; @@ -137,8 +166,11 @@ export const noSecretsPlugin: Plugin = { Then add it to `BUILTIN_PLUGINS` and, if it should be on by default, `DEFAULT_ENABLED`. -Note the four tool names. Every write tool has to be listed, and `multi_edit` is easy to miss -— a guard that only checks `write_file` and `edit_file` is bypassed by a batch edit. +Note the six tool names. Every write tool has to be listed, and `multi_edit`, `apply_patch`, +and `move_file` are all easy to miss — a guard that only checks `write_file` and `edit_file` is +bypassed by a batch edit, a patch, or a rename. `apply_patch` and `move_file` also carry their +paths somewhere other than `path`, so a guard reading only that field sees nothing to check. +The builtins share one `writtenPaths` helper for exactly that reason. Write the `appendix` whenever the plugin can block something. Without it the model hits a refusal it was never told about and tries to route around it. diff --git a/docs/skills.md b/docs/skills.md index 67ea508..434a0cc 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -3,8 +3,8 @@ A skill is a markdown file with instructions for one kind of task. Only its name and description sit in the system prompt; the body is loaded on demand. -That split matters. The six bundled skills are 8,900 characters of body against roughly 1,000 -characters of catalogue — paid on every request. Putting every body in the prompt +That split matters. The nine bundled skills are roughly 14,000 characters of body against about +1,500 characters of catalogue — paid on every request. Putting every body in the prompt would cost that on every turn, for instructions relevant to one turn in twenty. ## Format @@ -77,6 +77,17 @@ what was not verified. match the repository's message style, and the refusals — no amending pushed commits, no `--no-verify`, no push unless asked. +**`security`** — find the trust boundary, then work outward: injection, missing authorisation, +path traversal, secrets in the wrong place, SSRF, hand-rolled crypto. Do not report a finding +without a path from an attacker-controlled value to the sink. + +**`perf`** — measure before changing anything, find where the time actually goes, change one +thing at a time, and stop at a target stated up front. Report the baseline alongside the win. + +**`migrate`** — read the changelog first, find every call site before changing one (including +CI, Dockerfiles, and docs), apply one shape of change rather than improving as you pass, and +never hand-merge a lockfile. + They are string constants in `src/skills-builtin.ts` rather than files, because `bun build --compile` only embeds modules reachable through imports. A directory of `.md` files would be missing from the shipped binary. diff --git a/docs/tools.md b/docs/tools.md index 1791e8d..5ba53e3 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -15,8 +15,8 @@ auto-approved. reaches the context is on the wire and in the session file, and there is no taking it back. `*.env.example` is allowed. -**Asked by default.** `write_file`, `edit_file`, `multi_edit`, `apply_patch`, `bash`, `web_fetch`, -and every `mcp__*` tool. +**Asked by default.** `write_file`, `edit_file`, `multi_edit`, `apply_patch`, `move_file`, +`delete_file`, `bash`, `web_fetch`, and every `mcp__*` tool. ``` bash wants to run @@ -46,7 +46,7 @@ Three more things sit around the rules: ## Tool sets Each tool costs its name, its description, and its JSON schema on **every request**. The current -registry has sixteen built-ins. `/tools` shows the live set; disabling an optional set removes +registry has nineteen built-ins. `/tools` shows the live set; disabling an optional set removes its schemas from both the request and the system prompt. | Tool | Bytes | Tool | Bytes | @@ -67,8 +67,8 @@ Sets let you switch off what a project does not need: | Set | Tools | Cost | |---|---|---| | `core` | `read_file` `write_file` `edit_file` `glob` `grep` `bash` | ~2,993 B | -| `edit-plus` | `multi_edit` `list_dir` `read_many_files` `apply_patch` | patch included | -| `git` | `git_status` `git_diff` `git_log` `git_show` `git_blame` | ~2,180 B | +| `edit-plus` | `multi_edit` `list_dir` `read_many_files` `apply_patch` `move_file` `delete_file` | patch and file ops | +| `git` | `git_status` `git_diff` `git_log` `git_show` `git_blame` `git_branch` `git_commit_message` | ~2,180 B + message | | `net` | `web_fetch` | opt in | ```json @@ -151,6 +151,16 @@ content full contents New files and full rewrites only. Creates parent directories. +A rewrite that collapses whitespace is flagged in the result: similar character count, +a fraction of the lines. A model writing a large file under output pressure squeezes +newlines and indentation before it cuts markup — the bytes survive, the layout does not — +so the result names the collapse and the turn fixes it in place: + +``` +Wrote 139 chars to index.blade.php, but it collapsed 9 lines into 1. If that was not +intended, re-send the content with its original newlines and indentation. +``` + ### `edit_file` ``` @@ -203,6 +213,31 @@ unchanged. Use it when one change spans files that must land together; use `mult several edits to one file and `edit_file` for one edit. Paths stay inside the workspace and the call asks for approval. +### `move_file` + +``` +from existing file path +to new path, including the filename +``` + +Renames or relocates one file, creating the target directory. Refuses a missing source and an +occupied target, so a rename cannot silently overwrite work. Permission rules match **both** +ends, so denying `src/generated/*` catches a move that lands there as well as one that starts +there. + +For a rename plus its callers in one atomic step, `apply_patch` is the better tool: it lands +the move and the edits together or not at all. + +### `delete_file` + +``` +path file to delete +``` + +Deletes one file and reports its size. A directory is refused: removing a tree is exactly what +the guard plugin blocks in `bash`, and it is not something to do implicitly through a tool +whose name says "file". Delete the files you mean, one call each. + ### `list_dir` ``` @@ -334,10 +369,32 @@ git_diff staged? path? unified diff of uncommitted changes git_log limit? path? hash, date, author, subject; newest first git_show ref path? one commit: message, author, diff git_blame path startLine? endLine? who last changed each line +git_branch remote? branches, newest commit first, current marked ``` +### `git_commit_message` + +Generates one commit message from the staged changes. The nested model call sees two +things: the staged diff, and the fifteen most recent commit subjects, because a message +that ignores the repository's established style reads as foreign however accurate it is. +An oversized diff is truncated before it reaches the model. + +It never commits — it returns the message only, approval-free, because generating text +cannot mutate anything. Running the commit stays on the gated `bash` path, where the +user sees the message and the command together. + +``` +$ git_commit_message +bump the server port to 9090 +``` + +Nothing staged is a stated error rather than an empty message, so the model's next move +is to stage, not to guess. + `git_log` defaults to 15 commits and caps at 40. `git_blame` without a range blames the whole -file; with `startLine` and no `endLine` it covers 40 lines from there. +file; with `startLine` and no `endLine` it covers 40 lines from there. `git_branch` sorts by +last commit and marks the current branch with `*`, which is what makes an already-taken branch +name obvious before proposing one. Everything here is also reachable through `bash`. The reason the set exists anyway is the approval boundary: `bash git diff` stops for a decision on every call, while `git_diff` cannot