diff --git a/README.md b/README.md index 3267cf8..4196079 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ agent: default thinking: medium cwd: /home/you/project skills: debug, refactor, review, test plugins: guard, time -approvals: on for write_file, edit_file, bash, mcp__* +approvals: on for write_file, edit_file, multi_edit, bash, mcp__* /help for commands > why does the pagination test fail? @@ -60,13 +60,26 @@ approvals: on for write_file, edit_file, bash, mcp__* ## What it does **Answers about your code, grounded in your code.** `grep` goes through ripgrep when it is -installed and honours `.gitignore`. `read_file` refuses binaries rather than filling the -context with mojibake. +installed and honours `.gitignore`. `list_dir` gives an ignore-aware tree so it stops globbing +blindly to orient, and `read_many_files` pulls a batch in one round trip. `read_file` refuses +binaries rather than filling the context with mojibake. -**Edits with your approval.** Every `write_file`, `edit_file`, and `bash` call stops for a -`y`/`a`/`n` decision, with a coloured diff for edits. The `guard` plugin refuses -irreversible commands outright — `rm -rf`, `git reset --hard`, force pushes, `DROP TABLE` — -and `--yolo` cannot bypass it. +**Edits with your approval.** Every `write_file`, `edit_file`, `multi_edit`, and `bash` call +stops for a `y`/`a`/`n` decision, with a coloured diff for edits. `multi_edit` is atomic, so +a failing match leaves the file untouched rather than half-changed. The `guard` plugin +refuses irreversible commands outright — `rm -rf`, `git reset --hard`, force pushes, +`DROP TABLE` — and `--yolo` cannot bypass it. + +**Shows its work.** Reasoning streams to a collapsed panel you can expand with `ctrl-r`, the +tool in flight is named as it runs, and `bash` output streams live instead of arriving all at +once when the command exits. `ctrl-c` kills a runaway command without ending the turn. + +**Takes prompts while it works.** Type during a turn and it queues; the queue drains in order +when the turn ends. `esc` interrupts and clears it. `@` completes workspace paths. + +**Reads git without touching it.** `git_status`, `git_diff`, `git_log`, `git_show`, and +`git_blame` are approval-free, because they spawn git with a fixed argument list and cannot +mutate anything. **Asks instead of guessing.** When a request has two readings that lead to different work, the agent puts a question on screen with options. @@ -83,12 +96,16 @@ so they survive both automatic pruning and `/compact`. **Runs headless.** `shiro -p "review this diff" --json` for scripts and CI. +**Keeps the tool list affordable.** Fourteen 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. + ## Documentation | Guide | Contents | |---|---| | [Configuration](docs/configuration.md) | config file, environment variables, every flag | -| [Tools](docs/tools.md) | every tool, the approval model, the guard | +| [Tools](docs/tools.md) | every tool, tool sets, the approval model, the guard | | [Agents and thinking](docs/agents.md) | variants, thinking levels, read-only modes | | [Skills](docs/skills.md) | the bundled skills and writing your own | | [Plugins](docs/plugins.md) | the plugin interface and the builtins | @@ -110,16 +127,20 @@ Type `/` and a menu appears, narrowing as you type. /tools /compact /cost /sessions /resume /save /clear /exit ``` -`esc` dismisses a panel or interrupts a running turn. Up and down recall earlier prompts. +`esc` dismisses a panel, interrupts a running turn, and clears the queue. `ctrl-c` kills the +running command but keeps the turn. `ctrl-r` expands the reasoning panel. `@` completes a +workspace path. Up and down recall earlier prompts. ## Status Working: the agent loop, tool approvals, subagents, skills, plugins, per-project memory, -session persistence, MCP, markdown rendering, headless mode, five-platform builds. +session persistence, MCP, markdown rendering, headless mode, five-platform builds, streaming +reasoning display, the mid-turn prompt queue, gateable tool sets, read-only git tools, batch +reads, `@file` completion, and interruptible commands. Next up is in [TODO.md](TODO.md); the longer view and what has been declined are in -[ROADMAP.md](ROADMAP.md). The short version of what is missing: streaming reasoning display, -a message queue for prompts typed mid-turn, `@file` completion, and git-aware tools. +[ROADMAP.md](ROADMAP.md). The short version of what is missing: a summary of what compaction +discarded, `web_fetch`, and a cheaper model for subagent searches. ## License diff --git a/ROADMAP.md b/ROADMAP.md index 91608e2..9dcda6d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -49,48 +49,78 @@ reported rather than fatal. **Distribution** — five-platform cross-compiled binaries, checksums, install scripts, CI on three operating systems, tag-driven releases. +### 0.1.0-beta.2 (unreleased) + +Fourteen built-in tools, up from six, with sets so the schema cost stays controllable. + +**Visible process** — reasoning streams to a collapsed panel with an estimated token count, +`ctrl-r` expands it, and it leaves with the turn since it is progress rather than the answer. +The tool in flight is named from `tool-input-start`, before its arguments have finished +streaming, and cleared on its result. + +**Message queue** — the input stays mounted while the model works. A prompt typed mid-turn +queues, the panel counts what is waiting, and the queue drains in order when the turn ends. +`esc` clears the queue as well as aborting. Queued slash commands replay as if typed. + +**More tools** — `multi_edit` applies several edits to one file atomically, validating every +edit in memory first so a late failure cannot leave the file half-written. `list_dir` gives an +ignore-aware depth-limited tree. Five read-only git tools, spawned with a fixed argv rather +than a shell string, which is what makes them safe to auto-approve. + +**`activeTools` gating** — `toolSets` in config: `core` always on, `edit-plus` and `git` +optional. A disabled set reaches neither the wire nor the system prompt. `/tools` names the +set each live tool came from. + +**Pruning correctness** — a tool result whose tool call the pruner discarded is now dropped +with it. Message-counted pruning cut between an assistant tool-call and the tool message +answering it, and the OpenAI responses API rejects the result on its own with 400 "No tool +call found for function call output with call_id ...". + +**Batch reads** — `read_many_files` takes up to twenty paths, each with its own window, and +runs them concurrently. An unreadable path is reported in its own block rather than throwing, +so one wrong guess costs a line instead of the call. + +**`@file` completion** — `@` opens a picker fed by the ignore-aware walker, narrowing as you +type. Prefix matches rank above substring matches, so `@src/` means "under src/" rather than +"anything containing src/". Tab inserts a plain relative path. The walk happens on the first +`@` rather than at startup. + +**Interruptible commands** — `ctrl-c` kills the command in flight and keeps the turn: the call +fails with a message saying the command did not finish and its effects are unknown, and the +model takes its next step from there. The kill takes the whole process tree, because killing +`cmd /c` alone leaves the real command holding both pipes open and the read never returns. + --- ## Next -### Visible process +### Lossless-enough compaction -The agent's reasoning is discarded. `reasoning-delta` already arrives from the session; the -transcript drops it. A collapsed panel showing what the model is thinking, expandable with a -key, is the largest gap between this and a tool that feels responsive on a slow turn. +Compaction says the history was pruned but not what was in it, so the model can contradict +its own earlier decision with confidence. A summary of the discarded span costs one cheap +call and removes the whole class of problem. -Also missing: which file is being read or written as it happens. `tool-call` events carry -the path but the transcript only shows a one-line summary after the fact. +### Cost control -### Message queue +Two halves of the same problem: an `explore` subagent pays the parent's reasoning rate for +what is really a search, and nothing stops a headless run that loops. A cheaper subagent model +and a per-session ceiling are both small changes on top of the pricing that already exists. -Typing during a turn does nothing. It should queue and run when the turn ends. Interrupting -with `esc` then retyping loses the thought. Requires input to stay live while `busy`, which -means the prompt and the spinner have to coexist rather than swap. +### Derived tool metadata -### More tools +`TOOL_SETS` and `MUTATING_TOOLS` are hand-maintained lists of tool names. A tool added to one +and forgotten in the other is a silently ungated write. Marking each tool where it is defined, +and checking the coverage in the suite, removes the failure mode rather than documenting it. -Measured cost: 553 characters of schema per tool, sent every request. Thirteen live tools is -already at the point where selection accuracy starts to matter, so the next additions need -`activeTools` gating per set before the count grows. +### `web_fetch` -Ordered by value per line of code: - -- `multi_edit` — several edits to one file atomically, killing read-edit-read-edit churn -- `list_dir` — a tree view, so the model stops globbing blindly to orient -- `read_many_files` — batch reads in one round trip -- git read-only set — `git_status`, `git_diff`, `git_log`, `git_show`, `git_blame`, all - approval-free because they cannot mutate -- `web_fetch` — URL to markdown +URL to markdown, in a `net` set that is off by default — it is the one tool that leaves the +machine. Declined: wrappers around a single bash line with no added guarantee. `run_tests`, `typecheck`, `lint`, `build` are five tools of pure schema tax when the real commands are already in `AGENTS.md`. -### `@file` completion - -Typing `@src/` should complete paths. The last real ergonomic gap in the input. - --- ## Later @@ -101,9 +131,6 @@ handles multiple agents; the loop does not fan out. **Session branching.** Fork a session at a message to try a different approach without losing the original. -**Cost budgets.** A per-session ceiling that warns, then stops. Pricing and accounting exist; -the limit does not. - **Structured diff review.** Approve or reject individual hunks of an `edit_file` call rather than the whole thing. diff --git a/TODO.md b/TODO.md index 6cc1f55..4f7f20d 100644 --- a/TODO.md +++ b/TODO.md @@ -8,74 +8,71 @@ Longer-term direction lives in [ROADMAP.md](ROADMAP.md). ## Now -### Show reasoning in the transcript +### Summarize the pruned span -`src/session.ts` already yields `{ type: 'reasoning' }`; `src/ui/App.tsx` ignores it. +Compaction tells the model the history was pruned but not what was in it, so it can +confidently contradict a decision it made forty messages ago. -- [ ] Accumulate reasoning deltas into their own buffer, separate from `text` -- [ ] Render as a dim collapsed panel: `thinking... 412 tokens`, expandable with a key -- [ ] Drop it from the transcript when the turn ends — reasoning is not part of the answer -- [ ] Test: a model emitting `reasoning-delta` puts text on screen before any `text-delta` +- [ ] Summarize the discarded messages before dropping them +- [ ] Inject the summary in place of the count +- [ ] Budget it: a summary that grows with the session defeats the point +- [ ] Test: a pruned decision is still recoverable from the summary -### Show the file being touched +### A cheaper model for subagents -`tool-call` carries the path but the transcript only shows a summary after the call returns. +The subagent shares the parent's model. An `explore` run is search, not reasoning, and it +currently pays the parent's per-token rate. -- [ ] Render an active-tool line while a call is in flight: `read src/session.ts` -- [ ] Clear it on `tool-result` or `tool-error` -- [ ] Test: a slow tool leaves its line on screen for the duration +- [ ] `subagentModel` in config, defaulting to the parent +- [ ] `/cost` separates parent from subagent spend +- [ ] Test: the subagent's calls go to the configured model, the parent's do not -### Queue prompts typed during a turn +### A spend ceiling -- [ ] Keep `PromptInput` mounted while `busy`, alongside the spinner -- [ ] Submitting while busy appends to a queue and shows `queued: 2` -- [ ] Drain the queue in order when the turn ends -- [ ] `esc` clears the queue as well as aborting -- [ ] Test: two prompts typed during a turn run in order afterwards +A headless run that loops costs real money with nothing to stop it. + +- [ ] `maxSpendUsd` in config, checked after every turn +- [ ] Warn at 80%, refuse to start another turn at 100% +- [ ] Headless exits non-zero with the ceiling named, rather than stopping silently +- [ ] Test: a session past its ceiling refuses the next turn and says why --- ## Next -### `activeTools` gating per tool set +### Summarize the pruned span -Needed before the tool count grows. Measured at 553 chars of schema per tool per request. +Compaction tells the model the history was pruned but not what was in it, so it can +confidently contradict a decision it made forty messages ago. -- [ ] `toolSets` in config: which sets are live -- [ ] Sets: `core`, `git`, `edit-plus`, `net` -- [ ] `prepareStep` narrows `activeTools` to the enabled sets -- [ ] `/tools` shows which set each tool came from -- [ ] Test: a disabled set's tools reach neither the wire nor the prompt +- [ ] Summarize the discarded messages before dropping them +- [ ] Inject the summary in place of the count +- [ ] Budget it: a summary that grows with the session defeats the point +- [ ] Test: a pruned decision is still recoverable from the summary -### `multi_edit` +### `web_fetch` -- [ ] Several `{ oldString, newString }` edits against one file -- [ ] Atomic: any failing match aborts the whole call, file untouched -- [ ] Each edit applied to the result of the previous one -- [ ] Approval prompt shows one combined diff -- [ ] Test: a failing second edit leaves the file exactly as it was +- [ ] URL to markdown, size-capped +- [ ] Belongs to a `net` set, off by default — it is the one tool that leaves the machine +- [ ] Test: a redirect is followed, an oversized body is truncated with a note -### `list_dir` +### Derive the tool-name lists -- [ ] Tree view honouring `.gitignore`, depth-limited, entry-capped -- [ ] Marks directories and shows file sizes -- [ ] Test: respects ignore rules, stops at the depth limit +`TOOL_SETS` and `MUTATING_TOOLS` both list names by hand. A tool added to one and forgotten +in the other is a silently ungated write, which is the worst kind of bug this codebase can +have. -### Git read-only tools +- [ ] 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 -All approval-free, since none can mutate. +### Subagent parallelism -- [ ] `git_status`, `git_diff`, `git_log`, `git_show`, `git_blame` -- [ ] Structured output, not raw porcelain -- [ ] Fail clearly outside a repo instead of returning git's error text -- [ ] Test: each returns something usable in a temp repo, and a clean error outside one +Two independent searches run sequentially. The panel already renders several agents; the loop +does not fan out. -### `@file` completion - -- [ ] `@` in the input opens a path picker fed by the ignore-aware walker -- [ ] Tab completes, continued typing narrows -- [ ] Completed path inserted as a plain relative path -- [ ] Test: `@src/` narrows to files under `src/` +- [ ] `task` accepts several investigations and runs them together +- [ ] Test: two delegated searches overlap in time rather than queueing --- @@ -85,9 +82,8 @@ All approval-free, since none can mutate. - [ ] `estimateTokens` divides JSON length by four. Good enough for a compaction threshold, wrong enough to mislead in `/cost`. Either label it an estimate everywhere or use a real tokenizer -- [ ] The subagent shares the parent's model. A cheaper model for search would cut cost - substantially on `explore` runs -- [ ] No spend ceiling. A headless run that loops costs real money with nothing to stop it +- [ ] `listPaths` walks up to 5000 files once per session. Fine for a repo, wasteful in a + monorepo, and it never notices a file created after the first `@` --- @@ -98,12 +94,36 @@ Not bugs exactly, but things that will bite someone. - **`/clear` wipes the terminal scrollback.** `` output is already committed, so clearing React state alone leaves it on screen. The escape sequence works but takes the user's earlier terminal history with it. -- **Compaction is lossy in a way the model cannot see.** It is told the history was pruned, - but not what was in the pruned part. A summary of the discarded span would be better than - a count. - **Memory has no conflict resolution.** Two contradictory notes both persist and both get injected. `/memory` may merge them, or may keep both. -- **`bash` cannot be interrupted independently.** `esc` aborts the whole turn, killing the - command. There is no way to stop a runaway command and keep the turn. - **Windows `cmd /c` differs from `bash -lc`.** A command the model writes for one shell may fail on the other. The prompt states the platform; it does not translate. +- **An unknown name in `toolSets` is dropped silently.** The header line shows which sets + actually loaded, but a typo reads as "that set is off" rather than as a mistake. +- **The reasoning panel is per-turn, not per-step.** Reasoning from an early step stays on + screen through later ones until the turn ends. +- **An interrupted command's effects are unknown, and the model is told so.** Nothing can know + how far a half-run migration got. +- **`@` completion lists files, not directories.** `@src/` narrows correctly, but you cannot + complete to `src/` itself, because the walker only yields files. + +--- + +## Done + +Kept for one release, then deleted. + +- [x] Reasoning streamed to a collapsed panel, `ctrl-r` to expand, dropped when the turn ends +- [x] The tool in flight named on screen from `tool-input-start` until its result arrives +- [x] Prompts typed during a turn queue and drain in order; `esc` clears the queue +- [x] `toolSets` gating, so a disabled set reaches neither the wire nor the prompt +- [x] `multi_edit`, atomic across several edits to one file +- [x] `list_dir`, ignore-aware and depth-limited +- [x] Read-only git tools: `git_status` `git_diff` `git_log` `git_show` `git_blame` +- [x] Orphaned tool results dropped during pruning, fixing the 400 "No tool call found for + function call output with call_id ..." +- [x] `read_many_files`, concurrent, one labelled block per file, a bad path reported in place +- [x] `@file` completion: picker fed by the ignore-aware walker, tab inserts a relative path +- [x] `ctrl-c` kills the running command and keeps the turn. The kill takes the whole process + tree: killing `cmd /c` alone left the real command holding both pipes open, so the + interrupt appeared to do nothing for 19 seconds diff --git a/docs/agents.md b/docs/agents.md index dbd9629..f6b3a84 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -32,9 +32,23 @@ pure latency. **`deep`** asks for more than one hypothesis before acting, more reading before concluding, and findings recorded with `remember` so they survive compaction. -**`plan`** and **`review`** are genuinely read-only. `write_file`, `edit_file`, and `bash` -are withheld from the model, not merely discouraged in prose — a model that cannot see a -tool cannot call it. Their prompts also forbid describing edits as if they had been made. +**`plan`** and **`review`** are genuinely read-only. `write_file`, `edit_file`, `multi_edit`, +and `bash` are withheld from the model, not merely discouraged in prose — a model that cannot +see a tool cannot call it. They keep everything that only reads, including `read_many_files`, +`list_dir`, and the git tools. Their prompts also forbid describing edits as if they had been +made. + +## Variants and tool sets + +Two separate things narrow the tool list, and they compose. + +A variant withholds tools by *capability*: `plan` cannot write, whatever the config says. +`toolSets` withholds them by *cost*: a project that never wants the git tools switches that set +off for every variant. See [tools](tools.md#tool-sets). + +Both go through one function, so a withheld tool is missing from the wire and from the system +prompt together. `/tools` lists what is actually offered this turn, with the set each tool came +from. ## Thinking levels diff --git a/docs/architecture.md b/docs/architecture.md index d04775a..6d47b67 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -62,7 +62,9 @@ That is not an optimisation. A `todo_write` on step one must be visible to step `prepareStep` is the only place per-step state can enter. The prompt also describes only the tools actually offered this turn. A prompt that mentions a -withheld tool teaches the model to attempt impossible calls. +withheld tool teaches the model to attempt impossible calls. Two things narrow that set: a +read-only agent variant, and `toolSets` in config. Both go through `activeTools()`, so a +withheld tool is absent from the wire and from the prompt together. ## Rendering @@ -74,6 +76,10 @@ Two things fix it: - Finished lines go into ``, rendered once and never redrawn. - Token deltas accumulate in a ref and flush on a 60 ms interval, not per token. +Answer text and reasoning text are separate refs on the same interval. Reasoning is shown +collapsed as a token estimate, expandable with `ctrl-r`, and dropped when the turn ends: it is +progress, not the answer, and keeping it would bury the reply it was leading up to. + Markdown is parsed on every flush. An unclosed fence renders as a code block that grows, which is what a reader expects while text is still arriving. @@ -83,9 +89,59 @@ which is what a reader expects while text is still arriving. recall is impossible, and it only ever *shrinks* its internal cursor offset, so an externally set value leaves the cursor stranded mid-string. -`src/ui/PromptInput.tsx` owns the cursor. That also gives home, end, and ctrl-a/e/k/u/w for -free. It hands up, down, tab, and escape to a parent callback first, so the command menu and -open panels can claim them before the input treats them as editing keys. +`src/ui/PromptInput.tsx` owns the cursor and reports it with every change, which is what makes +`@path` completion possible at all. That also gives home, end, and ctrl-a/e/k/u/w for free. It +hands up, down, tab, and escape to a parent callback first, so the file picker, the command +menu, and open panels can claim them before the input treats them as editing keys. + +The file picker claims those keys ahead of the command menu. While an `@` token is open, up and +down mean "move in the list", not "recall an earlier prompt". + +`src/complete.ts` holds the token extraction, ranking, and insertion as pure functions, so the +rules are testable without a terminal. Two of them are decisions rather than mechanics: + +- The `@` must start a word, or `user@host` opens a file picker. +- Prefix matches rank above substring matches, because `@src/` means "under `src/`" and a + substring hit on `vendor/src/` would bury what the user pointed at. + +## The prompt queue + +The input stays mounted while the model works. A prompt submitted mid-turn is pushed onto a +queue and drained in order when the turn ends, going back through `submit` so a queued slash +command behaves exactly as if it were typed at that moment. + +The queue is a ref as well as state. The drain runs synchronously as the turn ends, between +renders, and a closure over a stale array would silently lose a prompt. `busy` is mirrored into +a ref for the same reason. + +`esc` clears the queue as well as aborting. Interrupting and then watching two more prompts +fire anyway is not what anyone means by interrupt. + +## Interrupting one command + +`esc` aborts the whole turn. That is the wrong tool for a runaway command, because it throws +away the conversation to stop a `sleep`. + +`ctrl-c` kills the command in flight and leaves the turn alive. `src/tools.ts` keeps the +running processes by tool call id, and `interruptBash()` kills them and returns what it killed. +The call then **throws** rather than returning: + +``` +The user interrupted this command. It did not finish, so its effects are unknown. +``` + +Throwing is the point. A returned `exit: 1` reads to the model as a command that ran and +failed on its own terms, which is a different fact from a command that was stopped partway. +The model gets a tool error, and the loop continues to the next step. + +The kill has to take the whole process tree. `cmd /c` and `bash -lc` run the real command as a +child, and killing the shell alone leaves that child holding both pipes open, so the read never +returns — measured at 19 seconds for a `ping -n 20` that should have died instantly. On Windows +that means `taskkill /T /F`. The kill is also awaited before the tool returns, because a +surviving grandchild keeps the working directory locked. + +Ink's own `exitOnCtrlC` is turned off in `cli.tsx` so the key reaches the app; with nothing +running, the handler exits as usual. ## Subagents @@ -120,22 +176,38 @@ Only `api.openai.com` gets the chain. Third-party endpoints do not implement `/v ## Compaction and its repair -`pruneMessages({ reasoning: 'all' })` strips a reasoning item and keeps the message item from -the same response. The responses API treats the message as that reasoning item's dependent -and returns 400. +Pruning breaks two different provider invariants, and `src/prune.ts` repairs both. + +**A message without its reasoning item.** `pruneMessages({ reasoning: 'all' })` strips a +reasoning item and keeps the message item from the same response. The responses API treats the +message as that reasoning item's dependent and returns 400. The two carry different ids, so they cannot be matched by id. What links them is the assistant message they arrived in: one message is one response, and its reasoning item covers every -other item in it. `src/prune.ts` drops the dependent parts of any turn whose reasoning was +other item in it. `dropOrphanedItems` drops the dependent parts of any turn whose reasoning was removed — which costs nothing, since pruning was already discarding those turns. +**A tool result without its tool call.** `toolCalls: 'before-last-3-messages'` counts +*messages*, so the cut lands between an assistant `tool-call` and the `tool` message answering +it. What reaches the wire is a `function_call_output` with no `function_call`: + +``` +400 No tool call found for function call output with call_id call_… +``` + +`dropOrphanedResults` collects the surviving call ids and drops any result that has none. The +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. + ## Module map | Module | Responsibility | |---|---| | `session.ts` | the loop, approvals, compaction, event stream | -| `tools.ts` | file and shell tools, ripgrep bridge, bash streaming | +| `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 | | `ignore.ts` | gitignore-aware walker, path jail | +| `complete.ts` | `@path` token extraction, ranking, insertion | | `prompt.ts` | system prompt assembly from live state | | `agents.ts` | variants, thinking levels | | `skills.ts` | discovery, catalogue, `skill` tool | @@ -146,7 +218,7 @@ removed — which costs nothing, since pruning was already discarding those turn | `ask.ts` | the `ask` tool | | `mcp.ts` | MCP clients and namespacing | | `fallback.ts` | endpoint chain | -| `prune.ts` | provider-item repair | +| `prune.ts` | provider-item and tool-pairing repair | | `markdown.ts` | parser, no dependency | | `store.ts` | sessions, prompt history | | `config.ts` | resolution, model construction | @@ -162,9 +234,10 @@ Every module is pure of the UI except `ui/`, and `ui/` never touches the SDK. Th ## Testing -404 tests, no mocking framework. `MockLanguageModelV4` from `ai/test` drives the loop; +482 tests, 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 are tested against a local HTTP server. +server subprocess; provider wire formats are tested against a local HTTP server; the interrupt +path spawns a real subprocess and asserts it died early rather than ran out. The pattern throughout is to assert on what actually crossed a boundary — what went on the wire, what is on screen, what is on disk — rather than on internal calls. diff --git a/docs/configuration.md b/docs/configuration.md index 028f2b3..593702e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,6 +21,7 @@ Written by `/provider`, editable by hand. Every field is optional. "thinking": "medium", "maxRetries": 3, "plugins": ["guard", "time"], + "toolSets": ["edit-plus", "git"], "mcpServers": { "fs": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] } } @@ -38,6 +39,7 @@ Written by `/provider`, editable by hand. Every field is optional. | `thinking` | default level: `off`, `low`, `medium`, `high`, `max` | | `maxRetries` | retries per model call for transient failures. Default 3 | | `plugins` | which plugins to enable. Omit for `["guard", "time"]` | +| `toolSets` | optional tool sets beyond `core`: `edit-plus`, `git`. Omit for all of them. See [tools](tools.md) | | `mcpServers` | see [MCP](mcp.md) | ## Provider presets diff --git a/docs/development.md b/docs/development.md index afe3ccc..a37c4f2 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 # 404 tests +bun test # 482 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 @@ -69,21 +69,32 @@ mock-verification test: - `pruneMessages` leaving a message item without its reasoning item — visible only in the 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 - `--json` serialising `Error` as `{}` — visible only in the printed output - Automatic approval requests prompting the user — visible only in the event sequence +- `ctrl-c` killing `cmd /c` but not the command under it — visible only as elapsed time, since + the interrupt reported success while the command ran for another 19 seconds ## 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. If it mutates anything, add it to `MUTATING_TOOLS` so it requires approval. -4. Add a line to `TOOL_DOCS` in `src/prompt.ts` saying *when* to reach for it. -5. Test the behaviour in a temp directory, including the failure path. +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 + it. +7. Test the behaviour in a temp directory, including the failure path. -Every tool costs roughly 550 characters of schema on every request. Thirteen live tools is -already where selection accuracy starts to matter, so a new tool needs to earn its place — -see [ROADMAP.md](../ROADMAP.md) for what has been declined and why. +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). + +Every tool costs roughly 550 characters of schema on every request. Fourteen built-in tools is +well 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. ## Adding a slash command @@ -149,10 +160,13 @@ handling differs — a Windows-only break is invisible on Linux until someone hi ## Debugging the agent itself `--no-plugins --no-skills --no-memory --no-instructions --no-subagent --no-mcp` strips it to -the seven core tools, which isolates whether a problem is the loop or something layered on it. +the built-in tools alone, which isolates whether a problem is the loop or something layered on +it. `{ "toolSets": [] }` narrows it further, to the six core tools. `--json` in headless mode shows the exact event sequence. For provider issues, a local `Bun.serve` that logs the request body and returns a canned SSE stream answers "what did we actually send" faster than any amount of reading. Several bugs in -this codebase were found that way. +this codebase were found that way. Making that stub *reject* the thing you think you fixed is +better still: the tool-pairing repair was confirmed by a stub that returned the real 400 for an +orphaned result, then stopped doing so. diff --git a/docs/headless.md b/docs/headless.md index 741093b..27fcafa 100644 --- a/docs/headless.md +++ b/docs/headless.md @@ -16,13 +16,14 @@ There is no terminal to approve on, so every gated tool is denied unless `--yolo ``` $ shiro -p "add a test for paginate()" -shiro: headless denies write_file, edit_file, bash and mcp tools unless --yolo is passed +shiro: headless denies write_file, edit_file, multi_edit, bash and mcp tools unless --yolo is passed [tool] write_file {"path":"test/paginate.test.ts",...} [denied] write_file (run with --yolo to allow tool use in headless mode) ``` Read-only tools work either way, so `-p` without `--yolo` is a safe way to ask questions -about a codebase from a script. +about a codebase from a script. That includes `read_many_files`, `list_dir`, and the git tools, +which is enough to review a diff or explain a module without any write access at all. **`--yolo` does not disable plugin guards.** `rm -rf` is still refused. @@ -47,14 +48,18 @@ src/prune.ts repairs provider-item dependencies after pruneMessages strips reaso ```bash $ shiro -p "count the tools" --json +{"type":"tool-start","id":"c1","name":"grep"} {"type":"tool-call","id":"c1","name":"grep","input":{"pattern":"tool\\("}} {"type":"tool-result","id":"c1","name":"grep","output":"src/tools.ts:26: ..."} -{"type":"text","text":"There are 6 built-in file and shell tools."} +{"type":"text","text":"There are 14 built-in tools."} {"type":"done","inputTokens":4210,"outputTokens":88} ``` -Event types: `text`, `reasoning`, `tool-call`, `tool-output`, `tool-result`, `tool-error`, -`tool-denied`, `compacted`, `notice`, `error`, `done`. +Event types: `text`, `reasoning`, `tool-start`, `tool-call`, `tool-output`, `tool-result`, +`tool-error`, `tool-denied`, `compacted`, `notice`, `error`, `done`. + +`tool-start` arrives before the arguments have finished streaming, so it carries the name but +no input. Use `tool-call` when you need the arguments. Errors are flattened to message strings, because `JSON.stringify` turns an `Error` into `{}` and a JSON stream that reports failures as empty objects is useless for the one case it @@ -85,6 +90,10 @@ is told to decide and state its assumption instead. Subagent progress events are not emitted; the report still comes back. +There is no terminal, so `ctrl-c` cannot interrupt a single command the way it does +interactively — a signal kills the run. Cap the risk with the `timeout` the model passes to +`bash`, or with `--agent quick` to cap the step count. + ## CI recipes Review a pull request diff: @@ -125,4 +134,5 @@ env: ## Cost control Headless runs are unattended, so a runaway loop costs real money. `--agent quick` caps the -step count at 12. There is no spend ceiling yet — see [ROADMAP.md](../ROADMAP.md). +step count at 12, and `{ "toolSets": [] }` trims the schema sent every request. There is no +spend ceiling yet — see [TODO.md](../TODO.md). diff --git a/docs/memory.md b/docs/memory.md index 7eede11..c04ea77 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -135,23 +135,46 @@ and outcomes, what remains — and it replaces the transcript entirely. ### The pruning repair -`pruneMessages({ reasoning: 'all' })` strips a reasoning item and keeps the message item from -the same response. The OpenAI responses API treats the message as a dependent of that -reasoning item and rejects the request: +Pruning breaks two provider invariants. `src/prune.ts` repairs both, and both were real 400s +before it did. + +**A message without its reasoning item.** `pruneMessages({ reasoning: 'all' })` strips a +reasoning item and keeps the message item from the same response. The OpenAI responses API +treats the message as a dependent of that reasoning item and rejects the request: ``` 400 Item 'msg_…' of type 'message' was provided without its required 'reasoning' item: 'rs_…' ``` The two carry different ids, so they cannot be matched by id. What links them is the -assistant message they arrived in — one message is one response. `src/prune.ts` drops the +assistant message they arrived in — one message is one response. `dropOrphanedItems` drops the dependent parts of any turn whose reasoning was removed. That costs nothing, because pruning was already discarding those turns. +**A tool result without its tool call.** `toolCalls: 'before-last-3-messages'` counts +*messages*, not pairs, so the cut can land between the assistant message holding a `tool-call` +and the `tool` message answering it: + +``` +400 No tool call found for function call output with call_id call_… +``` + +`dropOrphanedResults` drops any result whose call id no longer survives. The reverse is left +alone deliberately: a tool call still waiting for its result is what a suspended approval looks +like, and dropping it would break `/resume`. + ## Prompt history Per-directory, capped at 200, deduplicated against the previous entry. Up and down in the -input walk it; down past the newest restores what you were typing. +input walk it; down past the newest restores what you were typing. While an `@` token is open +those keys move in the file picker instead. Stored at `~/.shiro-neko/history/.json`, where the hash is a SHA-256 prefix of the project path. + +## The prompt queue + +Not persisted, and deliberately so. A prompt typed during a turn lives in memory until the turn +ends, then runs. `esc` clears it along with aborting the turn, and quitting discards it — a +queued thought that fires on next launch, against a workspace that has since changed, is worse +than a lost one. diff --git a/docs/plugins.md b/docs/plugins.md index 79c28fe..4d3b7ba 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -42,6 +42,10 @@ Two decisions worth knowing about: **Blocks are checked before approval.** `--yolo` skips prompts; it does not skip guards. A plugin block is a refusal, not a permission question. +**A guard sees `bash` before the command runs, not while it runs.** The guard is the only thing +that can refuse a command outright; once one is running, `ctrl-c` is what stops it. Both matter: +a pattern the guard does not know about is still interruptible by hand. + ## Builtins ### `guard` (default on) @@ -98,7 +102,7 @@ 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 (toolName !== 'write_file' && toolName !== 'edit_file') return undefined; + if (toolName !== 'write_file' && toolName !== 'edit_file' && toolName !== 'multi_edit') 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`; @@ -110,6 +114,9 @@ export const noSecretsPlugin: Plugin = { Then add it to `BUILTIN_PLUGINS` and, if it should be on by default, `DEFAULT_ENABLED`. +Note the three 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. + 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/tools.md b/docs/tools.md index f54252c..b0f4621 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -4,14 +4,15 @@ Three categories. -**Free.** Read-only, no prompt: `read_file`, `glob`, `grep`, `task`. +**Free.** Read-only, no prompt: `read_file`, `read_many_files`, `glob`, `grep`, `list_dir`, +`task`, and the whole git set. **Session tools.** Also free, because they touch the agent's own state rather than your files: `todo_write`, `remember`, `recall`, `forget`, `skill`, `ask`, and anything a plugin marks auto-approved. -**Gated.** Every call stops for a decision: `write_file`, `edit_file`, `bash`, and every -`mcp__*` tool. +**Gated.** Every call stops for a decision: `write_file`, `edit_file`, `multi_edit`, `bash`, +and every `mcp__*` tool. ``` edit_file wants to run @@ -29,6 +30,28 @@ to ask what to do instead. `--yolo` skips all prompts. **The guard runs before all of this.** It is not an approval — it is a refusal, and `--yolo` does not reach it. See [plugins](plugins.md). +## Tool sets + +Each tool costs roughly 550 characters of JSON schema on every request, and selection +accuracy drops as the list grows. Sets let you switch off what a project does not need: + +| Set | Tools | +|---|---| +| `core` | `read_file` `write_file` `edit_file` `glob` `grep` `bash` | +| `edit-plus` | `multi_edit` `list_dir` `read_many_files` | +| `git` | `git_status` `git_diff` `git_log` `git_show` `git_blame` | + +```json +{ "toolSets": ["edit-plus"] } +``` + +Omit `toolSets` for all of them. `core` is always on — without read, edit, and bash the +agent is not an agent. A disabled set reaches neither the wire nor the system prompt, since +a prompt that names an absent tool teaches the model to attempt calls that cannot succeed. +Session, plugin, and MCP tools are not part of this budget and are never gated here. + +`/tools` shows which set each live tool came from. + ## File tools ### `read_file` @@ -43,6 +66,27 @@ Returns contents with 1-based line numbers. Refuses binaries: a NUL byte in the means the file is not text, and a model that reads a 90 MB executable has burned its whole context on nothing. +### `read_many_files` + +``` +files [{ path, offset?, limit? }], at most 20 +``` + +One round trip for several files, each with its own window. Reads run concurrently and the +blocks come back in the order given, labelled: + +``` +===== src/app.ts ===== +1: export const port = 8080; + +===== src/gone.ts ===== +[unreadable: No such file: src/gone.ts] +``` + +A path that cannot be read is reported in its own block rather than throwing, so one wrong +guess costs a line instead of the whole call. Numbering and binary refusal are the same code +path as `read_file`, so a batch read cannot drift from a single one. + ### `write_file` ``` @@ -65,6 +109,32 @@ replaceAll replace every occurrence instead of requiring exactly one An ambiguous match is an error naming the count, which pushes the model to add surrounding context rather than guessing which occurrence it meant. +### `multi_edit` + +``` +path file path +edits [{ oldString, newString, replaceAll? }], in the order to apply them +``` + +Several edits to one file in one call, one approval, one write. Each edit sees the result of +the previous one, so edits may build on each other. + +Atomic: every edit is validated and applied in memory first, so a failure on the third edit +leaves the file exactly as it was rather than half-changed. The same uniqueness rule as +`edit_file` applies per edit, and the error names which edit failed. + +### `list_dir` + +``` +path directory, relative to the workspace root, default the root +depth levels to descend, 1-6, default 2 +includeIgnored also show files git ignores +``` + +Tree view honouring `.gitignore`. Directories end with `/`, files show their size. Past the +depth limit the containing directory is still listed, so the shape of the tree stays visible +without its contents. Capped at 300 entries. + ### `glob` ``` @@ -75,7 +145,9 @@ includeIgnored also return files git ignores Walks the tree honouring `.gitignore` and `.shiroignore`, skipping `.git` and `node_modules` unconditionally. Nested ignore files apply only within their own directory, -as git does. Returns posix paths relative to the workspace root. +as git does. Returns posix paths relative to the workspace root. A symlinked directory is +classified as a directory and not descended into, since it can point anywhere including +back into the tree. ### `grep` @@ -104,6 +176,39 @@ command that fills one while you block on the other deadlocks. Returns exit code, stdout, stderr, and a note if a signal killed it. +**`ctrl-c` interrupts the command, not the turn.** The shell and everything it started are +killed — on Windows through `taskkill /T`, because killing `cmd` alone leaves the real command +holding both pipes open and the read never ends. The call then fails rather than returning, +so the model cannot mistake a killed command for one that ran and failed on its own: + +``` +The user interrupted this command. It did not finish, so its effects are unknown. +stdout: +[whatever it printed first] +``` + +The turn continues from there. `esc` still aborts everything, and `ctrl-c` with nothing +running quits as usual. + +## Git tools + +All five are read-only and therefore approval-free. Each spawns `git` with a fixed argument +array rather than a shell string, so an argument like `--author="; rm -rf /"` can only ever +be a literal argument — which is what makes auto-approval safe. + +Output is described rather than raw porcelain: `git_status` names the branch and says +`staged modified` or `untracked` per file instead of leaving the model to decode two columns +of flags. Outside a repository they fail with ` is not a git repository.` rather than +passing git's own error text through. + +``` +git_status branch, staged, modified, untracked +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 +``` + ## Agent tools ### `task` @@ -168,5 +273,6 @@ every call rather than being assumed. ## Output caps Any single tool result is truncated at 30,000 characters with a note saying how much was -cut. `grep` stops at 200 hits, `glob` at 200 paths, `read_file` at 2000 lines by default. -Without caps one `grep` for `function` can end a session. +cut. `grep` stops at 200 hits, `glob` at 200 paths, `list_dir` at 300 entries, +`read_many_files` at 20 files, `read_file` at 2000 lines by default. Without caps one `grep` +for `function` can end a session. diff --git a/src/agents.ts b/src/agents.ts index 139f1df..abab573 100644 --- a/src/agents.ts +++ b/src/agents.ts @@ -28,9 +28,15 @@ export type AgentVariant = { const READ_ONLY = [ 'read_file', + 'read_many_files', 'glob', 'grep', 'list_dir', + 'git_status', + 'git_diff', + 'git_log', + 'git_show', + 'git_blame', 'task', 'todo_write', 'remember', diff --git a/src/cli.tsx b/src/cli.tsx index 1254a19..b29b01b 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -7,6 +7,7 @@ import { configPath, loadConfig, missingKeyMessage, resolveModel, writeConfigFil import type { FallbackEvent } from './fallback'; import { readStdin, runHeadless } from './headless'; import { INIT_PROMPT, loadInstructions } from './instructions'; +import { walk } from './ignore'; import { connectMcp } from './mcp'; import { Memory, KIND_LABEL } from './memory'; import { costOf } from './pricing'; @@ -55,6 +56,7 @@ first run: start shiro with no key and it opens provider setup, or use /provider config: ${configPath()} { "provider": "openai", "model": "gpt-5", "apiKey": "...", "agent": "default", "thinking": "medium", "plugins": ["guard", "time"], + "toolSets": ["edit-plus", "git"], "mcpServers": { "fs": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] } } } env: SHIRO_PROVIDER SHIRO_MODEL SHIRO_BASE_URL SHIRO_API_KEY @@ -213,6 +215,7 @@ const session = new Session({ skills, plugins, agent: agentVariant, + ...(cfg.toolSets ? { toolSets: cfg.toolSets } : {}), // Headless has no one to answer, so the tool is withheld rather than left to hang. ...(headless ? {} : { ask: askBridge.ask }), ...(memory ? { memory } : {}), @@ -253,7 +256,9 @@ if (printArg !== undefined) { await shutdown(1); } if (!yolo) { - process.stderr.write('shiro: headless denies write_file, edit_file, bash and mcp tools unless --yolo is passed\n'); + process.stderr.write( + 'shiro: headless denies write_file, edit_file, multi_edit, bash and mcp tools unless --yolo is passed\n', + ); } const code = await runHeadless({ session, prompt, format: has('--json') ? 'json' : 'text' }); await shutdown(code); @@ -270,6 +275,11 @@ const hooks: AppHooks = { sessionId: record.id, config: () => cfg, instructionFiles: () => instructions.map((i) => i.path), + listPaths: async () => { + const found: string[] = []; + for await (const rel of walk({ limit: 5000 })) found.push(rel); + return found; + }, initPrompt: INIT_PROMPT, history: promptHistory, recordPrompt: (text) => void store.appendHistory(text), @@ -392,12 +402,15 @@ const header = [ memory && memory.all().length > 0 ? `memory: ${memory.all().length} notes about this project` : undefined, mcp && Object.keys(mcp.tools).length > 0 ? `mcp: ${Object.keys(mcp.tools).length} tools` : undefined, ...(mcp?.errors ?? []).map((e) => `mcp ${e.server} failed: ${e.message}`), - yolo ? 'approvals: OFF (--yolo)' : 'approvals: on for write_file, edit_file, bash, mcp__*', + yolo ? 'approvals: OFF (--yolo)' : 'approvals: on for write_file, edit_file, multi_edit, bash, mcp__*', + cfg.toolSets ? `tool sets: core, ${cfg.toolSets.join(', ')}` : undefined, '/help for commands', ] .filter(Boolean) .join('\n'); +// ctrl-c has to reach the App: with a command running it kills that command and +// keeps the turn. Ink's own handler would exit the process before we saw the key. const app = render( , + { exitOnCtrlC: false }, ); await app.waitUntilExit(); await shutdown(0); diff --git a/src/commands.ts b/src/commands.ts index 9a83962..6d4e86f 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -62,9 +62,14 @@ const usage = (c: CommandSpec) => `/${c.name}${c.arg ? ` ${c.arg}` : ''}`; export const HELP = [ ...COMMANDS.map((c) => `${usage(c).padEnd(18)} ${c.summary}`), '', - 'esc interrupt the running turn', - 'tab complete the highlighted command', - 'up / down recall earlier prompts', + 'esc interrupt the running turn and clear the queue', + 'ctrl-c kill the running command, keeping the turn', + 'ctrl-r expand or collapse the reasoning panel', + 'tab complete the highlighted command or file path', + 'up / down recall earlier prompts, or move in an open menu', + '@ complete a workspace path', + '', + 'typing during a turn queues the prompt; queued prompts run in order afterwards', ].join('\n'); /** diff --git a/src/complete.ts b/src/complete.ts new file mode 100644 index 0000000..1dd9456 --- /dev/null +++ b/src/complete.ts @@ -0,0 +1,68 @@ +/** + * `@path` completion, kept pure so the ranking and the insertion can be tested + * without a terminal. + */ + +export type PathToken = { + /** Index of the `@`. */ + start: number; + /** Index just past the token, always the cursor. */ + end: number; + /** Text between the `@` and the cursor, possibly empty. */ + query: string; +}; + +/** + * The `@`-token the cursor sits in, if any. + * + * The `@` has to start a word, or `user@host` and an email address would open a + * file picker. A space ends the token, so `@src/a.ts and then` is not still + * completing after the space. + */ +export function pathToken(value: string, cursor: number): PathToken | undefined { + const before = value.slice(0, cursor); + const at = before.lastIndexOf('@'); + if (at === -1) return undefined; + + const prev = at === 0 ? undefined : before[at - 1]; + if (prev !== undefined && !/\s/.test(prev)) return undefined; + + const query = before.slice(at + 1); + if (/\s/.test(query)) return undefined; + + return { start: at, end: cursor, query }; +} + +const MAX_MATCHES = 8; + +/** + * Paths worth offering for a query. + * + * Prefix matches come first because `@src/` means "under src/", and a substring + * match on some other directory would bury the thing the user is pointing at. + * Ties break on path length: the shallower file is more often the one meant. + */ +export function matchPaths(paths: readonly string[], query: string, limit = MAX_MATCHES): string[] { + if (!query) return [...paths].sort((a, b) => a.length - b.length || a.localeCompare(b)).slice(0, limit); + + const needle = query.toLowerCase(); + const prefix: string[] = []; + const substring: string[] = []; + + for (const path of paths) { + const lower = path.toLowerCase(); + if (lower.startsWith(needle)) prefix.push(path); + else if (lower.includes(needle)) substring.push(path); + } + + const byLength = (a: string, b: string) => a.length - b.length || a.localeCompare(b); + return [...prefix.sort(byLength), ...substring.sort(byLength)].slice(0, limit); +} + +export type Completion = { value: string; cursor: number }; + +/** Replaces the token with a plain relative path and a trailing space. */ +export function completePath(value: string, token: PathToken, path: string): Completion { + const next = `${value.slice(0, token.start)}${path} ${value.slice(token.end)}`; + return { value: next, cursor: token.start + path.length + 1 }; +} diff --git a/src/config.ts b/src/config.ts index 5287fe1..84d0654 100644 --- a/src/config.ts +++ b/src/config.ts @@ -6,6 +6,7 @@ import { homedir } from 'node:os'; import { join } from 'node:path'; import { withFallback, type FallbackEvent } from './fallback'; import type { McpServerConfig } from './mcp'; +import { isToolSetName, type ToolSetName } from './tools'; export type ProviderName = 'anthropic' | 'openai'; @@ -24,6 +25,8 @@ export type Config = { thinking?: string; /** Plugin names to enable; omit for the default set. */ plugins?: string[]; + /** Optional tool sets to offer beyond `core`; omit for all of them. */ + toolSets?: ToolSetName[]; mcpServers?: Record; }; @@ -85,6 +88,7 @@ export async function loadConfig(): Promise { ...(file.agent ? { agent: file.agent } : {}), ...(file.thinking ? { thinking: file.thinking } : {}), ...(Array.isArray(file.plugins) ? { plugins: file.plugins } : {}), + ...(Array.isArray(file.toolSets) ? { toolSets: file.toolSets.filter(isToolSetName) } : {}), ...(file.mcpServers ? { mcpServers: file.mcpServers } : {}), }; } diff --git a/src/ignore.ts b/src/ignore.ts index fc06564..1840b2b 100644 --- a/src/ignore.ts +++ b/src/ignore.ts @@ -1,5 +1,5 @@ import { isAbsolute, join, relative, resolve } from 'node:path'; -import { readdir as readdirFs } from 'node:fs/promises'; +import { readdir as readdirFs, stat as statFs } from 'node:fs/promises'; const ALWAYS_SKIP = ['.git', 'node_modules']; @@ -109,10 +109,16 @@ export async function* walk(options: WalkOptions = {}): AsyncGenerator { const { dir, rules } = queue.shift()!; let entries: Entry[]; try { - entries = (await readdirFs(dir, { withFileTypes: true })).map((d) => ({ - name: d.name, - isDirectory: d.isDirectory(), - })); + entries = await Promise.all( + (await readdirFs(dir, { withFileTypes: true })).map(async (d) => ({ + name: d.name, + // readdir reports a symlinked directory as a non-directory, which made + // junctions leak past `dir/` ignore rules and be yielded as files with a + // nonsense size. One stat per link fixes the classification. + isDirectory: d.isDirectory() || (d.isSymbolicLink() && (await isDirLink(join(dir, d.name)))), + isLink: d.isSymbolicLink(), + })), + ); } catch { continue; } @@ -124,6 +130,9 @@ export async function* walk(options: WalkOptions = {}): AsyncGenerator { if (!options.noIgnore && ignored(rel, entry.isDirectory, rules)) continue; if (entry.isDirectory) { + // A symlinked directory is not descended into: it can point anywhere, + // including back into the tree. + if (entry.isLink) continue; const nested = options.noIgnore ? rules : [...rules, ...(await rulesIn(root, full))]; queue.push({ dir: full, rules: nested }); } else { @@ -134,7 +143,15 @@ export async function* walk(options: WalkOptions = {}): AsyncGenerator { } } -type Entry = { name: string; isDirectory: boolean }; +async function isDirLink(path: string): Promise { + try { + return (await statFs(path)).isDirectory(); + } catch { + return false; + } +} + +type Entry = { name: string; isDirectory: boolean; isLink: boolean }; /** Resolves a model-supplied path inside the workspace, rejecting escapes. */ export function jail(p: string, root = process.cwd()): string { diff --git a/src/prompt.ts b/src/prompt.ts index 3154684..c24fe70 100644 --- a/src/prompt.ts +++ b/src/prompt.ts @@ -1,4 +1,5 @@ import { formatInstructions, type Instructions } from './instructions'; +import { GIT_TOOL_NAMES } from './tools-git'; export type PromptParts = { cwd: string; @@ -30,6 +31,10 @@ type ToolDoc = { name: string; line: string }; */ const TOOL_DOCS: ToolDoc[] = [ { name: 'read_file', line: 'read before you edit. Never describe code you have not opened.' }, + { + name: 'read_many_files', + line: 'read several files in one round trip once you know which ones you need. An unreadable path is reported in place, not fatal.', + }, { name: 'glob', line: 'find files by pattern. Skips binaries and .gitignore; pass includeIgnored to look anyway.', @@ -42,7 +47,15 @@ const TOOL_DOCS: ToolDoc[] = [ name: 'edit_file', line: 'oldString must match byte-for-byte including indentation, and be unique. Include surrounding lines to disambiguate. Prefer several small edits over one large rewrite.', }, + { + name: 'multi_edit', + line: 'several edits to one file, all or nothing. Use it instead of repeated edit_file calls on the same file: one approval, one write, and a failed match leaves the file untouched.', + }, { name: 'write_file', line: 'new files and full rewrites only. Reach for edit_file on anything that exists.' }, + { + name: 'list_dir', + line: 'tree view of a directory, ignore-aware and depth-limited. Cheaper than guessing at glob patterns in an unfamiliar project.', + }, { name: 'bash', line: 'builds, tests, git, package managers. Output streams live. Long-running commands are fine; interactive ones are not.', @@ -72,8 +85,17 @@ function renderTools(available: readonly string[]): string { const lines = known.map((d) => `- ${d.name}: ${d.line}`); + // The git set gets one shared line instead of five: they are all read-only, all + // free, and the schema already says what each takes. + const git = extra.filter((n) => GIT_TOOL_NAMES.includes(n)); const mcp = extra.filter((n) => n.startsWith('mcp__')); - const other = extra.filter((n) => !n.startsWith('mcp__')); + const other = extra.filter((n) => !GIT_TOOL_NAMES.includes(n) && !n.startsWith('mcp__')); + + if (git.length > 0) { + lines.push( + `- ${git.join(', ')}: read-only git, no approval needed. Use them instead of bash for history and diffs; they cannot mutate the repository.`, + ); + } if (mcp.length > 0) { lines.push( `- ${mcp.join(', ')}: from MCP servers, named mcp____. Each needs approval; read its own description before calling.`, diff --git a/src/prune.ts b/src/prune.ts index e9e221f..10479a6 100644 --- a/src/prune.ts +++ b/src/prune.ts @@ -1,6 +1,10 @@ import { pruneMessages, type ModelMessage } from 'ai'; -type Part = { type: string; providerOptions?: Record> }; +type Part = { + type: string; + toolCallId?: string; + providerOptions?: Record>; +}; /** Parts the OpenAI responses API refuses to accept without their reasoning item. */ const DEPENDENT = new Set(['text', 'tool-call']); @@ -79,8 +83,54 @@ export function dropOrphanedItems(before: ModelMessage[], after: ModelMessage[]) export type PruneOptions = Parameters[0]; +const ANSWER_PARTS = new Set(['tool-result', 'tool-error']); + +const anyParts = (message: ModelMessage): Part[] => + Array.isArray(message.content) ? (message.content as Part[]) : []; + +/** + * Drops tool results whose tool call is gone. + * + * The OpenAI responses API rejects a `function_call_output` with no `function_call` + * carrying the same call id: 400 "No tool call found for function call output with + * call_id ...". Two things strand a result that way, and both happen on a long turn: + * `pruneMessages({ toolCalls: 'before-last-3-messages' })` counts messages, so the + * cut can land between an assistant tool-call and the tool message answering it, and + * `dropOrphanedItems` removes a tool-call whose reasoning item did not survive while + * the result sits in a separate message it never looks at. + * + * The reverse pairing is left alone on purpose: a call still awaiting its result is + * exactly what a suspended approval looks like, and dropping it would break resume. + */ +export function dropOrphanedResults(messages: ModelMessage[]): ModelMessage[] { + const calls = new Set(); + for (const message of messages) { + for (const part of anyParts(message)) { + if (part.type === 'tool-call' && part.toolCallId) calls.add(part.toolCallId); + } + } + + const cleaned: ModelMessage[] = []; + for (const message of messages) { + const parts = anyParts(message); + if (parts.length === 0) { + cleaned.push(message); + continue; + } + + const kept = parts.filter( + (part) => !ANSWER_PARTS.has(part.type) || part.toolCallId === undefined || calls.has(part.toolCallId), + ); + + if (kept.length === parts.length) cleaned.push(message); + else if (kept.length > 0) cleaned.push({ ...message, content: kept } as ModelMessage); + } + + return cleaned; +} + /** pruneMessages, then repair the provider-item dependencies it breaks. */ export function prunePreservingItems(options: PruneOptions): ModelMessage[] { const pruned = pruneMessages(options); - return dropOrphanedItems(options.messages, pruned); + return dropOrphanedResults(dropOrphanedItems(options.messages, pruned)); } diff --git a/src/session.ts b/src/session.ts index d913574..6225598 100644 --- a/src/session.ts +++ b/src/session.ts @@ -16,7 +16,13 @@ import type { PluginHost } from './plugins'; import { systemPrompt } from './prompt'; import { prunePreservingItems } from './prune'; import { createSkillTool, renderSkills, type Skill } from './skills'; -import { MUTATING_TOOLS, onBashOutput, tools as builtinTools } from './tools'; +import { + MUTATING_TOOLS, + disabledToolNames, + onBashOutput, + tools as builtinTools, + type ToolSetName, +} from './tools'; export type ApprovalRequest = { approvalId: string; @@ -30,6 +36,7 @@ export type ApprovalDecision = 'once' | 'always' | 'deny'; export type AgentEvent = | { type: 'text'; text: string } | { type: 'reasoning'; text: string } + | { type: 'tool-start'; id: string; name: string } | { type: 'tool-call'; id: string; name: string; input: unknown } | { type: 'tool-output'; id: string; chunk: string } | { type: 'tool-result'; id: string; name: string; output: unknown } @@ -48,6 +55,8 @@ export type SessionOptions = { maxSteps?: number; /** MCP and subagent tools merged on top of the built-ins. */ extraTools?: ToolSet; + /** Tool sets offered this session; omit for all of them. `core` is always on. */ + toolSets?: readonly ToolSetName[]; /** Tool names that never prompt, e.g. the read-only subagent tool. */ autoApprove?: readonly string[]; /** Prune the history once the estimated token count crosses this. */ @@ -121,9 +130,14 @@ export class Session { return this.variant; } - /** Tool names offered this turn; a read-only variant hides the rest. */ + /** + * Tool names offered this turn. A read-only variant hides the mutating tools; + * a disabled tool set is withheld from the wire and from the prompt, since a + * prompt that names an absent tool teaches calls that cannot succeed. + */ activeTools(): string[] { - const all = Object.keys(this.tools); + const withheld = new Set(disabledToolNames(this.opts.toolSets)); + const all = Object.keys(this.tools).filter((name) => !withheld.has(name)); if (!this.variant.allowTools) return all; return all.filter((name) => this.variant.allowTools!.includes(name)); } @@ -300,6 +314,11 @@ export class Session { case 'reasoning-delta': yield { type: 'reasoning', text: part.text }; break; + case 'tool-input-start': + // Arrives before the arguments finish streaming, so the UI can name + // the tool while the model is still writing its input. + yield { type: 'tool-start', id: part.id, name: part.toolName }; + break; case 'tool-call': yield { type: 'tool-call', id: part.toolCallId, name: part.toolName, input: part.input }; break; diff --git a/src/tools-git.ts b/src/tools-git.ts new file mode 100644 index 0000000..58ed705 --- /dev/null +++ b/src/tools-git.ts @@ -0,0 +1,164 @@ +import { tool } from 'ai'; +import { z } from 'zod'; + +const MAX_OUTPUT = 30_000; +const MAX_LOG = 40; + +const cap = (s: string) => + s.length <= MAX_OUTPUT ? s : `${s.slice(0, MAX_OUTPUT)}\n... [truncated ${s.length - MAX_OUTPUT} chars]`; + +type GitResult = { ok: true; stdout: string } | { ok: false; message: string }; + +/** + * Runs git with an argument array, never a shell string. + * + * Arguments come from model output, so a shell would make `git log --author="; rm -rf /"` + * an injection. Spawning the binary directly with a fixed argv removes that entirely, + * which is also why these tools can be auto-approved. + */ +async function git(args: string[], cwd: string, timeout = 30_000): Promise { + let proc: Bun.Subprocess<'ignore', 'pipe', 'pipe'>; + try { + proc = Bun.spawn(['git', ...args], { cwd, stdout: 'pipe', stderr: 'pipe', timeout }); + } catch { + return { ok: false, message: 'git is not installed or not on PATH.' }; + } + + const [stdout, stderr, code] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + if (code !== 0) { + const message = stderr.trim() || stdout.trim() || `git exited ${code}`; + if (/not a git repository/i.test(message)) { + return { ok: false, message: `${cwd} is not a git repository.` }; + } + return { ok: false, message }; + } + + return { ok: true, stdout }; +} + +const run = async (args: string[], empty: string): Promise => { + const result = await git(args, process.cwd()); + if (!result.ok) throw new Error(result.message); + return cap(result.stdout.trim() || empty); +}; + +const STATUS_LABEL: Record = { + M: 'modified', + A: 'added', + D: 'deleted', + R: 'renamed', + C: 'copied', + U: 'conflicted', + '?': 'untracked', + '!': 'ignored', +}; + +export const gitStatusTool = 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.', + inputSchema: z.object({}), + execute: async () => { + const branch = await git(['rev-parse', '--abbrev-ref', 'HEAD'], process.cwd()); + if (!branch.ok) throw new Error(branch.message); + + const status = await git(['status', '--porcelain=v1'], process.cwd()); + if (!status.ok) throw new Error(status.message); + + const lines = status.stdout.split('\n').filter(Boolean); + if (lines.length === 0) return `On ${branch.stdout.trim()}, working tree clean.`; + + // Porcelain v1 packs staged and unstaged state into two leading columns; naming + // them is the difference between the model understanding the state and guessing. + const described = lines.slice(0, 200).map((line) => { + const staged = line[0] ?? ' '; + const unstaged = line[1] ?? ' '; + const path = line.slice(3); + const parts: string[] = []; + if (staged !== ' ' && staged !== '?') parts.push(`staged ${STATUS_LABEL[staged] ?? staged}`); + if (unstaged !== ' ') parts.push(`${STATUS_LABEL[unstaged] ?? unstaged}`); + return `${path} (${parts.join(', ') || 'unknown'})`; + }); + + return cap([`On ${branch.stdout.trim()}, ${lines.length} changed:`, ...described].join('\n')); + }, +}); + +export const gitDiffTool = 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.', + inputSchema: z.object({ + staged: z.boolean().optional().describe('Diff the index against HEAD instead of the working tree'), + path: z.string().optional().describe('Limit the diff to one file or directory'), + }), + execute: async ({ staged, path }) => { + const args = ['diff', '--no-color']; + if (staged) args.push('--staged'); + if (path) args.push('--', path); + return run(args, staged ? 'Nothing staged.' : 'No uncommitted changes.'); + }, +}); + +export const gitLogTool = 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.', + inputSchema: z.object({ + limit: z.number().int().min(1).max(MAX_LOG).optional().describe(`Commits to return, default 15, max ${MAX_LOG}`), + path: z.string().optional().describe('Only commits touching this file or directory'), + }), + execute: async ({ limit, path }) => { + const args = ['log', `-n${limit ?? 15}`, '--date=short', '--pretty=format:%h %ad %an %s']; + if (path) args.push('--', path); + return run(args, 'No commits.'); + }, +}); + +export const gitShowTool = 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.', + inputSchema: z.object({ + ref: z.string().describe('Commit hash, tag, or ref'), + path: z.string().optional().describe('Limit the diff to one file'), + }), + execute: async ({ ref, path }) => { + const args = ['show', '--no-color', '--date=short', ref]; + if (path) args.push('--', path); + return run(args, 'Nothing to show.'); + }, +}); + +export const gitBlameTool = 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.', + inputSchema: z.object({ + path: z.string().describe('File to blame'), + startLine: z.number().int().min(1).optional(), + endLine: z.number().int().min(1).optional(), + }), + execute: async ({ path, startLine, endLine }) => { + const args = ['blame', '--date=short', '-w']; + if (startLine) args.push('-L', `${startLine},${endLine ?? startLine + 40}`); + args.push('--', path); + return run(args, 'No blame output.'); + }, +}); + +export const gitTools = { + git_status: gitStatusTool, + git_diff: gitDiffTool, + git_log: gitLogTool, + git_show: gitShowTool, + git_blame: gitBlameTool, +}; + +/** Read-only, so none of these ever prompt for approval. */ +export const GIT_TOOL_NAMES = Object.keys(gitTools); diff --git a/src/tools.ts b/src/tools.ts index f2c946f..6547e34 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -1,7 +1,9 @@ import { tool } from 'ai'; -import { resolve } from 'node:path'; +import { stat } from 'node:fs/promises'; +import { join, resolve } from 'node:path'; import { z } from 'zod'; import { jail, posix, walk } from './ignore'; +import { GIT_TOOL_NAMES, gitTools } from './tools-git'; /** Max chars returned by any single tool. Beyond this the output is truncated. */ const MAX_OUTPUT = 30_000; @@ -23,6 +25,20 @@ async function isBinary(abs: string): Promise { return bytes.includes(0); } +/** + * One file, numbered. Shared by read_file and read_many_files so a batch read + * cannot drift from a single read in numbering or in what it refuses. + */ +async function readNumbered(path: string, offset: number, limit: number): Promise { + const abs = jail(path); + const file = Bun.file(abs); + if (!(await file.exists())) throw new Error(`No such file: ${path}`); + if (await isBinary(abs)) throw new Error(`${path} is a binary file, not text. Use bash if you need to inspect it.`); + const lines = (await file.text()).split('\n'); + const slice = lines.slice(offset - 1, offset - 1 + limit); + return slice.map((l, i) => `${offset + i}: ${l}`).join('\n'); +} + export const readFileTool = tool({ description: 'Read a UTF-8 text file. Returns contents with 1-based line numbers.', inputSchema: z.object({ @@ -30,14 +46,42 @@ export const readFileTool = tool({ offset: z.number().int().min(1).optional().describe('First line to return (1-based)'), limit: z.number().int().min(1).optional().describe('Max lines to return, default 2000'), }), - execute: async ({ path, offset = 1, limit = 2000 }) => { - const abs = jail(path); - const file = Bun.file(abs); - if (!(await file.exists())) throw new Error(`No such file: ${path}`); - if (await isBinary(abs)) throw new Error(`${path} is a binary file, not text. Use bash if you need to inspect it.`); - const lines = (await file.text()).split('\n'); - const slice = lines.slice(offset - 1, offset - 1 + limit); - return cap(slice.map((l, i) => `${offset + i}: ${l}`).join('\n')); + execute: async ({ path, offset = 1, limit = 2000 }) => cap(await readNumbered(path, offset, limit)), +}); + +const MAX_BATCH_FILES = 20; + +export const readManyFilesTool = 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 ' + + 'in its own block and does not stop the others, so a wrong guess costs one line rather than the whole call.', + inputSchema: z.object({ + files: z + .array( + z.object({ + path: z.string().describe('File path relative to the workspace root'), + offset: z.number().int().min(1).optional().describe('First line to return (1-based)'), + limit: z.number().int().min(1).optional().describe('Max lines to return, default 2000'), + }), + ) + .min(1) + .max(MAX_BATCH_FILES) + .describe(`The files to read, at most ${MAX_BATCH_FILES}`), + }), + execute: async ({ files }) => { + // The whole point is one round trip, so the reads run together rather than + // in sequence. A rejection is reported in place, not thrown. + const blocks = await Promise.all( + files.map(async ({ path, offset = 1, limit = 2000 }) => { + try { + return `===== ${path} =====\n${await readNumbered(path, offset, limit)}`; + } catch (e) { + return `===== ${path} =====\n[unreadable: ${e instanceof Error ? e.message : String(e)}]`; + } + }), + ); + return cap(blocks.join('\n\n')); }, }); @@ -82,6 +126,61 @@ export const editFileTool = tool({ }, }); +export const multiEditTool = 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 ' + + 'written. Prefer this over repeated edit_file calls on the same file — one approval, one write, no risk of ' + + 'leaving the file half-changed.', + inputSchema: z.object({ + path: z.string(), + edits: z + .array( + z.object({ + oldString: z.string().describe('Exact text to find, including whitespace and indentation'), + newString: z.string().describe('Replacement text'), + replaceAll: z.boolean().optional(), + }), + ) + .min(1) + .describe('Edits in the order they should be applied'), + }), + execute: async ({ path, edits }) => { + const abs = jail(path); + const file = Bun.file(abs); + if (!(await file.exists())) throw new Error(`No such file: ${path}`); + + const original = await file.text(); + let text = original; + const applied: string[] = []; + + // Every edit is validated and applied in memory first. A failure on edit three + // must not leave the first two on disk, which is the whole point of this tool. + for (const [i, edit] of edits.entries()) { + const { oldString, newString, replaceAll = false } = edit; + if (oldString === newString) throw new Error(`edit ${i + 1}: oldString and newString are identical`); + + const count = text.split(oldString).length - 1; + if (count === 0) { + throw new Error(`edit ${i + 1}: oldString not found in ${path}. No edits were applied.`); + } + if (count > 1 && !replaceAll) { + throw new Error( + `edit ${i + 1}: oldString appears ${count} times in ${path}. Add surrounding context or set replaceAll. No edits were applied.`, + ); + } + + text = replaceAll ? text.split(oldString).join(newString) : text.replace(oldString, newString); + applied.push(`${replaceAll ? count : 1}x`); + } + + if (text === original) throw new Error(`No change to ${path}: the edits cancel out.`); + + await Bun.write(abs, text); + return `Applied ${edits.length} edit(s) to ${path} (${applied.join(', ')})`; + }, +}); + export const globTool = tool({ description: 'Find files by glob pattern, e.g. "src/**/*.ts". Skips anything .gitignore excludes. Returns paths relative to the workspace root.', @@ -102,6 +201,63 @@ export const globTool = tool({ }, }); +const MAX_TREE_ENTRIES = 300; + +export const listDirTool = 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.', + inputSchema: z.object({ + path: z.string().optional().describe('Directory to list, relative to the workspace root. Default the root.'), + depth: z.number().int().min(1).max(6).optional().describe('How many levels deep, default 2'), + includeIgnored: z.boolean().optional().describe('Also show files git ignores'), + }), + execute: async ({ path = '.', depth = 2, includeIgnored = false }) => { + const root = jail(path); + if (!(await isDir(root))) throw new Error(`Not a directory: ${path}`); + + const dirs = new Set(); + const files: { rel: string; size: number }[] = []; + + for await (const rel of walk({ root, noIgnore: includeIgnored })) { + const parts = rel.split('/'); + // Past the depth limit only the ancestors are interesting: the deepest one + // stands in for everything under it. + const shown = Math.min(parts.length - 1, depth); + for (let i = 1; i <= shown; i++) dirs.add(parts.slice(0, i).join('/')); + if (parts.length <= depth) files.push({ rel, size: Bun.file(join(root, rel)).size }); + if (files.length + dirs.size >= MAX_TREE_ENTRIES) break; + } + + const indent = (rel: string) => ' '.repeat(rel.split('/').length - 1); + const name = (rel: string) => rel.split('/').at(-1)!; + const rows = [ + ...[...dirs].map((d) => ({ key: `${d}/`, line: `${indent(d)}${name(d)}/` })), + ...files.map((f) => ({ key: f.rel, line: `${indent(f.rel)}${name(f.rel)} ${humanSize(f.size)}` })), + ].sort((a, b) => a.key.localeCompare(b.key)); + + const label = path === '.' ? '.' : `${posix(path).replace(/\/+$/, '')}/`; + if (rows.length === 0) return `${label} is empty (or everything in it is ignored).`; + + 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 { + return (await stat(abs)).isDirectory(); + } catch { + return false; + } +} + +const humanSize = (bytes: number) => { + if (bytes < 1024) return `${bytes}B`; + if (bytes < 1024 * 1024) return `${Math.round(bytes / 1024)}K`; + return `${(bytes / 1024 / 1024).toFixed(1)}M`; +}; + type GrepArgs = { pattern: string; include?: string; ignoreCase?: boolean; includeIgnored?: boolean }; /** @@ -229,8 +385,59 @@ async function pump( return all; } +type Running = { command: string; proc: Bun.Subprocess; interrupted: boolean; killed?: Promise }; + +const running = new Map(); + +/** + * Kills the shell and everything it started. + * + * `cmd /c` and `bash -lc` run the real command as a child, and killing only the + * shell leaves that child alive holding both pipes open — the read never ends, so + * the interrupt looks like it did nothing until the command finishes on its own. + * Measured at 19 seconds for `ping -n 20` on Windows. + * + * The promise settles once the kill is done, which also matters on Windows, where a + * surviving grandchild keeps its working directory locked against deletion. + */ +function killTree(proc: Bun.Subprocess): Promise { + if (process.platform === 'win32' && proc.pid) { + try { + const taskkill = Bun.spawn(['taskkill', '/PID', String(proc.pid), '/T', '/F'], { + stdout: 'ignore', + stderr: 'ignore', + }); + return taskkill.exited; + } catch { + // taskkill missing: fall through to the plain kill below. + } + } + proc.kill(); + return proc.exited; +} + +/** + * Kills the commands currently in flight, leaving the turn alive. + * + * `esc` aborts everything, which means a runaway command can only be stopped by + * throwing away the turn with it. This kills the process and lets `execute` throw, + * so the model receives a tool error and takes its next step knowing what happened. + * Returns the commands killed, for the notice shown to the user. + */ +export function interruptBash(): string[] { + const killed: string[] = []; + for (const entry of running.values()) { + entry.interrupted = true; + entry.killed = killTree(entry.proc); + killed.push(entry.command); + } + return killed; +} + export const bashTool = tool({ - description: 'Run a shell command in the workspace root. Use for builds, tests, git, and package managers.', + 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.', inputSchema: z.object({ command: z.string(), timeout: z.number().int().min(1000).max(600_000).optional().describe('Timeout in ms, default 120000'), @@ -245,37 +452,100 @@ export const bashTool = tool({ ...(abortSignal ? { signal: abortSignal } : {}), }); - // Drained concurrently: a command that fills one pipe while we block on the - // other would deadlock, and buffering both hides progress for minutes. - const [stdout, stderr, exitCode] = await Promise.all([ - pump(proc.stdout as ReadableStream, toolCallId), - pump(proc.stderr as ReadableStream, toolCallId), - proc.exited, - ]); + const entry: Running = { command, proc, interrupted: false }; + running.set(toolCallId, entry); - return cap( - [ - `exit: ${exitCode}`, - proc.signalCode && `(killed by ${proc.signalCode}; timeout is ${timeout}ms)`, - stdout.trim() && `stdout:\n${stdout.trim()}`, - stderr.trim() && `stderr:\n${stderr.trim()}`, - ] + try { + // Drained concurrently: a command that fills one pipe while we block on the + // other would deadlock, and buffering both hides progress for minutes. + const [stdout, stderr, exitCode] = await Promise.all([ + pump(proc.stdout as ReadableStream, toolCallId), + pump(proc.stderr as ReadableStream, toolCallId), + proc.exited, + ]); + + const body = [stdout.trim() && `stdout:\n${stdout.trim()}`, stderr.trim() && `stderr:\n${stderr.trim()}`] .filter(Boolean) - .join('\n\n'), - ); + .join('\n\n'); + + // Thrown rather than returned: the model must not read a killed command as + // a command that ran and failed on its own terms. + if (entry.interrupted) { + throw new Error( + cap( + `The user interrupted this command. It did not finish, so its effects are unknown.\n${ + body || '(no output before it was killed)' + }`, + ), + ); + } + + return cap( + [ + `exit: ${exitCode}`, + proc.signalCode && `(killed by ${proc.signalCode}; timeout is ${timeout}ms)`, + body, + ] + .filter(Boolean) + .join('\n\n'), + ); + } finally { + // Awaited so the process really is gone before the tool returns. On Windows a + // surviving grandchild holds the cwd open, which breaks the very next command. + await entry.killed; + running.delete(toolCallId); + } }, }); export const tools = { read_file: readFileTool, + read_many_files: readManyFilesTool, write_file: writeFileTool, edit_file: editFileTool, + multi_edit: multiEditTool, + list_dir: listDirTool, glob: globTool, grep: grepTool, bash: bashTool, + ...gitTools, }; +/** + * 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. + */ +export const TOOL_SETS = { + core: ['read_file', 'write_file', 'edit_file', 'glob', 'grep', 'bash'], + 'edit-plus': ['multi_edit', 'list_dir', 'read_many_files'], + git: GIT_TOOL_NAMES, +} as const satisfies Record; + +export type ToolSetName = keyof typeof TOOL_SETS; + +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); + +/** 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)); +} + +/** + * 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. + */ +export function disabledToolNames(enabled: readonly ToolSetName[] | undefined): string[] { + if (!enabled) return []; + const live = new Set([...enabled, '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', 'bash'] as const; +export const MUTATING_TOOLS = ['write_file', 'edit_file', 'multi_edit', 'bash'] as const; export { jail }; diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 6b6e744..cfa52d0 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -4,16 +4,18 @@ import Spinner from 'ink-spinner'; import React, { useCallback, useEffect, useRef, useState } from 'react'; import { parseCommand, matchCommands, type CommandSpec } from '../commands'; import { THINKING_LEVELS, VARIANTS } from '../agents'; +import { completePath, matchPaths, pathToken } from '../complete'; import type { Config } from '../config'; import { TODO_MARK, type NotebookState } from '../notebook'; import { costOf, formatUsd, usageLine } from '../pricing'; import type { ApprovalDecision, ApprovalRequest, Session } from '../session'; import type { SubagentEvent } from '../subagent'; +import { interruptBash, toolSetOf } from '../tools'; import { AskPanel, type AskBridge, type AskPending } from './Ask'; import { Diff } from './Diff'; import { Markdown } from './Markdown'; import { Onboard, type OnboardResult } from './Onboard'; -import { InfoPanel, OutputPanel, StatusBar, SubagentPanel, TodoPanel, type SubagentView } from './Panels'; +import { InfoPanel, OutputPanel, QueuePanel, StatusBar, SubagentPanel, ThinkingPanel, TodoPanel, ActiveTool, FileMenu, type SubagentView } from './Panels'; import { PromptInput } from './PromptInput'; type Line = @@ -135,6 +137,8 @@ export type AppHooks = { saveSession: () => Promise; /** Loaded AGENTS.md-style files, for /context. */ instructionFiles: () => string[]; + /** Ignore-aware workspace paths for `@` completion, loaded on first use. */ + listPaths: () => Promise; /** Prompt to hand the model for /init. */ initPrompt: string; history: string[]; @@ -242,7 +246,16 @@ export function App({ const [menuIndex, setMenuIndex] = useState(0); const [menuDismissed, setMenuDismissed] = useState(false); const [inputGeneration, setInputGeneration] = useState(0); + const [inputCursor, setInputCursor] = useState(0); const [toolOutput, setToolOutput] = useState(''); + const [active, setActive] = useState<{ name: string; summary?: string } | undefined>(); + const [thinking, setThinking] = useState(''); + const [thinkingOpen, setThinkingOpen] = useState(false); + const [queue, setQueue] = useState([]); + const [cursor, setCursor] = useState(0); + const [paths, setPaths] = useState(); + const [fileIndex, setFileIndex] = useState(0); + const [fileDismissed, setFileDismissed] = useState(false); const [recall, setRecall] = useState(hooks.history); const [notebook, setNotebook] = useState(session.notebook.state()); const [agents, setAgents] = useState([]); @@ -254,6 +267,24 @@ export function App({ const menuOpen = matches.length > 0 && !menuDismissed && !busy && !modal && !anyPicker && !panel; const highlighted = matches[Math.min(menuIndex, matches.length - 1)]; + const token = pathToken(draft, cursor); + const fileOpen = token !== undefined && !fileDismissed && !modal && !anyPicker; + const fileMatches = token && paths ? matchPaths(paths, token.query) : []; + const highlightedPath = fileMatches[Math.min(fileIndex, Math.max(0, fileMatches.length - 1))]; + + // The walk costs a full ignore-aware traversal, so it happens on the first `@` + // rather than at startup, and only once. + useEffect(() => { + if (token === undefined || paths !== undefined) return; + let live = true; + void hooks.listPaths().then((all) => { + if (live) setPaths(all); + }); + return () => { + live = false; + }; + }, [hooks, paths, token]); + useEffect(() => bridge.bind(setPending), [bridge]); useEffect(() => askBridge?.bind(setAsking), [askBridge]); @@ -265,16 +296,31 @@ export function App({ [subagents], ); - // Ink re-renders the whole tree per setState, so deltas accumulate in a ref + // Ink re-renders the whole tree per setState, so deltas accumulate in refs // and are flushed on a timer instead of once per token. const text = useRef(''); + const reasoning = useRef(''); useEffect(() => { const t = setInterval(() => { setLive((s) => (s === text.current ? s : text.current)); + setThinking((s) => (s === reasoning.current ? s : reasoning.current)); }, 60); return () => clearInterval(t); }, []); + // The queue is a ref as well as state: runTurn drains it synchronously as the + // turn ends, and a stale closure over the array would lose a prompt. + const queued = useRef([]); + const busyRef = useRef(false); + const submitRef = useRef<((raw: string) => Promise) | undefined>(undefined); + + // Kept in step with busy, since the queue drain reads it synchronously between + // renders and a state read there would be one turn stale. + const setWorking = useCallback((value: boolean) => { + busyRef.current = value; + setBusy(value); + }, []); + const push = useCallback((line: NewLine) => { setHistory((h) => [...h, { ...line, key: nextKey() }]); }, []); @@ -282,12 +328,31 @@ export function App({ useEffect(() => notices?.bind((text) => push({ kind: 'info', text })), [notices, push]); useInput( - (_input, key) => { - if (key.escape) session.abort(); + (input, key) => { + // esc drops the queue too: interrupting and then watching two more prompts + // fire anyway is not what anyone means by interrupt. + if (key.escape) { + queued.current.length = 0; + setQueue([]); + session.abort(); + return; + } + if (key.ctrl && input === 'r') setThinkingOpen((o) => !o); }, { isActive: busy && !modal }, ); + // ctrl-c kills only the command in flight, leaving the turn alive so the model + // gets a tool error and can decide what to do. With nothing running it keeps its + // usual meaning and quits, which is why Ink's own ctrl-c handling is turned off + // in cli.tsx rather than left to race with this. + useInput((input, key) => { + if (!key.ctrl || input !== 'c') return; + const killed = interruptBash(); + if (killed.length === 0) return exit(); + push({ kind: 'info', text: `interrupted: ${killed.join(', ')}` }); + }); + useInput( (_input, key) => { if (!key.escape) return; @@ -298,14 +363,43 @@ export function App({ { isActive: anyPicker }, ); - // PromptInput hands up/down/tab/esc to us first, so the menu and any open panel + // PromptInput hands up/down/tab/esc to us first, so the menus and any open panel // can claim them before the input treats them as editing keys. const handleInputKey = useCallback( - (_input: string, key: { upArrow: boolean; downArrow: boolean; tab: boolean; escape: boolean }) => { + (_input: string, key: { upArrow: boolean; downArrow: boolean; tab: boolean; escape: boolean; return: boolean }) => { if (key.escape && panel) { setPanel(undefined); return true; } + + // The file picker gets first refusal: while an `@` token is open its keys + // mean navigation, not history recall or command completion. + if (fileOpen) { + if (key.escape) { + setFileDismissed(true); + return true; + } + if (fileMatches.length > 0) { + if (key.upArrow) { + setFileIndex((i) => (i - 1 + fileMatches.length) % fileMatches.length); + return true; + } + if (key.downArrow) { + setFileIndex((i) => (i + 1) % fileMatches.length); + return true; + } + if ((key.tab || key.return) && highlightedPath && token) { + const next = completePath(draft, token, highlightedPath); + setDraft(next.value); + setCursor(next.cursor); + setFileIndex(0); + setInputCursor(next.cursor); + setInputGeneration((g) => g + 1); + return true; + } + } + } + if (!menuOpen) return false; if (key.escape) { setMenuDismissed(true); @@ -320,47 +414,65 @@ export function App({ return true; } if (key.tab && highlighted) { - setDraft(highlighted.arg ? `/${highlighted.name} ` : `/${highlighted.name}`); + const value = highlighted.arg ? `/${highlighted.name} ` : `/${highlighted.name}`; + setDraft(value); + setCursor(value.length); setMenuIndex(0); setMenuDismissed(true); + setInputCursor(value.length); setInputGeneration((g) => g + 1); return true; } return false; }, - [highlighted, matches.length, menuOpen, panel], + [draft, fileMatches.length, fileOpen, highlighted, highlightedPath, matches.length, menuOpen, panel, token], ); - const onDraftChange = useCallback((value: string) => { + const onDraftChange = useCallback((value: string, at: number) => { setDraft(value); + setCursor(at); setMenuIndex(0); setMenuDismissed(false); + setFileIndex(0); + setFileDismissed(false); }, []); const runTurn = useCallback( async (value: string) => { - setBusy(true); + setWorking(true); text.current = ''; + reasoning.current = ''; + setThinking(''); for await (const ev of session.send(value)) { switch (ev.type) { case 'text': text.current += ev.text; break; + case 'reasoning': + reasoning.current += ev.text; + break; + case 'tool-start': + setActive({ name: ev.name }); + break; case 'tool-call': + setActive({ name: ev.name, summary: preview(ev.input) }); push({ kind: 'tool', name: ev.name, summary: preview(ev.input), ok: true }); break; case 'tool-output': setToolOutput((s) => `${s}${ev.chunk}`.slice(-2000)); break; case 'tool-error': + setActive(undefined); push({ kind: 'tool', name: ev.name, summary: String(ev.error), ok: false }); break; case 'tool-result': + setActive(undefined); setToolOutput(''); setNotebook(session.notebook.state()); break; case 'tool-denied': + setActive(undefined); push({ kind: 'info', text: `denied ${ev.name}` }); break; case 'notice': @@ -375,7 +487,11 @@ export function App({ case 'done': { const full = text.current.trim(); text.current = ''; + // Reasoning is progress, not the answer, so it leaves with the turn. + reasoning.current = ''; + setThinking(''); setLive(''); + setActive(undefined); setToolOutput(''); setAgents([]); setHistory((h) => { @@ -396,16 +512,29 @@ export function App({ break; } } - setBusy(false); + + setWorking(false); + + // Drain one queued prompt per finished turn, in order. Going back through + // submit means a queued slash command behaves exactly as if typed now, and + // its own turn drains the next one. + const next = queued.current.shift(); + if (next !== undefined) { + setQueue([...queued.current]); + await submitRef.current?.(next); + } }, - [hooks, push, session], + [hooks, push, session, setWorking], ); const submit = useCallback( async (raw: string) => { setDraft(''); + setCursor(0); setMenuIndex(0); setMenuDismissed(false); + setFileIndex(0); + setFileDismissed(false); setPanel(undefined); // Enter on an open menu runs the highlighted entry, so `/mo` + enter works. @@ -421,6 +550,15 @@ export function App({ break; } + // Typed during a turn: queue it whole, including a slash command, and let + // the drain replay it once the model is free. Losing the thought to a + // swallowed keystroke is the thing this exists to prevent. + if (busyRef.current) { + queued.current.push(chosen); + setQueue([...queued.current]); + return; + } + // Nothing can reach the model until a provider is configured. if (unconfigured && action.type !== 'provider' && action.type !== 'info') { push({ kind: 'user', text: chosen.trim() }); @@ -453,7 +591,10 @@ export function App({ body: session .activeTools() .sort() - .map((t) => `- \`${t}\``) + .map((t) => { + const set = toolSetOf(t); + return `- \`${t}\`${set ? ` ${set}` : ''}`; + }) .join('\n'), }); return; @@ -537,13 +678,13 @@ export function App({ return; case 'memory': { push({ kind: 'user', text: chosen.trim() }); - setBusy(true); + setWorking(true); try { push({ kind: 'info', text: await hooks.summarizeMemory() }); } catch (e) { push({ kind: 'error', text: e instanceof Error ? e.message : String(e) }); } - setBusy(false); + setWorking(false); return; } case 'init': @@ -582,9 +723,9 @@ export function App({ return; case 'models': { push({ kind: 'user', text: chosen.trim() }); - setBusy(true); + setWorking(true); const { models, warning } = await hooks.listModels(); - setBusy(false); + setWorking(false); if (warning) push({ kind: 'info', text: `could not list models: ${warning}` }); if (models.length === 0) { push({ kind: 'error', text: 'no models to choose from - use /model or /provider' }); @@ -595,14 +736,14 @@ export function App({ } case 'compact': { push({ kind: 'user', text: chosen.trim() }); - setBusy(true); + setWorking(true); try { const { before, after } = await session.summarize(); push({ kind: 'info', text: `compacted ${before} messages into ${after}` }); } catch (e) { push({ kind: 'error', text: e instanceof Error ? e.message : String(e) }); } - setBusy(false); + setWorking(false); return; } case 'prompt': @@ -613,9 +754,13 @@ export function App({ return; } }, - [exit, highlighted, hooks, menuOpen, push, runTurn, session, unconfigured, write], + [exit, highlighted, hooks, menuOpen, push, runTurn, session, setWorking, unconfigured, write], ); + useEffect(() => { + submitRef.current = submit; + }, [submit]); + return ( @@ -752,6 +897,8 @@ export function App({ {busy && !modal && ( + + {active && } working... esc to interrupt @@ -759,21 +906,32 @@ export function App({ )} - {!busy && !modal && !anyPicker && ( + {!modal && !anyPicker && ( + {'> '} - {menuOpen && } + {fileOpen ? ( + + ) : ( + menuOpen && + )} + + + + {` ${name}`} + {summary ? {` ${summary}`} : null} + + ); +} + +/** + * The model's reasoning while it streams. + * + * Collapsed by default: it is progress, not the answer, and expanding it by default + * would bury the reply. The token count is an estimate from character length, which + * is close enough to tell a long think from a short one. + */ +export function ThinkingPanel({ text, expanded, lines = 8 }: { text: string; expanded?: boolean; lines?: number }) { + if (text.length === 0) return null; + const tokens = Math.round(text.length / 4); + + if (!expanded) { + return {`thinking... ~${tokens} tokens ctrl-r to expand`}; + } + + return ( + + {`thinking ~${tokens} tokens ctrl-r to collapse`} + {text + .split('\n') + .slice(-lines) + .map((l, i) => ( + + {` ${l}`} + + ))} + + ); +} + +/** Prompts typed during a turn, waiting their place in line. */ +export function QueuePanel({ prompts }: { prompts: readonly string[] }) { + if (prompts.length === 0) return null; + return ( + + {`queued: ${prompts.length}`} + {prompts.map((p, i) => ( + + {` ${i + 1}. ${p.length > 70 ? `${p.slice(0, 70)}...` : p}`} + + ))} + + ); +} + +/** Path picker for an `@` token, narrowing as the query grows. */ +export function FileMenu({ + paths, + index, + query, + loading, +}: { + paths: readonly string[]; + index: number; + query: string; + loading?: boolean; +}) { + if (loading) { + return ( + + indexing files... + + ); + } + + if (paths.length === 0) { + return ( + + {`no file matches ${query || '@'}`} + + ); + } + + return ( + + {paths.map((p, i) => ( + + {i === index ? '> ' : ' '} + {p} + + ))} + up/down move | tab or enter insert | esc dismiss + + ); +} + /** Status line under the transcript: model, agent, thinking, context, spend. */ export function StatusBar({ model, diff --git a/src/ui/PromptInput.tsx b/src/ui/PromptInput.tsx index 5601396..66bff96 100644 --- a/src/ui/PromptInput.tsx +++ b/src/ui/PromptInput.tsx @@ -3,7 +3,8 @@ import React, { useEffect, useState } from 'react'; export type PromptInputProps = { value: string; - onChange: (value: string) => void; + /** Cursor is reported alongside the value: `@path` completion needs to know where it is. */ + onChange: (value: string, cursor: number) => void; onSubmit: (value: string) => void; placeholder?: string; focus?: boolean; @@ -12,6 +13,8 @@ export type PromptInputProps = { history?: readonly string[]; /** Intercept a key before the input consumes it. Return true to swallow it. */ onKey?: (input: string, key: KeyLike) => boolean; + /** Where to put the cursor on mount, for a remounted input after a completion. */ + initialCursor?: number; }; type KeyLike = { @@ -51,8 +54,9 @@ export function PromptInput({ mask, history = [], onKey, + initialCursor, }: PromptInputProps) { - const [cursor, setCursor] = useState(value.length); + const [cursor, setCursor] = useState(initialCursor ?? value.length); // -1 means "editing a fresh line"; 0+ indexes back from the newest entry. const [recall, setRecall] = useState(-1); const [stash, setStash] = useState(''); @@ -62,8 +66,9 @@ export function PromptInput({ }, [value]); const set = (next: string, nextCursor = next.length) => { - onChange(next); - setCursor(Math.max(0, Math.min(nextCursor, next.length))); + const clamped = Math.max(0, Math.min(nextCursor, next.length)); + onChange(next, clamped); + setCursor(clamped); }; useInput( diff --git a/test/complete-ui.test.tsx b/test/complete-ui.test.tsx new file mode 100644 index 0000000..59d5dea --- /dev/null +++ b/test/complete-ui.test.tsx @@ -0,0 +1,222 @@ +import { expect, test } from 'bun:test'; +import { render } from 'ink-testing-library'; +import React from 'react'; +import { MockLanguageModelV4, simulateReadableStream } from 'ai/test'; +import { Session } from '../src/session'; +import { App, createApprovalBridge } from '../src/ui/App'; +import { testHooks } from './helpers'; + +const usage = { inputTokens: { total: 3, noCache: 3, cacheRead: 0, cacheWrite: 0 }, outputTokens: { total: 1 } } as any; + +const model = new MockLanguageModelV4({ + doStream: async () => + ({ + stream: simulateReadableStream({ + chunks: [ + { type: 'text-start', id: '0' }, + { type: 'text-delta', id: '0', delta: 'reply' }, + { type: 'text-end', id: '0' }, + { type: 'finish', finishReason: { unified: 'stop', raw: 'stop' }, usage }, + ], + chunkDelayInMs: null, + initialDelayInMs: null, + }), + }) as any, +}); + +const paths = ['README.md', 'src/app.ts', 'src/session.ts', 'src/ui/App.tsx', 'test/session.test.ts']; + +const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); +const DOWN = '\u001B[B'; +const TAB = '\t'; + +function mount(listPaths = async () => paths) { + const sent: string[] = []; + const bridge = createApprovalBridge(); + const session = new Session({ model, askApproval: bridge.ask }); + const app = render( + sent.push(t) })} + />, + ); + return { app, sent }; +} + +async function type(app: ReturnType, s: string) { + for (const ch of s) { + app.stdin.write(ch); + await wait(30); + } +} + +test('@ opens the file picker and typing narrows it', async () => { + const { app } = mount(); + await wait(150); + + await type(app, '@'); + await wait(200); + expect(app.lastFrame()).toContain('README.md'); + + await type(app, 'src/ui/'); + await wait(200); + + const frame = app.lastFrame() ?? ''; + expect(frame).toContain('src/ui/App.tsx'); + expect(frame).not.toContain('README.md'); + expect(frame).not.toContain('test/session.test.ts'); + + app.unmount(); +}, 25_000); + +test('tab inserts the highlighted path with no @', async () => { + const { app } = mount(); + await wait(150); + + await type(app, 'explain @src/ses'); + await wait(200); + expect(app.lastFrame()).toContain('src/session.ts'); + + app.stdin.write(TAB); + await wait(250); + + const frame = app.lastFrame() ?? ''; + expect(frame).toContain('explain src/session.ts'); + expect(frame).not.toContain('@src'); + expect(frame).not.toContain('up/down move | tab or enter insert'); + + app.unmount(); +}, 25_000); + +test('down then tab inserts the second match', async () => { + const { app } = mount(); + await wait(150); + + await type(app, '@src/'); + await wait(200); + app.stdin.write(DOWN); + await wait(150); + app.stdin.write(TAB); + await wait(250); + + expect(app.lastFrame()).toContain('src/session.ts'); + + app.unmount(); +}, 25_000); + +test('a completed prompt submits as a plain path', async () => { + const { app, sent } = mount(); + await wait(150); + + await type(app, '@src/app'); + await wait(200); + app.stdin.write(TAB); + await wait(250); + app.stdin.write('\r'); + await wait(400); + + expect(sent).toHaveLength(1); + expect(sent[0]).toBe('src/app.ts'); + + app.unmount(); +}, 25_000); + +test('esc dismisses the picker and leaves the text alone', async () => { + const { app } = mount(); + await wait(150); + + await type(app, '@src/'); + await wait(200); + expect(app.lastFrame()).toContain('src/app.ts'); + + app.stdin.write('\u001B'); + await wait(250); + + const frame = app.lastFrame() ?? ''; + expect(frame).not.toContain('tab or enter insert'); + expect(frame).toContain('@src/'); + + app.unmount(); +}, 25_000); + +test('a query matching nothing says so instead of showing a stale list', async () => { + const { app } = mount(); + await wait(150); + + await type(app, '@zzzz'); + await wait(250); + + expect(app.lastFrame()).toContain('no file matches zzzz'); + + app.unmount(); +}, 25_000); + +test('an @ inside a word is not a completion', async () => { + const { app } = mount(); + await wait(150); + + await type(app, 'mail me@example'); + await wait(250); + + const frame = app.lastFrame() ?? ''; + expect(frame).not.toContain('tab or enter insert'); + expect(frame).not.toContain('no file matches'); + + app.unmount(); +}, 25_000); + +test('the walk is reported as loading and only runs once', async () => { + let calls = 0; + const { app } = mount(async () => { + calls++; + await wait(400); + return paths; + }); + await wait(150); + + await type(app, '@'); + await wait(80); + expect(app.lastFrame()).toContain('indexing files'); + + await wait(600); + expect(app.lastFrame()).toContain('README.md'); + + // Dismiss, reopen: the list is cached rather than walked again. + app.stdin.write('\u001B'); + await wait(150); + await type(app, ' @src/'); + await wait(300); + + expect(app.lastFrame()).toContain('src/app.ts'); + expect(calls).toBe(1); + + app.unmount(); +}, 25_000); + +test('the command menu still works and does not fight the file picker', async () => { + const { app } = mount(); + await wait(150); + + await type(app, '/mo'); + await wait(200); + + const frame = app.lastFrame() ?? ''; + expect(frame).toContain('models'); + expect(frame).not.toContain('tab or enter insert'); + + app.unmount(); +}, 25_000); + +test('ctrl-c with nothing running does not kill a command that is not there', async () => { + const { app } = mount(); + await wait(150); + + app.stdin.write('\u0003'); + await wait(250); + + expect(app.lastFrame() ?? '').not.toContain('interrupted:'); + + app.unmount(); +}, 25_000); diff --git a/test/complete.test.ts b/test/complete.test.ts new file mode 100644 index 0000000..393cb8a --- /dev/null +++ b/test/complete.test.ts @@ -0,0 +1,89 @@ +import { expect, test } from 'bun:test'; +import { completePath, matchPaths, pathToken } from '../src/complete'; + +const paths = [ + 'README.md', + 'src/app.ts', + 'src/session.ts', + 'src/ui/App.tsx', + 'src/ui/Panels.tsx', + 'test/session.test.ts', + 'vendor/src/legacy.ts', +]; + +test('a bare @ opens the token with an empty query', () => { + expect(pathToken('@', 1)).toEqual({ start: 0, end: 1, query: '' }); +}); + +test('the token is the text between @ and the cursor', () => { + expect(pathToken('look at @src/ses', 16)).toEqual({ start: 8, end: 16, query: 'src/ses' }); +}); + +test('@ mid-word is not a completion, so an email is left alone', () => { + expect(pathToken('mail me@example.com', 19)).toBeUndefined(); + expect(pathToken('user@host', 9)).toBeUndefined(); +}); + +test('a space ends the token', () => { + expect(pathToken('@src/app.ts and then', 20)).toBeUndefined(); +}); + +test('the cursor before the @ sees no token', () => { + expect(pathToken('@src', 0)).toBeUndefined(); +}); + +test('the nearest @ wins when there are two', () => { + const token = pathToken('@first then @sec', 16); + expect(token?.query).toBe('sec'); + expect(token?.start).toBe(12); +}); + +test('an empty query offers the shallowest paths first', () => { + expect(matchPaths(paths, '', 3)).toEqual(['README.md', 'src/app.ts', 'src/session.ts']); +}); + +test('a directory prefix narrows to what is under it', () => { + const hits = matchPaths(paths, 'src/ui/'); + expect(hits).toEqual(['src/ui/App.tsx', 'src/ui/Panels.tsx']); +}); + +test('prefix matches rank above substring matches', () => { + const hits = matchPaths(paths, 'src/'); + expect(hits[0]).toBe('src/app.ts'); + // vendor/src/legacy.ts contains "src/" but is not under src/, so it comes last. + expect(hits.at(-1)).toBe('vendor/src/legacy.ts'); + expect(hits.indexOf('src/session.ts')).toBeLessThan(hits.indexOf('vendor/src/legacy.ts')); +}); + +test('matching ignores case', () => { + expect(matchPaths(paths, 'readme')).toEqual(['README.md']); +}); + +test('a query matching nothing yields nothing', () => { + expect(matchPaths(paths, 'zzz')).toEqual([]); +}); + +test('the limit is honoured', () => { + expect(matchPaths(paths, 's', 2)).toHaveLength(2); +}); + +test('completion replaces the token with a plain relative path', () => { + const value = 'look at @src/ses'; + const token = pathToken(value, value.length)!; + expect(completePath(value, token, 'src/session.ts')).toEqual({ + value: 'look at src/session.ts ', + cursor: 23, + }); +}); + +test('completion keeps whatever followed the cursor', () => { + const value = '@src/ses and explain'; + const token = pathToken(value, 8)!; + const { value: next } = completePath(value, token, 'src/session.ts'); + expect(next).toBe('src/session.ts and explain'); +}); + +test('the inserted path carries no @', () => { + const token = pathToken('@READ', 5)!; + expect(completePath('@READ', token, 'README.md').value).not.toContain('@'); +}); diff --git a/test/helpers.ts b/test/helpers.ts index 7e013e5..1682803 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -20,6 +20,7 @@ export function testHooks(over: Partial = {}): AppHooks { resumeSession: async () => 'resumed', saveSession: async () => 'saved', instructionFiles: () => [], + listPaths: async () => [], initPrompt: 'write AGENTS.md', history: [], recordPrompt: () => {}, diff --git a/test/prune.test.ts b/test/prune.test.ts index e441c58..0a2c6c2 100644 --- a/test/prune.test.ts +++ b/test/prune.test.ts @@ -1,6 +1,6 @@ import { expect, test } from 'bun:test'; import type { ModelMessage } from 'ai'; -import { dropOrphanedItems, prunePreservingItems } from '../src/prune'; +import { dropOrphanedItems, dropOrphanedResults, prunePreservingItems } from '../src/prune'; const kinds = (messages: ModelMessage[]) => messages.map((m) => (Array.isArray(m.content) ? `${m.role}:${m.content.map((p) => p.type).join('+')}` : m.role)); @@ -152,3 +152,99 @@ test('a provider other than openai is handled the same way', () => { ]; expect(dropOrphanedItems(before, after)).toEqual([]); }); + +/** The assistant tool-call plus the tool message answering it, as one exchange. */ +const callAndResult = (call: string, rs?: string): ModelMessage[] => [ + { + role: 'assistant', + content: [ + ...(rs ? [{ type: 'reasoning' as const, text: 'deciding', providerOptions: { openai: { itemId: rs } } }] : []), + { type: 'tool-call', toolCallId: call, toolName: 'grep', input: { pattern: 'x' } }, + ], + }, + { + role: 'tool', + content: [{ type: 'tool-result', toolCallId: call, toolName: 'grep', output: { type: 'text', value: 'hit' } }], + }, +]; + +test('a tool result left without its tool call is dropped', () => { + const [, resultMessage] = callAndResult('call_A'); + const cleaned = dropOrphanedResults([{ role: 'user', content: 'q' }, resultMessage!]); + expect(JSON.stringify(cleaned)).not.toContain('call_A'); + expect(kinds(cleaned)).toEqual(['user']); +}); + +test('a tool result keeps its place while the call is still there', () => { + const messages: ModelMessage[] = [{ role: 'user', content: 'q' }, ...callAndResult('call_A')]; + expect(dropOrphanedResults(messages)).toEqual(messages); +}); + +test('a tool call awaiting its result survives, since that is a suspended approval', () => { + const [callMessage] = callAndResult('call_A'); + const messages: ModelMessage[] = [{ role: 'user', content: 'q' }, callMessage!]; + expect(dropOrphanedResults(messages)).toEqual(messages); +}); + +test('a tool-error is treated as a result and dropped with its call', () => { + const messages: ModelMessage[] = [ + { role: 'user', content: 'q' }, + { + role: 'tool', + content: [{ type: 'tool-error', toolCallId: 'call_A', toolName: 'grep', error: 'boom' } as never], + }, + ]; + expect(JSON.stringify(dropOrphanedResults(messages))).not.toContain('call_A'); +}); + +test('only the orphaned result is dropped, not a healthy one beside it', () => { + const messages: ModelMessage[] = [ + { role: 'user', content: 'q' }, + ...callAndResult('call_LIVE'), + { + role: 'tool', + content: [ + { type: 'tool-result', toolCallId: 'call_LIVE', toolName: 'grep', output: { type: 'text', value: 'a' } }, + { type: 'tool-result', toolCallId: 'call_GONE', toolName: 'grep', output: { type: 'text', value: 'b' } }, + ], + }, + ]; + + const json = JSON.stringify(dropOrphanedResults(messages)); + expect(json).toContain('call_LIVE'); + expect(json).not.toContain('call_GONE'); +}); + +/** + * The 400 this guards against: "No tool call found for function call output with + * call_id ...". Pruning counts messages, so its cut lands between the assistant + * tool-call and the tool message answering it, stranding the result on the wire. + */ +test('prunePreservingItems never strands a tool result on the wire', () => { + const messages: ModelMessage[] = [{ role: 'user', content: `q ${'x'.repeat(4000)}` }]; + for (let i = 0; i < 5; i++) { + messages.push(...callAndResult(`call_${i}`, `rs_${i}`)); + messages.push({ role: 'user', content: `follow up ${i} ${'y'.repeat(4000)}` }); + } + + const pruned = prunePreservingItems({ + messages, + reasoning: 'all', + toolCalls: 'before-last-3-messages', + emptyMessages: 'remove', + }); + + const calls = new Set(); + for (const m of pruned) { + if (!Array.isArray(m.content)) continue; + for (const p of m.content as { type: string; toolCallId?: string }[]) { + if (p.type === 'tool-call' && p.toolCallId) calls.add(p.toolCallId); + } + } + for (const m of pruned) { + if (!Array.isArray(m.content)) continue; + for (const p of m.content as { type: string; toolCallId?: string }[]) { + if (p.type === 'tool-result' || p.type === 'tool-error') expect(calls.has(p.toolCallId!)).toBe(true); + } + } +}); diff --git a/test/queue.test.tsx b/test/queue.test.tsx new file mode 100644 index 0000000..2154c9a --- /dev/null +++ b/test/queue.test.tsx @@ -0,0 +1,207 @@ +import { expect, test } from 'bun:test'; +import { render } from 'ink-testing-library'; +import React from 'react'; +import { MockLanguageModelV4, simulateReadableStream } from 'ai/test'; +import type { LanguageModelV4StreamPart } from '@ai-sdk/provider'; +import { Session } from '../src/session'; +import { App, createApprovalBridge } from '../src/ui/App'; +import { testHooks } from './helpers'; + +const usage = { + inputTokens: { total: 4, noCache: 4, cacheRead: 0, cacheWrite: 0 }, + outputTokens: { total: 2 }, +} as any; + +const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** One assistant reply, delivered slowly enough to type during. */ +const slowReply = (body: string): LanguageModelV4StreamPart[] => [ + { type: 'text-start', id: '0' }, + { type: 'text-delta', id: '0', delta: body }, + { type: 'text-end', id: '0' }, + { type: 'finish', finishReason: { unified: 'stop', raw: 'stop' }, usage }, +]; + +function mount(chunkDelayInMs: number) { + const prompts: string[] = []; + const model = new MockLanguageModelV4({ + doStream: async (opts) => { + const last = opts.prompt.at(-1); + const content = last?.content; + prompts.push(typeof content === 'string' ? content : JSON.stringify(content)); + return { + stream: simulateReadableStream({ + chunks: slowReply(`reply ${prompts.length}`), + chunkDelayInMs, + initialDelayInMs: null, + }), + }; + }, + }); + + const bridge = createApprovalBridge(); + const session = new Session({ model, askApproval: bridge.ask }); + const app = render(); + return { app, prompts, session }; +} + +async function type(app: ReturnType, s: string) { + for (const ch of s) { + app.stdin.write(ch); + await wait(25); + } + app.stdin.write('\r'); + await wait(60); +} + +test('the input stays live while a turn runs, and a submission is queued', async () => { + const { app, prompts } = mount(600); + await wait(150); + + await type(app, 'first'); + await wait(200); + + // Mid-turn: the spinner and the input coexist rather than swapping. The + // placeholder's first character is inverted for the cursor, hence the offset. + const midTurn = app.lastFrame() ?? ''; + expect(midTurn).toContain('working...'); + expect(midTurn).toContain('ype to queue'); + + await type(app, 'second'); + await wait(150); + + expect(app.lastFrame()).toContain('queued: 1'); + expect(prompts).toHaveLength(1); + + app.unmount(); +}, 20_000); + +test('two prompts typed during a turn run in order afterwards', async () => { + const { app, prompts } = mount(400); + await wait(150); + + await type(app, 'first'); + await wait(120); + await type(app, 'second'); + await type(app, 'third'); + + expect(app.lastFrame()).toContain('queued: 2'); + + await wait(3000); + + expect(prompts).toHaveLength(3); + expect(prompts[0]).toContain('first'); + expect(prompts[1]).toContain('second'); + expect(prompts[2]).toContain('third'); + expect(app.lastFrame()).not.toContain('queued:'); + + app.unmount(); +}, 25_000); + +test('esc clears the queue as well as aborting the turn', async () => { + const { app, prompts } = mount(800); + await wait(150); + + await type(app, 'first'); + await wait(150); + await type(app, 'queued one'); + await type(app, 'queued two'); + expect(app.lastFrame()).toContain('queued: 2'); + + app.stdin.write('\u001B'); + await wait(1200); + + expect(app.lastFrame()).not.toContain('queued:'); + expect(prompts).toHaveLength(1); + + app.unmount(); +}, 25_000); + +test('reasoning shows as a collapsed line before any text arrives, then leaves with the turn', async () => { + const model = new MockLanguageModelV4({ + doStream: async () => ({ + stream: simulateReadableStream({ + chunks: [ + { type: 'reasoning-start', id: 'r' }, + { type: 'reasoning-delta', id: 'r', delta: 'weighing the options at some length' }, + { type: 'reasoning-end', id: 'r' }, + { type: 'text-start', id: '0' }, + { type: 'text-delta', id: '0', delta: 'the answer' }, + { type: 'text-end', id: '0' }, + { type: 'finish', finishReason: { unified: 'stop', raw: 'stop' }, usage }, + ] as LanguageModelV4StreamPart[], + chunkDelayInMs: 250, + initialDelayInMs: null, + }), + }), + }); + + const bridge = createApprovalBridge(); + const session = new Session({ model, askApproval: bridge.ask }); + const app = render(); + await wait(150); + + await type(app, 'think about it'); + await wait(700); + + const thinking = app.lastFrame() ?? ''; + expect(thinking).toContain('thinking...'); + expect(thinking).toContain('tokens'); + expect(thinking).not.toContain('weighing the options'); + + await wait(2500); + + const done = app.lastFrame() ?? ''; + expect(done).toContain('the answer'); + expect(done).not.toContain('thinking'); + + app.unmount(); +}, 25_000); + +test('the tool in flight is named on screen and cleared when it returns', async () => { + const orig = process.cwd(); + let n = 0; + const model = new MockLanguageModelV4({ + doStream: async () => { + const chunks: LanguageModelV4StreamPart[] = + n++ === 0 + ? [ + { type: 'tool-input-start', id: 'c1', toolName: 'read_file' }, + { type: 'tool-input-end', id: 'c1' }, + { + type: 'tool-call', + toolCallId: 'c1', + toolName: 'read_file', + input: JSON.stringify({ path: 'src/session.ts' }), + }, + { type: 'finish', finishReason: { unified: 'tool-calls', raw: 'tool_use' }, usage }, + ] + : [ + { type: 'text-start', id: '0' }, + { type: 'text-delta', id: '0', delta: 'read it' }, + { type: 'text-end', id: '0' }, + { type: 'finish', finishReason: { unified: 'stop', raw: 'stop' }, usage }, + ]; + return { stream: simulateReadableStream({ chunks, chunkDelayInMs: 200, initialDelayInMs: null }) }; + }, + }); + + try { + const bridge = createApprovalBridge(); + const session = new Session({ model, askApproval: bridge.ask }); + const app = render(); + await wait(150); + + await type(app, 'read the session file'); + await wait(500); + + expect(app.lastFrame()).toContain('read_file'); + + await wait(2500); + expect(app.lastFrame()).toContain('read it'); + + app.unmount(); + } finally { + process.chdir(orig); + } +}, 25_000); diff --git a/test/session-features.test.ts b/test/session-features.test.ts index d5af5f5..4fa3a23 100644 --- a/test/session-features.test.ts +++ b/test/session-features.test.ts @@ -7,6 +7,8 @@ import { createHost } from '../src/plugins'; import { guardPlugin, timePlugin } from '../src/plugins-builtin'; import { Session } from '../src/session'; import { loadSkills } from '../src/skills'; +import { MUTATING_TOOLS, TOOL_SETS, TOOL_SET_NAMES, isToolSetName, toolSetOf } from '../src/tools'; +import { GIT_TOOL_NAMES } from '../src/tools-git'; const usage = { inputTokens: { total: 5, noCache: 5, cacheRead: 0, cacheWrite: 0 }, @@ -226,3 +228,71 @@ test('afterTurn fires once the turn ends', async () => { for await (const _ of session.send('hi')) void _; expect(fired).toBe(1); }); + +test('the git tools are offered by default and never prompt', async () => { + const { seen, model } = recorder(); + const session = new Session({ model, askApproval: async () => 'deny' }); + for await (const _ of session.send('what changed')) void _; + + const offered = (seen[0]?.tools ?? []).map((t) => t.name); + for (const name of GIT_TOOL_NAMES) expect(offered).toContain(name); + for (const name of GIT_TOOL_NAMES) expect(MUTATING_TOOLS as readonly string[]).not.toContain(name); +}); + +test('a disabled tool set reaches neither the wire nor the prompt', async () => { + const { seen, model } = recorder(); + const session = new Session({ model, askApproval: async () => 'deny', toolSets: [] }); + for await (const _ of session.send('hi')) void _; + + const offered = (seen[0]?.tools ?? []).map((t) => t.name); + expect(offered).toContain('read_file'); + expect(offered).toContain('bash'); + expect(offered).not.toContain('git_status'); + expect(offered).not.toContain('multi_edit'); + expect(offered).not.toContain('list_dir'); + + const system = JSON.stringify(seen[0]?.prompt.find((m) => m.role === 'system')); + expect(system).not.toContain('git_status'); + expect(system).not.toContain('multi_edit'); +}); + +test('an enabled set is offered while the others stay withheld', async () => { + const { seen, model } = recorder(); + const session = new Session({ model, askApproval: async () => 'deny', toolSets: ['git'] }); + for await (const _ of session.send('hi')) void _; + + const offered = (seen[0]?.tools ?? []).map((t) => t.name); + expect(offered).toContain('git_diff'); + expect(offered).not.toContain('multi_edit'); +}); + +test('core is never withheld, whatever the config says', async () => { + const { seen, model } = recorder(); + const session = new Session({ model, askApproval: async () => 'deny', toolSets: ['git'] }); + for await (const _ of session.send('hi')) void _; + + const offered = (seen[0]?.tools ?? []).map((t) => t.name); + for (const name of TOOL_SETS.core) expect(offered).toContain(name); +}); + +test('session tools survive tool-set gating, since they are not part of that budget', async () => { + const { seen, model } = recorder(); + const session = new Session({ model, askApproval: async () => 'deny', toolSets: [], memory: new Memory('/repo-test') }); + for await (const _ of session.send('hi')) void _; + + const offered = (seen[0]?.tools ?? []).map((t) => t.name); + expect(offered).toContain('todo_write'); + expect(offered).toContain('remember'); +}); + +test('toolSetOf names the set a tool came from, and nothing for a session tool', () => { + expect(toolSetOf('read_file')).toBe('core'); + expect(toolSetOf('multi_edit')).toBe('edit-plus'); + expect(toolSetOf('git_log')).toBe('git'); + expect(toolSetOf('todo_write')).toBeUndefined(); +}); + +test('isToolSetName accepts the real sets only, so a typo in config is ignored', () => { + for (const name of TOOL_SET_NAMES) expect(isToolSetName(name)).toBe(true); + expect(isToolSetName('gti')).toBe(false); +}); diff --git a/test/session.test.ts b/test/session.test.ts index ad85277..a0eb036 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -7,6 +7,7 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { Session } from '../src/session'; +import { interruptBash } from '../src/tools'; const usage = { inputTokens: { total: 10, noCache: 10, cacheRead: 0, cacheWrite: 0 }, @@ -62,7 +63,7 @@ test('read-only tool runs without approval and the loop terminates', async () => const kinds: string[] = []; for await (const ev of session.send('read note.txt')) kinds.push(ev.type); - expect(kinds).toEqual(['tool-call', 'tool-result', 'text', 'done']); + expect(kinds).toEqual(['tool-start', 'tool-call', 'tool-result', 'text', 'done']); expect(call).toBe(2); })); @@ -264,7 +265,7 @@ test('a read-only built-in stays free even when mcp tools are present', async () const kinds: string[] = []; for await (const ev of session.send('read note.txt')) kinds.push(ev.type); - expect(kinds).toEqual(['tool-call', 'tool-result', 'text', 'done']); + expect(kinds).toEqual(['tool-start', 'tool-call', 'tool-result', 'text', 'done']); })); test('setModel swaps the model used by the next turn', async () => { @@ -302,8 +303,7 @@ test('reset clears history and token counters; replace swaps history in', async expect(session.messages).toEqual([{ role: 'user', content: 'restored' }]); }); -test('abort mid-stream ends the turn with done, keeping the text already delivered', async () => { - const session = new Session({ +test('abort mid-stream ends the turn with done, keeping the text already delivered', async () => { const session = new Session({ model: new MockLanguageModelV4({ doStream: async () => ({ stream: simulateReadableStream({ @@ -338,3 +338,40 @@ test('abort mid-stream ends the turn with done, keeping the text already deliver expect(kinds.at(-1)).toBe('done'); expect(kinds).not.toContain('error'); }, 15_000); + +test('an interrupted command becomes a tool error and the turn carries on', async () => + inTempDir(async () => { + const sleeper = process.platform === 'win32' ? 'ping -n 20 127.0.0.1 > nul' : 'sleep 20'; + let call = 0; + const session = new Session({ + yolo: true, + model: new MockLanguageModelV4({ + doStream: async () => + stream(call++ === 0 ? toolCall('c1', 'bash', { command: sleeper }) : text('I stopped there.')), + }), + askApproval: async () => { + throw new Error('yolo must not ask'); + }, + }); + + const kinds: string[] = []; + let toolError = ''; + const turn = (async () => { + for await (const ev of session.send('run the long thing')) { + kinds.push(ev.type); + if (ev.type === 'tool-error') toolError = String((ev.error as Error).message ?? ev.error); + } + })(); + + // Interrupt once the command is actually running. + await Bun.sleep(700); + expect(interruptBash()).toEqual([sleeper]); + await turn; + + expect(kinds).toContain('tool-error'); + expect(toolError).toMatch(/user interrupted this command/i); + // The turn survived: the model was asked again and its reply arrived. + expect(kinds).toContain('text'); + expect(kinds.at(-1)).toBe('done'); + expect(call).toBe(2); + }), 30_000); diff --git a/test/subagent.test.ts b/test/subagent.test.ts index 27ac9f0..e6b8998 100644 --- a/test/subagent.test.ts +++ b/test/subagent.test.ts @@ -73,7 +73,7 @@ test('subagent greps the workspace and returns text to the parent', async () => const events: string[] = []; for await (const ev of session.send('where is login defined?')) events.push(ev.type); - expect(events).toEqual(['tool-call', 'tool-result', 'text', 'done']); + expect(events).toEqual(['tool-start', 'tool-call', 'tool-result', 'text', 'done']); // The subagent gets only read tools, so it can never trigger an approval prompt. const subagentTools = (seen[1]?.tools ?? []).map((t) => t.name).sort(); diff --git a/test/tools-git.test.ts b/test/tools-git.test.ts new file mode 100644 index 0000000..3cac7da --- /dev/null +++ b/test/tools-git.test.ts @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + GIT_TOOL_NAMES, + gitBlameTool, + gitDiffTool, + gitLogTool, + gitShowTool, + gitStatusTool, + gitTools, +} from '../src/tools-git'; + +let dir: string; +let origCwd: string; + +const run = (t: { execute?: (input: T, opts: any) => unknown }, input: T) => + Promise.resolve(t.execute!(input, { toolCallId: 't1', messages: [] })) as Promise; + +async function git(...args: string[]): Promise { + const proc = Bun.spawn(['git', ...args], { cwd: dir, stdout: 'pipe', stderr: 'pipe' }); + const code = await proc.exited; + if (code !== 0) throw new Error(`git ${args.join(' ')} failed: ${await new Response(proc.stderr).text()}`); +} + +beforeEach(() => { + origCwd = process.cwd(); + dir = mkdtempSync(join(tmpdir(), 'shiro-git-')); + process.chdir(dir); +}); + +afterEach(() => { + process.chdir(origCwd); + rmSync(dir, { recursive: true, force: true }); +}); + +async function repoWithOneCommit(): Promise { + await git('init', '-b', 'main'); + await git('config', 'user.email', 'test@example.com'); + await git('config', 'user.name', 'Test'); + await Bun.write(join(dir, 'app.ts'), 'export const port = 8080;\n'); + await git('add', '.'); + await git('commit', '-m', 'add the server port'); +} + +test('every git tool is registered and named consistently', () => { + expect(GIT_TOOL_NAMES.sort()).toEqual(['git_blame', 'git_diff', 'git_log', 'git_show', 'git_status']); + expect(Object.keys(gitTools).sort()).toEqual(GIT_TOOL_NAMES.sort()); +}); + +test('git_status names the branch and describes each change', async () => { + await repoWithOneCommit(); + expect(await run(gitStatusTool, {})).toContain('working tree clean'); + + await Bun.write(join(dir, 'app.ts'), 'export const port = 9090;\n'); + await Bun.write(join(dir, 'new.ts'), 'x\n'); + + const out = await run(gitStatusTool, {}); + expect(out).toContain('On main'); + expect(out).toContain('app.ts'); + expect(out).toContain('modified'); + expect(out).toContain('new.ts'); + expect(out).toContain('untracked'); +}); + +test('git_status separates staged from unstaged', async () => { + await repoWithOneCommit(); + await Bun.write(join(dir, 'app.ts'), 'export const port = 9090;\n'); + await git('add', 'app.ts'); + + expect(await run(gitStatusTool, {})).toContain('staged modified'); +}); + +test('git_diff shows the working tree, and staged on request', async () => { + await repoWithOneCommit(); + expect(await run(gitDiffTool, {})).toBe('No uncommitted changes.'); + + await Bun.write(join(dir, 'app.ts'), 'export const port = 9090;\n'); + const unstaged = await run(gitDiffTool, {}); + expect(unstaged).toContain('-export const port = 8080;'); + expect(unstaged).toContain('+export const port = 9090;'); + + expect(await run(gitDiffTool, { staged: true })).toBe('Nothing staged.'); + await git('add', 'app.ts'); + expect(await run(gitDiffTool, { staged: true })).toContain('9090'); +}); + +test('git_diff narrows to a path', async () => { + await repoWithOneCommit(); + await Bun.write(join(dir, 'app.ts'), 'changed\n'); + await Bun.write(join(dir, 'other.ts'), 'also changed\n'); + await git('add', 'other.ts'); + await git('commit', '-m', 'add other'); + await Bun.write(join(dir, 'other.ts'), 'changed again\n'); + + const out = await run(gitDiffTool, { path: 'app.ts' }); + expect(out).toContain('app.ts'); + expect(out).not.toContain('other.ts'); +}); + +test('git_log lists commits newest first and honours the limit', async () => { + await repoWithOneCommit(); + await Bun.write(join(dir, 'app.ts'), 'export const port = 9090;\n'); + await git('commit', '-am', 'bump the port'); + + const out = await run(gitLogTool, {}); + expect(out.split('\n')[0]).toContain('bump the port'); + expect(out).toContain('add the server port'); + expect(out).toContain('Test'); + + expect((await run(gitLogTool, { limit: 1 })).split('\n')).toHaveLength(1); +}); + +test('git_show renders one commit with its diff', async () => { + await repoWithOneCommit(); + const out = await run(gitShowTool, { ref: 'HEAD' }); + expect(out).toContain('add the server port'); + expect(out).toContain('+export const port = 8080;'); +}); + +test('git_show reports a bad ref rather than returning nothing', async () => { + await repoWithOneCommit(); + expect(run(gitShowTool, { ref: 'no-such-ref' })).rejects.toThrow(); +}); + +test('git_blame attributes each line and narrows by range', async () => { + await repoWithOneCommit(); + const out = await run(gitBlameTool, { path: 'app.ts' }); + expect(out).toContain('Test'); + expect(out).toContain('export const port = 8080;'); + + expect(await run(gitBlameTool, { path: 'app.ts', startLine: 1, endLine: 1 })).toContain('8080'); +}); + +test('outside a repository every tool fails with a clear message, not git porcelain', async () => { + for (const [name, t] of Object.entries(gitTools)) { + const input = + name === 'git_show' ? { ref: 'HEAD' } : name === 'git_blame' ? { path: 'nothing.ts' } : ({} as never); + expect(run(t as never, input as never), name).rejects.toThrow(/not a git repository/i); + } +}); + +test('an argument that looks like a shell injection is passed through as one argument', async () => { + await repoWithOneCommit(); + // argv spawning, not a shell string, so this can only ever be a pathspec. + const out = await run(gitLogTool, { path: '; touch pwned.txt' }).catch((e: Error) => e.message); + expect(await Bun.file(join(dir, 'pwned.txt')).exists()).toBe(false); + expect(out).toBeTruthy(); +}); diff --git a/test/tools.test.ts b/test/tools.test.ts index 3e11780..dd5dcdf 100644 --- a/test/tools.test.ts +++ b/test/tools.test.ts @@ -7,9 +7,13 @@ import { editFileTool, globTool, grepTool, + interruptBash, jail, + listDirTool, + multiEditTool, onBashOutput, readFileTool, + readManyFilesTool, writeFileTool, } from '../src/tools'; @@ -58,6 +62,52 @@ test('read_file still accepts UTF-8 with high codepoints', async () => { expect(await run(readFileTool, { path: 'u.txt' })).toContain('hello -> world'); }); +test('read_many_files returns one labelled block per file', async () => { + await Bun.write(join(dir, 'a.ts'), 'const a = 1;\n'); + await Bun.write(join(dir, 'b.ts'), 'const b = 2;\n'); + + const out = await run(readManyFilesTool, { files: [{ path: 'a.ts' }, { path: 'b.ts' }] }); + expect(out).toContain('===== a.ts ====='); + expect(out).toContain('1: const a = 1;'); + expect(out).toContain('===== b.ts ====='); + expect(out).toContain('1: const b = 2;'); +}); + +test('read_many_files keeps the order it was given', async () => { + await Bun.write(join(dir, 'first.ts'), 'x\n'); + await Bun.write(join(dir, 'second.ts'), 'y\n'); + + const out = await run(readManyFilesTool, { files: [{ path: 'second.ts' }, { path: 'first.ts' }] }); + expect(out.indexOf('second.ts')).toBeLessThan(out.indexOf('first.ts')); +}); + +test('read_many_files honours a per-file offset and limit', async () => { + await Bun.write(join(dir, 'long.ts'), 'one\ntwo\nthree\nfour\n'); + const out = await run(readManyFilesTool, { files: [{ path: 'long.ts', offset: 2, limit: 2 }] }); + expect(out).toContain('2: two'); + expect(out).toContain('3: three'); + expect(out).not.toContain('1: one'); + expect(out).not.toContain('4: four'); +}); + +test('an unreadable path is named in its own block without aborting the rest', async () => { + await Bun.write(join(dir, 'good.ts'), 'fine\n'); + await Bun.write(join(dir, 'blob.bin'), new Uint8Array([0x00, 0x01, 0x02])); + + const out = await run(readManyFilesTool, { + files: [{ path: 'good.ts' }, { path: 'gone.ts' }, { path: 'blob.bin' }], + }); + + expect(out).toContain('1: fine'); + expect(out).toContain('===== gone.ts ====='); + expect(out).toContain('No such file: gone.ts'); + expect(out).toContain('binary file'); +}); + +test('read_many_files refuses a path outside the workspace', async () => { + expect(run(readManyFilesTool, { files: [{ path: '../escape.ts' }] })).resolves.toContain('escapes workspace'); +}); + test('edit_file replaces a unique occurrence', async () => { await Bun.write(join(dir, 'x.ts'), 'const a = 1;\nconst b = 2;\n'); await run(editFileTool, { path: 'x.ts', oldString: 'const b = 2;', newString: 'const b = 3;' }); @@ -85,6 +135,100 @@ test('write_file then glob and grep find the content', async () => { ); }); +test('multi_edit applies every edit in order, each seeing the last', async () => { + await Bun.write(join(dir, 'm.ts'), 'const a = 1;\nconst b = 2;\n'); + const out = await run(multiEditTool, { + path: 'm.ts', + edits: [ + { oldString: 'const a = 1;', newString: 'const a = 10;' }, + { oldString: 'const a = 10;\nconst b = 2;', newString: 'const a = 10;\nconst b = 20;' }, + ], + }); + + expect(out).toContain('2 edit(s)'); + expect(await Bun.file(join(dir, 'm.ts')).text()).toBe('const a = 10;\nconst b = 20;\n'); +}); + +test('a failing second edit leaves the file exactly as it was', async () => { + const before = 'const a = 1;\nconst b = 2;\n'; + await Bun.write(join(dir, 'm.ts'), before); + + expect( + run(multiEditTool, { + path: 'm.ts', + edits: [ + { oldString: 'const a = 1;', newString: 'const a = 10;' }, + { oldString: 'const NOPE = 0;', newString: 'x' }, + ], + }), + ).rejects.toThrow(/edit 2: oldString not found/); + + await Bun.sleep(20); + expect(await Bun.file(join(dir, 'm.ts')).text()).toBe(before); +}); + +test('multi_edit refuses an ambiguous match unless replaceAll, writing nothing', async () => { + const before = 'x\nx\n'; + await Bun.write(join(dir, 'a.ts'), before); + + expect( + run(multiEditTool, { path: 'a.ts', edits: [{ oldString: 'x', newString: 'y' }] }), + ).rejects.toThrow(/appears 2 times/); + await Bun.sleep(20); + expect(await Bun.file(join(dir, 'a.ts')).text()).toBe(before); + + await run(multiEditTool, { path: 'a.ts', edits: [{ oldString: 'x', newString: 'y', replaceAll: true }] }); + expect(await Bun.file(join(dir, 'a.ts')).text()).toBe('y\ny\n'); +}); + +test('multi_edit reports a missing file rather than creating one', async () => { + expect( + run(multiEditTool, { path: 'gone.ts', edits: [{ oldString: 'a', newString: 'b' }] }), + ).rejects.toThrow(/No such file/); + expect(await Bun.file(join(dir, 'gone.ts')).exists()).toBe(false); +}); + +test('list_dir shows a tree with sizes and marks directories', async () => { + await Bun.write(join(dir, 'src/app.ts'), 'x'.repeat(2048)); + await Bun.write(join(dir, 'readme.md'), 'hi'); + + const out = await run(listDirTool, {}); + expect(out).toContain('src/'); + expect(out).toContain('app.ts'); + expect(out).toContain('2K'); + expect(out).toContain('readme.md 2B'); +}); + +test('list_dir stops at the depth limit, still naming the directory', async () => { + await Bun.write(join(dir, 'a/b/c/deep.ts'), 'x'); + + const shallow = await run(listDirTool, { depth: 1 }); + expect(shallow).toContain('a/'); + expect(shallow).not.toContain('deep.ts'); + + expect(await run(listDirTool, { depth: 4 })).toContain('deep.ts'); +}); + +test('list_dir honours .gitignore and includeIgnored', async () => { + await Bun.write(join(dir, '.gitignore'), 'dist/\n'); + await Bun.write(join(dir, 'dist/bundle.js'), 'x'); + await Bun.write(join(dir, 'src/app.ts'), 'x'); + + expect(await run(listDirTool, {})).not.toContain('bundle.js'); + expect(await run(listDirTool, { includeIgnored: true })).toContain('bundle.js'); +}); + +test('list_dir scopes to a subdirectory and refuses a file', async () => { + await Bun.write(join(dir, 'src/app.ts'), 'x'); + await Bun.write(join(dir, 'other.ts'), 'x'); + + const out = await run(listDirTool, { path: 'src' }); + expect(out).toContain('app.ts'); + expect(out).not.toContain('other.ts'); + + expect(run(listDirTool, { path: 'other.ts' })).rejects.toThrow(/Not a directory/); +}); + 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'); @@ -170,3 +314,40 @@ test('the bash listener is cleared when unset', async () => { await run(bashTool, { command: 'echo two' }); expect(chunks.length).toBe(afterFirst); }, 20_000); + +const sleeper = process.platform === 'win32' ? 'ping -n 20 127.0.0.1 > nul' : 'sleep 20'; + +test('interruptBash kills the command in flight and names it', async () => { + const started = Date.now(); + const call = run(bashTool, { command: sleeper, timeout: 30_000 }); + + await Bun.sleep(400); + expect(interruptBash()).toEqual([sleeper]); + + expect(call).rejects.toThrow(/user interrupted this command/i); + await call.catch(() => {}); + // Killed, not waited out: the 20s command must not have run to completion. + expect(Date.now() - started).toBeLessThan(10_000); +}, 30_000); + +test('the interrupt error carries whatever the command printed first', async () => { + const script = + process.platform === 'win32' ? 'echo before && ping -n 20 127.0.0.1 > nul' : 'echo before; sleep 20'; + const call = run(bashTool, { command: script, timeout: 30_000 }); + + await Bun.sleep(600); + interruptBash(); + + const message = await call.then(() => '', (e: Error) => e.message); + expect(message).toContain('before'); + expect(message).toContain('effects are unknown'); +}, 30_000); + +test('interruptBash with nothing running is a no-op', () => { + expect(interruptBash()).toEqual([]); +}); + +test('a command that finished is no longer interruptible', async () => { + await run(bashTool, { command: 'echo done' }); + expect(interruptBash()).toEqual([]); +}, 20_000); diff --git a/test/ui-panels.test.tsx b/test/ui-panels.test.tsx index 90eb634..c522990 100644 --- a/test/ui-panels.test.tsx +++ b/test/ui-panels.test.tsx @@ -5,7 +5,7 @@ import { createAskTool } from '../src/ask'; import { applySubagentEvent } from '../src/ui/App'; import { AskPanel, createAskBridge } from '../src/ui/Ask'; import { Markdown } from '../src/ui/Markdown'; -import { SubagentPanel, TodoPanel, StatusBar, InfoPanel, type SubagentView } from '../src/ui/Panels'; +import { SubagentPanel, TodoPanel, StatusBar, InfoPanel, ActiveTool, QueuePanel, ThinkingPanel, type SubagentView } from '../src/ui/Panels'; import type { SubagentEvent } from '../src/subagent'; const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); @@ -116,6 +116,55 @@ test('the info panel renders a markdown body', () => { app.unmount(); }); +test('the active tool line names the tool and the file it is touching', () => { + const app = render(); + const frame = app.lastFrame() ?? ''; + expect(frame).toContain('read_file'); + expect(frame).toContain('src/session.ts'); + app.unmount(); +}); + +test('the active tool line renders before the arguments have arrived', () => { + const app = render(); + expect(app.lastFrame()).toContain('grep'); + app.unmount(); +}); + +test('thinking collapses to a token count, and expands on request', () => { + const text = 'x'.repeat(1648); + const collapsed = render(); + expect(collapsed.lastFrame()).toContain('~412 tokens'); + expect(collapsed.lastFrame()).not.toContain('xxxx'); + collapsed.unmount(); + + const open = render(); + const frame = open.lastFrame() ?? ''; + expect(frame).toContain('second line'); + expect(frame).toContain('collapse'); + open.unmount(); +}); + +test('no reasoning renders nothing at all', () => { + const app = render(); + expect(app.lastFrame() ?? '').toBe(''); + app.unmount(); +}); + +test('the queue panel counts what is waiting and lists it in order', () => { + const app = render(); + const frame = app.lastFrame() ?? ''; + expect(frame).toContain('queued: 2'); + expect(frame).toContain('1. first thing'); + expect(frame).toContain('2. second thing'); + app.unmount(); +}); + +test('an empty queue renders nothing', () => { + const app = render(); + expect(app.lastFrame() ?? '').toBe(''); + app.unmount(); +}); + const events: SubagentEvent[] = [ { type: 'start', id: 'a', kind: 'explore', description: 'first task' }, { type: 'step', id: 'a', tool: 'grep', summary: 'needle' },