- Diagnostics from LSP/linters immediately after edits - Prompt history with ↑↓ navigation in .codey/history - Claude Code's skills discovery and integration - Lifecycle hook system for external tools - Herdr workspace manager state reporting - Background process and working plan tools
Showing 86 changed files with 15450 additions and 206 deletions.
| @@ -54,7 +54,8 @@ would break your Claude Code login. | ||
| 54 | 54 | |
| 55 | 55 | Useful flags: `-model`, `-effort low|medium|high|xhigh|max`, `-max-turns`, |
| 56 | 56 | `-resume latest`, `-no-stream`, `-no-cache`, `-no-auto-compact`, `-v`, `-yolo`, |
| 57 | - `-no-tui`, `-no-skills`, `-no-agents`, `-no-web`, `-no-claude-code-auth`. | |
| 57 | + `-no-tui`, `-no-skills`, `-no-agents`, `-no-web`, `-no-hooks`, | |
| 58 | + `-no-diagnostics`, `-no-claude-code-auth`. | |
| 58 | 59 | |
| 59 | 60 | ### Flags and thresholds |
| 60 | 61 | |
| @@ -108,14 +109,18 @@ root: | ||
| 108 | 109 | ▄██████▄ codey v0.6.0 |
| 109 | 110 | █ ██ █ Opus 5 · anthropic |
| 110 | 111 | ███▀▀███ ~/Work/GO/coding-harness |
| 111 | - ▀█▀▀█▀ | |
| 112 | + ▀█▀▀█▀ /help · ↑ earlier prompts · ! shell · shift+tab approvals | |
| 112 | 113 | ``` |
| 113 | 114 | |
| 114 | 115 | The gopher is drawn in half-blocks, so it needs no patched font, and it is |
| 115 | 116 | dropped rather than sliced on a terminal too narrow to hold it beside a |
| 116 | 117 | readable amount of text: half a mascot reads as a rendering bug, where the |
| 117 | - three lines alone read as a choice. From there it settles into a transcript | |
| 118 | - above a prompt: | |
| 118 | + three lines alone read as a choice. The fourth line is the handful of things | |
| 119 | + there is no other way to find out, and it costs no height at all: the gopher is | |
| 120 | + four rows tall and the text was three. It is dropped whole rather than | |
| 121 | + truncated when the terminal is too narrow for it, because half a sentence of | |
| 122 | + hints is worse than none. From there it settles into a transcript above a | |
| 123 | + prompt: | |
| 119 | 124 | |
| 120 | 125 | ``` |
| 121 | 126 | ⏺ Let me see what the test expects before changing anything. |
| @@ -439,6 +444,34 @@ level for `/effort` — and `esc` dismisses the menu without touching what you | ||
| 439 | 444 | typed. The menu offers exactly the commands the TUI answers, so nothing it |
| 440 | 445 | suggests can come back as "unknown command". |
| 441 | 446 | |
| 447 | + ### Earlier prompts | |
| 448 | + | |
| 449 | + `↑` from the first line of the prompt box walks back through what has been | |
| 450 | + typed into this checkout before, `↓` walks forward again, and `esc` abandons | |
| 451 | + the walk. The empty box says so — its placeholder names the key, and sheds | |
| 452 | + clauses rather than characters as the terminal narrows, since a hint truncated | |
| 453 | + to `! shell · ↑ earli` advertises nothing while looking like a bug. `ctrl+p` and `ctrl+n` do the same, because they already do in every | |
| 454 | + other list Codey draws. | |
| 455 | + | |
| 456 | + The rule about the first line is what lets one key do two jobs without a mode: | |
| 457 | + anywhere below it, `↑` is moving the cursor through a paragraph you are still | |
| 458 | + writing, which is what it looks like it is doing. Walking all the way forward | |
| 459 | + again gives back the half-written prompt the walk started from — losing that is | |
| 460 | + the one thing a history feature can do that is worse than not existing — and | |
| 461 | + editing a recalled line ends the walk, because the line is yours now. | |
| 462 | + | |
| 463 | + A `!` or `$` line and a prompt are different kinds of thing, so they are | |
| 464 | + offered back separately: `↑` at the shell prompt walks shell commands, and the | |
| 465 | + prompt aimed at the model never hands you a `go test` to send as prose. | |
| 466 | + | |
| 467 | + The last few hundred entries are kept in `.codey/history` inside the checkout | |
| 468 | + Codey was started in — not per session, which would forget the thing you are | |
| 469 | + most likely to want back, and not one file per machine, which would offer you | |
| 470 | + the test command of a project you are not in. Two tabs append to it rather than | |
| 471 | + rewriting it, so neither loses the other's lines. It is inside the state | |
| 472 | + directory, which `.gitignore` already covers; delete the file and the prompt | |
| 473 | + box simply stops remembering. | |
| 474 | + | |
| 442 | 475 | ### Running something yourself |
| 443 | 476 | |
| 444 | 477 | `!` on an empty prompt points the box at a shell instead of at the model; `$` |
| @@ -717,7 +750,10 @@ agents at once. `-p` and `-no-tui` runs are not offered the tools at all, and | ||
| 717 | 750 | ## Setup |
| 718 | 751 | |
| 719 | 752 | The first run on a terminal opens a full-screen wizard instead of the chat: the |
| 720 | - mark assembles, the name lights up, and it asks five questions one at a time. | |
| 753 | + mark assembles, the name lights up, and it asks five questions one at a time, | |
| 754 | + plus the two that are only sometimes there — the Claude skills question on a | |
| 755 | + machine that has some, and the herdr one on a machine that has | |
| 756 | + [herdr](#herdr). | |
| 721 | 757 | "First run" means the config file does not say `"setup": "done"` — the marker |
| 722 | 758 | the wizard stamps when the last question is answered — rather than that no file |
| 723 | 759 | exists, so a config written by hand, or by a build with fewer questions in it, |
| @@ -746,6 +782,20 @@ still gets asked the ones it is missing. | ||
| 746 | 782 | | Approvals | ask before every mutating call, or accept them all | |
| 747 | 783 | | Web search | `none`, Claude's hosted search, or a command on this machine | |
| 748 | 784 | | Theme | `ember` `mint` `iris` `mono` | |
| 785 | + | Claude skills | leave them alone, use them in place, or copy them once — asked only where there are some | | |
| 786 | + | herdr skill | install it, or not — asked only where herdr is | | |
| 787 | + | |
| 788 | + The last two are the steps that are sometimes not there. The rail knows how | |
| 789 | + many questions this run is actually asking rather than how many the build has, | |
| 790 | + so a machine with neither gets a five-node rail with no gaps in it, and | |
| 791 | + `step 6 of 7` on a machine with both counts the questions you are being asked. | |
| 792 | + The herdr answer is also the one that is not a preference: it is an act, | |
| 793 | + carried out once, so nothing about it is written to the config. It appears when | |
| 794 | + `HERDR_ENV=1` or `herdr` is on `PATH`. What it installs, and what happens when | |
| 795 | + the download fails, is [below](#the-herdr-skill). The Claude skills question is | |
| 796 | + described under [skills](#claude-codes-skills); "copy them once" is an act in | |
| 797 | + the same way, carried out as the wizard closes, and it is the mode rather than | |
| 798 | + the act that is saved. | |
| 749 | 799 | |
| 750 | 800 | It is the same catalogue and the same pricing table `/model` uses, through the |
| 751 | 801 | same `GET /v1/models` call, so the two cannot disagree about what a model is |
| @@ -770,6 +820,8 @@ honoured): | ||
| 770 | 820 | "approvals": "ask", |
| 771 | 821 | "web_search": "custom", |
| 772 | 822 | "search_command": "ddgs text -q {{query}} -m {{count}} -nc", |
| 823 | + "claude_skills": "use", | |
| 824 | + "diagnostics": "on", | |
| 773 | 825 | "theme": "ember", |
| 774 | 826 | "auto_compact": "on", |
| 775 | 827 | "setup": "done" |
| @@ -777,7 +829,11 @@ honoured): | ||
| 777 | 829 | ``` |
| 778 | 830 | |
| 779 | 831 | `auto_compact` is the one key neither the wizard nor `/settings` offers a row |
| 780 | - for — set it by hand, or pass `-no-auto-compact` for a single run. Both screens | |
| 832 | + for — set it by hand, or pass `-no-auto-compact` for a single run. | |
| 833 | + `diagnostics` is the one `/settings` has a row for and the wizard does not | |
| 834 | + ask about: it defaults to on, it runs a tool already installed on this machine, | |
| 835 | + and nothing it reads leaves the machine, which is the test every question in | |
| 836 | + that interview is really asking. Both screens | |
| 781 | 837 | write the whole file but only touch what they asked about, so it survives them. |
| 782 | 838 | Set to `"off"`, the conversation is never folded on its own: the window becomes |
| 783 | 839 | a wall rather than a trigger, and at `-compact-floor` the turn stops before |
| @@ -820,9 +876,10 @@ rather than the project's. | ||
| 820 | 876 | |
| 821 | 877 | | Path | What | |
| 822 | 878 | | --- | --- | |
| 823 | - | `~/.config/codey/config.json` | the answers, written by the wizard and `/settings` | | |
| 879 | + | `~/.config/codey/config.json` | the answers, written by the wizard and `/settings`; [hooks](#hooks) go here by hand | | |
| 824 | 880 | | `<repo>/.codey/sessions/` | saved conversations, which is what `-resume` and `/resume` read | |
| 825 | 881 | | `<repo>/.codey/events.jsonl` | the event log METRICS.md is built from | |
| 882 | + | `<repo>/.codey/history` | what has been typed at the prompt here, which `↑` walks back through | | |
| 826 | 883 | |
| 827 | 884 | Codey was called `harness` until recently, and both of those used to live under |
| 828 | 885 | that name. Neither rename costs you anything. The config is read from |
| @@ -837,15 +894,18 @@ both keeps the newer one and leaves `.harness/` alone. | ||
| 837 | 894 | |
| 838 | 895 | ### /settings |
| 839 | 896 | |
| 840 | - `/settings` is the same four settings without the interview: one row each, | |
| 841 | - changed in place, applied as you move. | |
| 897 | + `/settings` is the same settings without the interview: one row each, changed | |
| 898 | + in place, applied as you move. | |
| 842 | 899 | |
| 843 | 900 | ``` |
| 844 | 901 | Settings ↑↓ row · ←→ change · esc done |
| 845 | - ❯ Model Opus 5 $5 / $25 per Mtok | |
| 846 | - Effort high default | |
| 847 | - Approvals Ask me first recommended | |
| 848 | - Theme ember | |
| 902 | + ❯ Model Opus 5 $5 / $25 per Mtok | |
| 903 | + Effort high default | |
| 904 | + Approvals Ask me first recommended | |
| 905 | + Web search No web access default | |
| 906 | + Diagnostics On default | |
| 907 | + Claude skills Leave them alone default | |
| 908 | + Theme ember | |
| 849 | 909 | claude-opus-5 · enter to pick from anthropic |
| 850 | 910 | saved to ~/.config/codey/config.json |
| 851 | 911 | ``` |
| @@ -879,15 +939,65 @@ and any key skips the one playing. | ||
| 879 | 939 | |
| 880 | 940 | | Tool | Approval | Notes | |
| 881 | 941 | | --- | --- | --- | |
| 882 | - | `read_file` | automatic | Numbered lines, `offset`/`limit` for paging | | |
| 942 | + | `read_file` | automatic | Numbered lines, `offset`/`limit` for paging; a PNG, JPEG, GIF or WebP comes back as a picture the model can look at | | |
| 883 | 943 | | `glob` | automatic | Newest first; `*.go` matches at any depth | |
| 884 | 944 | | `grep` | automatic | ripgrep when present, pure Go otherwise, identical output either way | |
| 885 | - | `edit_file` | prompts | Requires a prior read; refuses ambiguous matches; returns a diff | | |
| 945 | + | `edit_file` | prompts | Requires a prior read; refuses ambiguous matches; returns a diff and any [diagnostics](#diagnostics) | | |
| 886 | 946 | | `write_file` | prompts | Whole files only; separate from `edit_file` so the metrics stay honest | |
| 887 | - | `bash` | prompts | Own process group, killed whole on timeout; output capped head and tail | | |
| 947 | + | `bash` | prompts | Own process group, killed whole on timeout; output capped head and tail. `background: true` returns a handle instead of waiting | | |
| 948 | + | `process_output` | never | New output from a background process since you last asked, and whether it is still running | | |
| 949 | + | `process_kill` | never | Stops a background process and its children; everything is killed when the session ends anyway | | |
| 950 | + | `todo_write` | never | The working plan for a multi-step task; the whole list is sent each time | | |
| 951 | + | `context_status` | never | How full the window is, how many turns and what they cost | | |
| 888 | 952 | | `ask_user` | never | Puts a multiple-choice question to you and waits — see [questions](#questions) | |
| 889 | 953 | | `web_search` | prompts | Only with `web_search: custom`; runs `ddgs` on this machine | |
| 890 | - | `web_fetch` | prompts | Only with `web_search: custom`; refuses private and loopback addresses | | |
| 954 | + | `web_fetch` | prompts | With either web search mode; refuses private and loopback addresses | | |
| 955 | + | |
| 956 | + ## Diagnostics | |
| 957 | + | |
| 958 | + The agent writes a file and finds out it broke the build three tool calls | |
| 959 | + later, if at all. The cheapest fix is to put the compiler's own opinion in the | |
| 960 | + result the model is already reading: | |
| 961 | + | |
| 962 | + ``` | |
| 963 | + ⏺ EditFile(path=internal/agent/loop.go) | |
| 964 | + ⎿ 4 additions, 1 removal | |
| 965 | + … | |
| 966 | + | |
| 967 | + diagnostics (gopls): 2 problems | |
| 968 | + loop.go:14:2: undefined: fmt.Printl | |
| 969 | + loop.go:19:6: declared and not used: n | |
| 970 | + ``` | |
| 971 | + | |
| 972 | + After a successful `edit_file` or `write_file`, the changed file goes to | |
| 973 | + whatever this machine already has for its language — `gopls`, | |
| 974 | + `pyright-langserver`, `typescript-language-server` or `rust-analyzer` if one is | |
| 975 | + on `PATH`, and otherwise the compiler or linter: `go build` and `go vet`, `ruff` | |
| 976 | + or `pyflakes`, `eslint` or `tsc`, `shellcheck`. Nothing is installed and | |
| 977 | + nothing leaves the machine. | |
| 978 | + | |
| 979 | + **A clean file adds nothing at all.** Silence is the good news, and a "0 | |
| 980 | + problems" line on every edit would be a paragraph of tokens per edit saying | |
| 981 | + that nothing happened. Ten problems are reported and the rest counted, and a | |
| 982 | + diagnostic about a *neighbouring* file — the helper you have just renamed out | |
| 983 | + from under — is kept and labelled, because that is the one the agent has no | |
| 984 | + other way to learn about. | |
| 985 | + | |
| 986 | + Language servers are started once and kept warm between edits, keyed by the | |
| 987 | + workspace around the file rather than by the working root, so `/worktree` does | |
| 988 | + not strand a warm `gopls` behind it. A check that has not finished in two | |
| 989 | + seconds is abandoned rather than waited for: a tool call that hangs because a | |
| 990 | + language server is confused is far worse than one that reports nothing. Every | |
| 991 | + other way this can go wrong lands in the same place — a server that will not | |
| 992 | + start, that dies mid-session, that answers with garbage, or one on `PATH` that | |
| 993 | + turns out to be a broken shim — and the answer is always no diagnostics and a | |
| 994 | + fall through to the linter, never a hung tool call. A server that fails to | |
| 995 | + start is put on a five-minute cooldown rather than retried on every edit. | |
| 996 | + | |
| 997 | + It is on by default, which is the one setting here that leans that way: it runs | |
| 998 | + a tool you already have, against a file you already told the agent to write, | |
| 999 | + and nothing it reads goes anywhere. `"diagnostics": "off"` in the config, the | |
| 1000 | + `/settings` row, or `-no-diagnostics` for a single run turns it off. | |
| 891 | 1001 | |
| 892 | 1002 | ## Skills |
| 893 | 1003 | |
| @@ -916,7 +1026,22 @@ as long as you like. | ||
| 916 | 1026 | Two ways in, one way to load. The agent calls the `skill` tool when a |
| 917 | 1027 | description matches what it is doing; you can force one with `/commit-helper |
| 918 | 1028 | fix the flaky test`, which the model answers by making the same call. `/skills` |
| 919 | - lists what was found, project shadowing personal on a name collision. | |
| 1029 | + lists what was found, project shadowing personal on a name collision, grouped | |
| 1030 | + by where each one came from: | |
| 1031 | + | |
| 1032 | + ``` | |
| 1033 | + skills: 3 | |
| 1034 | + personal | |
| 1035 | + /commit-helper Use when writing commit messages | |
| 1036 | + claude | |
| 1037 | + /pdf Extract text and tables from PDFs | |
| 1038 | + claude:superpowers | |
| 1039 | + /brainstorming Use when exploring an idea before writing a plan | |
| 1040 | + ``` | |
| 1041 | + | |
| 1042 | + One line each, truncated to the width. A description written for another | |
| 1043 | + harness can be three sentences long, and this list is for recognising a skill | |
| 1044 | + you already know about rather than for reading it. | |
| 920 | 1045 | |
| 921 | 1046 | Skill directories are readable by `read_file`, `glob` and `grep` even though |
| 922 | 1047 | they sit outside the working root, so a skill can ship reference files and |
| @@ -936,6 +1061,37 @@ miss on every turn for a file that never changes. Restart to pick up a new | ||
| 936 | 1061 | skill. `-no-skills` turns the whole thing off; the eval runner never discovers |
| 937 | 1062 | skills at all, so METRICS.md is unaffected by what is in your home directory. |
| 938 | 1063 | |
| 1064 | + ### Claude Code's skills | |
| 1065 | + | |
| 1066 | + Codey can also use the skills Claude Code already has on this machine — the | |
| 1067 | + ones under `~/.claude/skills`, the ones its installed plugins ship, and a | |
| 1068 | + project's `.claude/skills`. The setup wizard asks about them, and only when | |
| 1069 | + there are some to offer, because a question about nothing is a question worth | |
| 1070 | + not asking: | |
| 1071 | + | |
| 1072 | + | `claude_skills` | What happens | | |
| 1073 | + | --- | --- | | |
| 1074 | + | `off` | they are left alone; Codey reads only its own roots. This is the default | | |
| 1075 | + | `use` | they are read where they lie, on every start, so updating a plugin in Claude Code updates the skill here with nothing to re-run | | |
| 1076 | + | `copy` | they are copied once into `~/.agents/skills`, where they are yours to edit and unaffected by the next plugin update | | |
| 1077 | + | |
| 1078 | + Off is the default for the same reason `web_search` defaults to none: a skill | |
| 1079 | + is a page of instructions the model will follow, and it should be loaded | |
| 1080 | + because you said so rather than because a directory happened to exist in your | |
| 1081 | + home directory. | |
| 1082 | + | |
| 1083 | + Plugins come from `~/.claude/plugins/installed_plugins.json`, newest install | |
| 1084 | + per plugin, so the stale versions left in the cache are not loaded three times | |
| 1085 | + over. A skill of your own in `~/.agents/skills` wins when it shares a name with | |
| 1086 | + one of Claude's — what you put there is a decision, what arrived with a plugin | |
| 1087 | + is not — and `copy` never overwrites a skill already there. Frontmatter Codey | |
| 1088 | + would otherwise reject is accommodated rather than skipped: a name with | |
| 1089 | + capitals, underscores or spaces in it, or one that disagrees with its own | |
| 1090 | + directory, is normalised into a slug instead of becoming a load error, because | |
| 1091 | + these files were written for another harness and are not yours to fix. | |
| 1092 | + `/skills` lists each one with where it came from — `claude`, or | |
| 1093 | + `claude:superpowers` for a plugin's. | |
| 1094 | + | |
| 939 | 1095 | ## Web search |
| 940 | 1096 | |
| 941 | 1097 | The agent's knowledge of a library ends where its training did, and the two |
| @@ -999,6 +1155,153 @@ set is for. The query and the URL are the detail on the prompt, so what is | ||
| 999 | 1155 | leaving is on screen before it leaves. "Always this tool" is the answer most |
| 1000 | 1156 | people give once. |
| 1001 | 1157 | |
| 1158 | + ## Hooks | |
| 1159 | + | |
| 1160 | + Codey publishes its lifecycle — a session opened, a run started, a tool ran, an | |
| 1161 | + agent stopped to ask you something — so that things outside the process can | |
| 1162 | + watch it. That is a different job from the event stream the TUI draws from: | |
| 1163 | + this one carries no conversation content, no text deltas and no tool output, | |
| 1164 | + because its audience is a sidebar, a notifier, or a script, and what the model | |
| 1165 | + wrote is not theirs to have. | |
| 1166 | + | |
| 1167 | + | Event | When | | |
| 1168 | + | --- | --- | | |
| 1169 | + | `session_start` / `session_end` | one conversation, from open to close; a second tab is a second session | | |
| 1170 | + | `run_start` / `run_end` | one prompt, from your message to the model's final answer, tool calls and all | | |
| 1171 | + | `turn_start` | one model call inside a run | | |
| 1172 | + | `tool_start` / `tool_result` | one tool call; the result says whether it failed, not what it returned | | |
| 1173 | + | `blocked` / `unblocked` | waiting on a human: an approval prompt, or a question from `ask_user` | | |
| 1174 | + | `paused` / `resumed` | the loop held at a turn boundary, and let go again | | |
| 1175 | + | |
| 1176 | + `blocked` is the one you cannot get any other way. From outside the process an | |
| 1177 | + agent thinking hard and an agent that has been sitting on an approval prompt | |
| 1178 | + for ten minutes look exactly alike: the run has not returned and nothing is | |
| 1179 | + being printed. Only one of them will finish without you. | |
| 1180 | + | |
| 1181 | + Hooks in the config run shell commands: | |
| 1182 | + | |
| 1183 | + ```json | |
| 1184 | + { | |
| 1185 | + "hooks": { | |
| 1186 | + "run_end": ["notify-send 'codey' \"$CODEY_SESSION finished\""], | |
| 1187 | + "blocked": ["notify-send -u critical 'codey needs you' \"$CODEY_MESSAGE\""] | |
| 1188 | + } | |
| 1189 | + } | |
| 1190 | + ``` | |
| 1191 | + | |
| 1192 | + Each command gets the event twice over: as `CODEY_HOOK_EVENT`, `CODEY_SESSION`, | |
| 1193 | + `CODEY_AGENT`, `CODEY_TURN`, `CODEY_TOOL`, `CODEY_REASON`, `CODEY_MESSAGE` and | |
| 1194 | + `CODEY_ERROR` — always exported, sometimes empty, so a one-liner never has to | |
| 1195 | + test first — and as the same fields in JSON on stdin, for anything that wants a | |
| 1196 | + parser rather than string surgery. They run in the working root, one after | |
| 1197 | + another, on the agent's own goroutine, which is why each is given ten seconds | |
| 1198 | + and then abandoned: a hook that hangs would otherwise be a session that hangs. | |
| 1199 | + Output is swallowed unless the command fails, since the TUI owns the screen; a | |
| 1200 | + failure is written to the event log as a `hook_error` and changes nothing about | |
| 1201 | + the run. An unknown event name is refused at startup rather than ignored, | |
| 1202 | + because a hook that silently never fires is the one bug with nothing to notice. | |
| 1203 | + | |
| 1204 | + This is the single setting the wizard never writes. A hook is a command your | |
| 1205 | + machine runs on your behalf, so it gets into the file because you typed it | |
| 1206 | + there. `-no-hooks` turns the lot off for one run. | |
| 1207 | + | |
| 1208 | + ### herdr | |
| 1209 | + | |
| 1210 | + [herdr](https://herdr.dev) is a terminal workspace manager for coding agents: a | |
| 1211 | + background server owns the terminals, panes survive a detach, and a sidebar | |
| 1212 | + shows across every project which agent is working, which is blocked, and which | |
| 1213 | + is idle. Codey reports itself to herdr, automatically — there is nothing to | |
| 1214 | + install and nothing to configure. Run it in a pane and it says so at startup: | |
| 1215 | + | |
| 1216 | + ``` | |
| 1217 | + codey: herdr detected — reporting agent state to pane w1:p1 | |
| 1218 | + ``` | |
| 1219 | + | |
| 1220 | + Herdr reads most agents' states off the screen with pattern rules. Codey is | |
| 1221 | + told instead, through the contract herdr documents for agents it does not ship | |
| 1222 | + support for: a process inside a pane inherits `HERDR_ENV=1`, `HERDR_PANE_ID` | |
| 1223 | + and `HERDR_BIN_PATH`, and reports semantic state by running `herdr pane | |
| 1224 | + report-agent`. Being told beats guessing in both directions — herdr never has | |
| 1225 | + to infer anything from what the TUI happened to draw, and the state it could | |
| 1226 | + not see at all, that the agent is sitting on an approval prompt behind a | |
| 1227 | + full-screen view, is stated outright along with what it is waiting for: | |
| 1228 | + | |
| 1229 | + ``` | |
| 1230 | + bash: rm -rf build | |
| 1231 | + asked about Storage, Error handling | |
| 1232 | + ``` | |
| 1233 | + | |
| 1234 | + A subagent's prompt counts. Spawning does not block, so a background agent can | |
| 1235 | + ask for approval long after the agent that spawned it went quiet, and a pane | |
| 1236 | + that called that idle would leave you waiting on an agent that is waiting on | |
| 1237 | + you. Two agents can be waiting at once, and answering one does not clear the | |
| 1238 | + other: the pane goes back to what it was doing only when the last prompt is | |
| 1239 | + answered. | |
| 1240 | + | |
| 1241 | + One pane holds one Codey and one Codey can hold several tabs, so the tabs are | |
| 1242 | + reduced to the single state the pane is in: blocked beats working beats idle. | |
| 1243 | + What a person scanning a sidebar needs to find is the agent that stopped and | |
| 1244 | + wants them, and after that the one still going. A paused agent reports idle | |
| 1245 | + rather than blocked — nobody is being waited on, and it has no business in the | |
| 1246 | + list of agents that need attention. | |
| 1247 | + | |
| 1248 | + Every report carries Codey's session id, so herdr's pane and agent APIs show | |
| 1249 | + which conversation is running in a pane and a restored one can be pointed back | |
| 1250 | + at it with `-resume`. Herdr restarts an agent by itself only where it knows how | |
| 1251 | + to launch it, which for now means the agents in its own registry; the reference | |
| 1252 | + is reported regardless, because it costs one flag and is the half of the | |
| 1253 | + problem that is ours. | |
| 1254 | + | |
| 1255 | + Reporting is a no-op anywhere else. Detection is those three environment | |
| 1256 | + variables and nothing else — no probing, no socket, no cost to anyone who has | |
| 1257 | + never heard of herdr — and all three are required, because a stale `HERDR_ENV` | |
| 1258 | + exported into a shell that outlived its pane is exactly the case where | |
| 1259 | + reporting would go nowhere, slowly, forever. `-no-hooks` disables it too. | |
| 1260 | + | |
| 1261 | + When a state looks wrong, `herdr agent list` shows what herdr sees, and the | |
| 1262 | + `hook_error` lines in `.codey/events.jsonl` show what Codey could not tell it. | |
| 1263 | + | |
| 1264 | + #### The herdr skill | |
| 1265 | + | |
| 1266 | + Reporting is one direction. The other is the agent driving herdr itself — | |
| 1267 | + splitting a pane, starting a server in it without stealing your focus, reading | |
| 1268 | + what another pane printed, waiting until another agent is genuinely blocked | |
| 1269 | + rather than merely slow. Herdr publishes a skill that teaches an agent to do | |
| 1270 | + that, and [the setup wizard](#setup) offers to install it when it finds herdr | |
| 1271 | + on the machine: | |
| 1272 | + | |
| 1273 | + ``` | |
| 1274 | + Install herdr's skill, so the agent can drive herdr? | |
| 1275 | + | |
| 1276 | + ▌ ◆ yes, install it ~/.agents/skills/herdr | |
| 1277 | + ◇ no | |
| 1278 | + ``` | |
| 1279 | + | |
| 1280 | + Yes downloads [herdr's `SKILL.md`](https://github.com/herdrdev/herdr/blob/master/skills/herdr/SKILL.md) | |
| 1281 | + and writes it to `~/.agents/skills/herdr/SKILL.md`, which is where Codey looks | |
| 1282 | + for personal skills — the same layout herdr publishes and the same one | |
| 1283 | + `npx skills add` writes, so a skill installed by either is found by both. From | |
| 1284 | + the next start it is listed to the agent like any other skill, and invoked the | |
| 1285 | + same way. | |
| 1286 | + | |
| 1287 | + It is fetched rather than shipped inside the binary. The file is a manual for a | |
| 1288 | + CLI that is still moving, and a copy frozen into a Codey release would go on | |
| 1289 | + teaching last year's flags long after they stopped working. The cost is that | |
| 1290 | + the install needs the network: when it fails, nothing is written, the wizard | |
| 1291 | + says why, and the answers you just gave are saved regardless — only the | |
| 1292 | + download is lost, and `/setup` offers it again. | |
| 1293 | + | |
| 1294 | + Nothing is written unread. What comes back is parsed by the same loader that | |
| 1295 | + will read it at startup, and rejected if it is not a skill Codey can load — | |
| 1296 | + a login page, an error page, or a file whose shape has changed. The write goes | |
| 1297 | + through a temporary file in the same directory, so a failed update leaves the | |
| 1298 | + copy you already had rather than half of a new one. | |
| 1299 | + | |
| 1300 | + This is the only question in the wizard that acts rather than records, so it is | |
| 1301 | + the only one where pressing enter through the screens does something: the row | |
| 1302 | + names the exact path before you answer, and the notice afterwards names the | |
| 1303 | + file that was written. Uninstalling is `rm -r ~/.agents/skills/herdr`. | |
| 1304 | + | |
| 1002 | 1305 | ## Evals |
| 1003 | 1306 | |
| 1004 | 1307 | ```bash |
| @@ -1031,6 +1334,14 @@ interpreter with `-c` cannot be analysed at all, so it is marked opaque and | ||
| 1031 | 1334 | allowlists for shell commands do not work; this exists to make the prompt |
| 1032 | 1335 | specific, not to make the decision for you. |
| 1033 | 1336 | |
| 1337 | + Hooks sit outside all three layers, deliberately: a command in `"hooks"` runs | |
| 1338 | + on every event it is attached to, without a prompt, because you put it in your | |
| 1339 | + own config rather than the agent asking for it. The path jail is what keeps the | |
| 1340 | + agent from adding one — the config lives in `~/.config/codey/`, outside any | |
| 1341 | + working root you would sensibly point Codey at — but the flip side is that | |
| 1342 | + anything able to write that file has arranged to run commands here. `-no-hooks` | |
| 1343 | + is the switch that says no for one run. | |
| 1344 | + | |
| 1034 | 1345 | **A sandbox.** This is the layer that actually contains a mistake, and the |
| 1035 | 1346 | Codey cannot provide it for you. The honest recipe on Linux: |
| 1036 | 1347 | |
| @@ -1063,11 +1374,14 @@ cmd/codey flag parsing, wiring, REPL | ||
| 1063 | 1374 | internal/llm Messages API: types, HTTP, SSE, pricing |
| 1064 | 1375 | internal/agent turn loop, conversation state, compaction, sessions |
| 1065 | 1376 | internal/tools the tool registry and the tools |
| 1066 | - internal/skills discovery and parsing of SKILL.md | |
| 1377 | + internal/diag language servers and linters, and what they say about an edit | |
| 1378 | + internal/skills discovery and parsing of SKILL.md, Codey's own and Claude Code's | |
| 1067 | 1379 | internal/websearch the search command, the URL jail, HTML to text |
| 1068 | - internal/config saved defaults: model, effort, approvals, web search, theme | |
| 1380 | + internal/config saved defaults: model, effort, approvals, web search, theme, diagnostics | |
| 1069 | 1381 | internal/approval allow / deny / ask, with shell analysis |
| 1070 | 1382 | internal/ask questions the agent puts to you mid-task, and the tool for it |
| 1383 | + internal/hooks the lifecycle, published: fan-out, and the config's commands | |
| 1384 | + internal/herdr reporting agent state to the herdr pane Codey is running in | |
| 1071 | 1385 | internal/attach pictures pasted into a prompt: clipboard, files, markers |
| 1072 | 1386 | internal/metrics JSONL event log |
| 1073 | 1387 | internal/ui line-based terminal rendering |
| @@ -4,3 +4,27 @@ | ||
| 4 | 4 | - [x] **Make a new name!** |
| 5 | 5 | - [x] **Web search** — a wizard question with three answers: none, Claude's |
| 6 | 6 | hosted tool, or a command on this machine (ddgs by default). |
| 7 | + - [x] **Arrow up for the previous prompt** — `↑`/`↓` from the edge of the | |
| 8 | + prompt box walk what has been typed in this checkout before, kept in | |
| 9 | + `.codey/history`. Shell and python lines are walked separately from | |
| 10 | + prompts, and the half-written draft comes back. | |
| 11 | + - [x] **Use Claude's skills and plugins** — the wizard asks, where there are | |
| 12 | + some to ask about: leave them alone, read them in place, or copy them | |
| 13 | + once into `~/.agents/skills`. | |
| 14 | + - [x] **Auto LSP and linting** — an edited file goes to gopls, pyright, | |
| 15 | + typescript-language-server or rust-analyzer where one is installed, and | |
| 16 | + to `go build`/`go vet`, ruff, eslint or shellcheck where one is not. What | |
| 17 | + they say is appended to the tool result. On by default. | |
| 18 | + - [x] **A little UI/UX cleanup** — a hints line on the welcome block, a prompt | |
| 19 | + placeholder that names the keys, `/help` in columns with sections, | |
| 20 | + `/skills` grouped by source, and a settings panel that measures its own | |
| 21 | + label column. | |
| 22 | + | |
| 23 | + Still open: | |
| 24 | + | |
| 25 | + - The line-based REPL (`-no-tui`) has no history of its own: it reads with | |
| 26 | + `bufio.Scanner`, so `↑` there is still an escape sequence on the screen. | |
| 27 | + Everything interactive lives in the TUI, so this has not been worth a | |
| 28 | + readline dependency yet. | |
| 29 | + - `diag` swallows the reason a check produced nothing. If that ever needs | |
| 30 | + debugging, `Report` is the place to put a line in the event log. | |
| @@ -19,6 +19,9 @@ import ( | ||
| 19 | 19 | "codey/internal/ask" |
| 20 | 20 | "codey/internal/claudecode" |
| 21 | 21 | "codey/internal/config" |
| 22 | + "codey/internal/diag" | |
| 23 | + "codey/internal/herdr" | |
| 24 | + "codey/internal/hooks" | |
| 22 | 25 | "codey/internal/llm" |
| 23 | 26 | "codey/internal/metrics" |
| 24 | 27 | "codey/internal/skills" |
| @@ -75,16 +78,18 @@ func run() error { | ||
| 75 | 78 | noAutoCompact := flag.Bool("no-auto-compact", false, "never fold the conversation on its own; stop the turn when the window fills instead") |
| 76 | 79 | noCache := flag.Bool("no-cache", false, "disable prompt cache breakpoints") |
| 77 | 80 | noStream := flag.Bool("no-stream", false, "wait for each complete response instead of streaming") |
| 78 | - logPath := flag.String("metrics", metrics.DefaultPath, "JSONL event log path, relative to the working root") | |
| 81 | + logPath := flag.String("metrics", metrics.DefaultPath, "JSONL event log path, relative to the working root unless absolute") | |
| 79 | 82 | prompt := flag.String("p", "", "run one prompt and exit, instead of starting the REPL") |
| 80 | 83 | resume := flag.String("resume", "", "resume a saved session by id, or \"latest\"") |
| 81 | 84 | verbose := flag.Bool("v", false, "print full tool output") |
| 82 | 85 | yolo := flag.Bool("yolo", false, "skip every approval prompt. Only against a throwaway clone or inside a container") |
| 83 | 86 | noTUI := flag.Bool("no-tui", false, "use the plain line-based REPL instead of the full-screen TUI") |
| 84 | 87 | noWeb := flag.Bool("no-web", false, "do not offer web search, whatever the config says") |
| 85 | - noSkills := flag.Bool("no-skills", false, "skip discovering skills in ~/"+skills.Dirname+" and ./"+skills.Dirname) | |
| 88 | + noSkills := flag.Bool("no-skills", false, "skip discovering skills in ~/"+skills.Dirname+", ./"+skills.Dirname+" and Claude Code's own") | |
| 86 | 89 | noAgents := flag.Bool("no-agents", false, "do not offer the subagent tools, so the agent works alone") |
| 87 | 90 | noClaudeCodeAuth := flag.Bool("no-claude-code-auth", false, "do not fall back to the OAuth login in ~/"+claudecode.Filename) |
| 91 | + noHooks := flag.Bool("no-hooks", false, "do not run the hooks in the config, and do not report agent state to herdr") | |
| 92 | + noDiagnostics := flag.Bool("no-diagnostics", false, "do not check an edited file with the language server or linter installed for it") | |
| 88 | 93 | flag.Parse() |
| 89 | 94 | |
| 90 | 95 | root, err := filepath.Abs(*workdir) |
| @@ -97,6 +102,9 @@ func run() error { | ||
| 97 | 102 | // and is reported rather than fatal: a stale directory that cannot be |
| 98 | 103 | // moved is a reason to say so, not a reason to refuse to start. |
| 99 | 104 | stateMoved, stateErr := statedir.Migrate(root) |
| 105 | + if err := statedir.Ensure(root); err != nil { | |
| 106 | + return err | |
| 107 | + } | |
| 100 | 108 | |
| 101 | 109 | // Saved preferences sit under the flags and over the built-in defaults. A |
| 102 | 110 | // config that cannot be read is reported and then ignored: Codey |
| @@ -164,7 +172,7 @@ func run() error { | ||
| 164 | 172 | autoCompactOff := *noAutoCompact || !cfg.AutoCompactEnabled() |
| 165 | 173 | |
| 166 | 174 | session := time.Now().UTC().Format("20060102T150405Z") |
| 167 | - log, err := metrics.Open(filepath.Join(root, *logPath), session) | |
| 175 | + log, err := metrics.Open(metricsPath(root, *logPath), session) | |
| 168 | 176 | if err != nil { |
| 169 | 177 | return err |
| 170 | 178 | } |
| @@ -223,12 +231,36 @@ func run() error { | ||
| 223 | 231 | } |
| 224 | 232 | |
| 225 | 233 | var found []skills.Skill |
| 234 | + // claudeFound is what Claude Code has on this machine: its own skills and | |
| 235 | + // the ones its plugins ship. They are discovered even when the config says | |
| 236 | + // to leave them alone, because the count is what decides whether the setup | |
| 237 | + // wizard has a question to ask at all — and a wizard that never mentions | |
| 238 | + // them is how someone with fourteen skills installed ends up with none. | |
| 239 | + var claudeFound []skills.Skill | |
| 240 | + // claudeMode is what this run is doing about them, which -no-skills can | |
| 241 | + // make "off" whatever the config says. | |
| 242 | + claudeMode := config.ClaudeSkillsOff | |
| 226 | 243 | if !*noSkills { |
| 227 | 244 | var problems []error |
| 228 | 245 | found, problems = skills.Discover(home, root) |
| 229 | 246 | for _, p := range problems { |
| 230 | 247 | notice("skill skipped: %v", p) |
| 231 | 248 | } |
| 249 | + | |
| 250 | + var claudeProblems []error | |
| 251 | + claudeFound, claudeProblems = skills.DiscoverClaude(home, root) | |
| 252 | + for _, p := range claudeProblems { | |
| 253 | + notice("claude skill skipped: %v", p) | |
| 254 | + } | |
| 255 | + // Only "use" merges them in. "copy" is a thing the wizard did once, | |
| 256 | + // and what it copied is already a personal skill that Discover found | |
| 257 | + // above; reading Claude's directories again would offer the same | |
| 258 | + // instructions twice under one name. | |
| 259 | + if cfg.ClaudeSkillsMode() == config.ClaudeSkillsUse && len(claudeFound) > 0 { | |
| 260 | + found = skills.Merge(found, claudeFound) | |
| 261 | + claudeMode = config.ClaudeSkillsUse | |
| 262 | + notice("using %d skill(s) from Claude Code", len(claudeFound)) | |
| 263 | + } | |
| 232 | 264 | } |
| 233 | 265 | |
| 234 | 266 | // Skill directories are readable by the search tools and by nothing else, |
| @@ -274,6 +306,38 @@ func run() error { | ||
| 274 | 306 | } |
| 275 | 307 | } |
| 276 | 308 | |
| 309 | + // Whatever this machine can say about a file the agent has just written. | |
| 310 | + // A nil Runner is the off switch, and every call site takes one, so the | |
| 311 | + // setting is a single branch here rather than a condition threaded through | |
| 312 | + // the tools. | |
| 313 | + // | |
| 314 | + // One Runner serves every root, including the ones /worktree moves to: it | |
| 315 | + // keys its language servers by the workspace it detects around the file, | |
| 316 | + // not by the root it was built for, so moving the work does not strand a | |
| 317 | + // warm gopls behind it. | |
| 318 | + var checks *diag.Runner | |
| 319 | + if cfg.DiagnosticsEnabled() && !*noDiagnostics { | |
| 320 | + checks = diag.NewRunner() | |
| 321 | + defer checks.Close() | |
| 322 | + } | |
| 323 | + | |
| 324 | + // Background commands outlive the call that started them, so something | |
| 325 | + // with the session's lifetime has to own them. Closing kills each process | |
| 326 | + // group: a dev server that outlived the harness holds its port with | |
| 327 | + // nothing left running to explain why. | |
| 328 | + procs := tools.NewProcesses() | |
| 329 | + defer procs.Close() | |
| 330 | + | |
| 331 | + // The working plan. It is the session's, not any one agent's: a subagent | |
| 332 | + // rewriting the parent's list would be two hands on the same page. | |
| 333 | + todos := tools.NewTodos() | |
| 334 | + // What was delivered rather than what was asked for, for the settings | |
| 335 | + // panel: -no-diagnostics leaves this off however the config reads. | |
| 336 | + diagMode := config.DiagnosticsOff | |
| 337 | + if checks != nil { | |
| 338 | + diagMode = config.DiagnosticsOn | |
| 339 | + } | |
| 340 | + | |
| 277 | 341 | // One factory, used for the session's own agent and for every subagent: |
| 278 | 342 | // a child that could reach further than its parent would be a way around |
| 279 | 343 | // the parent's limits, and a child that could reach less would be a |
| @@ -289,9 +353,11 @@ func run() error { | ||
| 289 | 353 | tools.NewRead(root, state, skillDirs...), |
| 290 | 354 | tools.NewGlob(root, skillDirs...), |
| 291 | 355 | tools.NewGrep(root, skillDirs...), |
| 292 | - tools.NewEdit(root, state), | |
| 293 | - tools.NewWrite(root, state), | |
| 294 | - tools.NewBash(root), | |
| 356 | + tools.NewEdit(root, state, tools.WithDiagnostics(checks)), | |
| 357 | + tools.NewWrite(root, state, tools.WithDiagnostics(checks)), | |
| 358 | + tools.NewBash(root, tools.WithProcesses(procs)), | |
| 359 | + tools.NewProcessOutput(procs), | |
| 360 | + tools.NewProcessKill(procs), | |
| 295 | 361 | ) |
| 296 | 362 | if t := tools.NewSkill(found); t != nil { |
| 297 | 363 | reg.Register(t) |
| @@ -301,6 +367,13 @@ func run() error { | ||
| 301 | 367 | // bash and curl instead. |
| 302 | 368 | if t := tools.NewWebSearch(searcher); t != nil { |
| 303 | 369 | reg.Register(t, tools.NewWebFetch()) |
| 370 | + } else if len(serverTools) > 0 { | |
| 371 | + // Hosted search has the same gap and no local half to close | |
| 372 | + // it: the model gets snippets it cannot follow, and a snippet | |
| 373 | + // is never enough to write code against. The consent is | |
| 374 | + // already given — this mode is the user asking for web access | |
| 375 | + // — so the fetch half goes in on its own. | |
| 376 | + reg.Register(tools.NewWebFetch()) | |
| 304 | 377 | } |
| 305 | 378 | return reg |
| 306 | 379 | } |
| @@ -323,6 +396,12 @@ func run() error { | ||
| 323 | 396 | // its report goes to the parent, and the parent is not a person. |
| 324 | 397 | asker := ask.NewAsker((&ui.Asker{In: in, Out: os.Stdout}).Prompt) |
| 325 | 398 | reg.Register(ask.Tool(asker.Ask)) |
| 399 | + // The plan belongs to the session agent for the same reason the question | |
| 400 | + // does: a subagent keeping its own list would show the user a plan for | |
| 401 | + // work they did not ask about. | |
| 402 | + if t := tools.NewTodoWrite(todos); t != nil { | |
| 403 | + reg.Register(t) | |
| 404 | + } | |
| 326 | 405 | |
| 327 | 406 | policy.YOLO = acceptAll |
| 328 | 407 | if acceptAll && !useTUI { |
| @@ -387,6 +466,9 @@ func run() error { | ||
| 387 | 466 | a := agent.New(client, reg, state, log, base) |
| 388 | 467 | a.Observe = render.Handle |
| 389 | 468 | a.Approve = policy |
| 469 | + // Registered after the agent because it reads that agent's own budget. | |
| 470 | + // /worktree rebuilds the set below and puts it back for the same reason. | |
| 471 | + reg.Register(tools.NewContextStatus(a.ContextReport)) | |
| 390 | 472 | |
| 391 | 473 | // newSessionTools is what /worktree calls to move the work: it builds the |
| 392 | 474 | // session's whole tool set against another root and takes the supervisor |
| @@ -395,11 +477,15 @@ func run() error { | ||
| 395 | 477 | newSessionTools := func(r string) *tools.Registry { |
| 396 | 478 | next := toolsIn(r) |
| 397 | 479 | built := next(state, log) |
| 480 | + built.Register(tools.NewContextStatus(a.ContextReport)) | |
| 398 | 481 | // The session's own agent keeps the tool that asks you a question, |
| 399 | 482 | // wherever the work has moved to. It is registered here rather than |
| 400 | 483 | // in the factory for the same reason the spawn tools are: a subagent |
| 401 | 484 | // reports to its parent, and the parent is not a person to ask. |
| 402 | 485 | built.Register(ask.Tool(asker.Ask)) |
| 486 | + if t := tools.NewTodoWrite(todos); t != nil { | |
| 487 | + built.Register(t) | |
| 488 | + } | |
| 403 | 489 | if sup != nil { |
| 404 | 490 | sup.SetTools(next) |
| 405 | 491 | built.Register(sup.Tools()...) |
| @@ -445,6 +531,49 @@ func run() error { | ||
| 445 | 531 | } |
| 446 | 532 | } |
| 447 | 533 | |
| 534 | + // Lifecycle hooks, wired here rather than where the agent was built | |
| 535 | + // because a resumed session only just took its id, and a state report | |
| 536 | + // that named the id this run would otherwise have had is a report nobody | |
| 537 | + // can resume from. | |
| 538 | + // | |
| 539 | + // What listens is a bystander: a multiplexer drawing a sidebar, a script | |
| 540 | + // that wants to know a turn finished. Nothing here can change what the | |
| 541 | + // agent does, and a hook that fails is logged rather than raised. | |
| 542 | + hk := hooks.New(session) | |
| 543 | + hk.OnError(func(p hooks.Payload, err error) { | |
| 544 | + log.Log(metrics.Event{Type: "hook_error", Err: err.Error(), | |
| 545 | + Extra: map[string]any{"event": string(p.Event)}}) | |
| 546 | + }) | |
| 547 | + if !*noHooks { | |
| 548 | + // Herdr owns the terminal Codey is running in and shows, across every | |
| 549 | + // project, which agent is working and which stopped to ask something. | |
| 550 | + // Detect returns nil unless this process really is inside one of its | |
| 551 | + // panes, so this line costs nothing to anyone who does not use it. | |
| 552 | + if reporter := herdr.Detect(); reporter != nil { | |
| 553 | + hk.Add(reporter) | |
| 554 | + notice("herdr detected — reporting agent state to pane %s", reporter.Pane()) | |
| 555 | + } | |
| 556 | + runner, hookErr := hooks.NewRunner(cfg.Hooks, root) | |
| 557 | + if hookErr != nil { | |
| 558 | + // A misspelt event name is worth stopping for. The alternative is | |
| 559 | + // a hook that silently never fires, which is the one failure mode | |
| 560 | + // nobody debugs because there is nothing to see. | |
| 561 | + return fmt.Errorf("config: %w", hookErr) | |
| 562 | + } | |
| 563 | + if runner != nil { | |
| 564 | + hk.Add(runner) | |
| 565 | + } | |
| 566 | + } | |
| 567 | + // Order matters on the way out: the session ends, and only then does the | |
| 568 | + // last handler get to release whatever it claimed. | |
| 569 | + defer hk.Close() | |
| 570 | + defer hk.Fire(context.Background(), hooks.Payload{Event: hooks.SessionEnd}) | |
| 571 | + | |
| 572 | + a.Hooks = hk | |
| 573 | + policy.Hooks = hk | |
| 574 | + asker.Hooks = hk | |
| 575 | + hk.Fire(context.Background(), hooks.Payload{Event: hooks.SessionStart}) | |
| 576 | + | |
| 448 | 577 | // Every tab is a session of its own, so opening one means building the |
| 449 | 578 | // whole stack a session stands on a second time: its own state, its own |
| 450 | 579 | // tool set, its own approval policy and its own subagent supervisor. Only |
| @@ -485,9 +614,18 @@ func run() error { | ||
| 485 | 614 | |
| 486 | 615 | a := agent.New(client, tabReg, st, log, base) |
| 487 | 616 | a.Approve = pol |
| 617 | + tabReg.Register(tools.NewContextStatus(a.ContextReport)) | |
| 618 | + // The same handlers, stamped with this tab's session id: a watcher | |
| 619 | + // outside the process sees one pane, and what it needs to know is | |
| 620 | + // whether anything in it is working or stuck — which is a question | |
| 621 | + // about all the tabs at once, not about whichever one is on screen. | |
| 622 | + tabHooks := hk.For(id) | |
| 623 | + a.Hooks = tabHooks | |
| 624 | + pol.Hooks = tabHooks | |
| 488 | 625 | log.Log(metrics.Event{Type: "session_start", Extra: map[string]any{ |
| 489 | 626 | "root": sessRoot, "model": client.Model(), "base_url": client.BaseURL, "tab": id, |
| 490 | 627 | }}) |
| 628 | + tabHooks.Fire(context.Background(), hooks.Payload{Event: hooks.SessionStart}) | |
| 491 | 629 | |
| 492 | 630 | return tui.SessionSpec{ |
| 493 | 631 | Agent: a, |
| @@ -508,6 +646,7 @@ func run() error { | ||
| 508 | 646 | if tabSup != nil { |
| 509 | 647 | tabSup.Shutdown() |
| 510 | 648 | } |
| 649 | + tabHooks.Fire(context.Background(), hooks.Payload{Event: hooks.SessionEnd}) | |
| 511 | 650 | }, |
| 512 | 651 | }, nil |
| 513 | 652 | } |
| @@ -557,6 +696,38 @@ func run() error { | ||
| 557 | 696 | // the only part that belongs out here. |
| 558 | 697 | SaveConfig: config.Save, |
| 559 | 698 | |
| 699 | + // What Claude Code has, so the wizard knows whether there is a | |
| 700 | + // question to ask and can say how many. The copy is a one-off act | |
| 701 | + // carried out on the way out of the wizard, which is why it is a | |
| 702 | + // callback rather than a setting: "I said yes in March" in the | |
| 703 | + // config file would be a standing instruction to copy again on | |
| 704 | + // every launch, over whatever had been edited since. | |
| 705 | + ClaudeSkills: len(claudeFound), | |
| 706 | + // What this session actually started under, which -no-skills can | |
| 707 | + // make "off" whatever the file says. The panel needs the truth to | |
| 708 | + // know when a change is one the next start will act on. | |
| 709 | + ClaudeSkillsMode: claudeMode, | |
| 710 | + // Likewise: what is actually attached to the edit tool, which is | |
| 711 | + // nothing on a machine with no checker installed for anything in | |
| 712 | + // the tree, whatever the file says. | |
| 713 | + Diagnostics: diagMode, | |
| 714 | + CopyClaudeSkills: func() (int, error) { | |
| 715 | + copied, problems, err := skills.CopyClaude(home, root) | |
| 716 | + for _, p := range problems { | |
| 717 | + log.Log(metrics.Event{Type: "skill_copy_problem", Err: p.Error()}) | |
| 718 | + } | |
| 719 | + return len(copied), err | |
| 720 | + }, | |
| 721 | + | |
| 722 | + // The setup wizard offers herdr's own skill — the one that | |
| 723 | + // teaches the agent to drive herdr, rather than merely report to | |
| 724 | + // it — but only on a machine that has herdr, and only when there | |
| 725 | + // is a home directory to install into. | |
| 726 | + HerdrPresent: home != "" && herdr.Present(), | |
| 727 | + InstallHerdrSkill: func(ctx context.Context) (string, error) { | |
| 728 | + return herdr.InstallSkill(ctx, home) | |
| 729 | + }, | |
| 730 | + | |
| 560 | 731 | // What ctrl+t needs to open a second conversation. |
| 561 | 732 | NewSession: newSession, |
| 562 | 733 | }) |
| @@ -606,6 +777,16 @@ func run() error { | ||
| 606 | 777 | // session and a collision waiting to happen for two opened in the same breath, |
| 607 | 778 | // which is exactly what a second tab is — so both the directory and the ids |
| 608 | 779 | // already handed out this run are checked, and a suffix settles it. |
| 780 | + // metricsPath anchors -metrics to the working root unless it is already | |
| 781 | + // absolute. Joining an absolute path onto root would bury the log under | |
| 782 | + // <root>/home/..., inside the very tree the agent is working in. | |
| 783 | + func metricsPath(root, flag string) string { | |
| 784 | + if filepath.IsAbs(flag) { | |
| 785 | + return flag | |
| 786 | + } | |
| 787 | + return filepath.Join(root, flag) | |
| 788 | + } | |
| 789 | + | |
| 609 | 790 | func freshSessionID(dir string, taken map[string]bool) (string, error) { |
| 610 | 791 | stamp := time.Now().UTC().Format("20060102T150405Z") |
| 611 | 792 | for n := 1; n < 1000; n++ { |
| @@ -0,0 +1,15 @@ | ||
| 1 | + package main | |
| 2 | + | |
| 3 | + import ( | |
| 4 | + "path/filepath" | |
| 5 | + "testing" | |
| 6 | + ) | |
| 7 | + | |
| 8 | + func TestMetricsPathLeavesAbsolutePathsAlone(t *testing.T) { | |
| 9 | + if got := metricsPath("/work", "/var/log/codey/events.jsonl"); got != "/var/log/codey/events.jsonl" { | |
| 10 | + t.Errorf("absolute -metrics was moved under the root: %s", got) | |
| 11 | + } | |
| 12 | + if got := metricsPath("/work", ".codey/events.jsonl"); got != filepath.Join("/work", ".codey/events.jsonl") { | |
| 13 | + t.Errorf("relative -metrics should anchor to the root, got %s", got) | |
| 14 | + } | |
| 15 | + } | |
| @@ -0,0 +1,240 @@ | ||
| 1 | + package agent | |
| 2 | + | |
| 3 | + import ( | |
| 4 | + "bytes" | |
| 5 | + "context" | |
| 6 | + "errors" | |
| 7 | + "os" | |
| 8 | + "path/filepath" | |
| 9 | + "sync" | |
| 10 | + "testing" | |
| 11 | + "time" | |
| 12 | + | |
| 13 | + "codey/internal/hooks" | |
| 14 | + ) | |
| 15 | + | |
| 16 | + // recorder keeps every payload fired at it. It locks because Paused fires from | |
| 17 | + // the agent's goroutine while the test reads from its own. | |
| 18 | + type recorder struct { | |
| 19 | + mu sync.Mutex | |
| 20 | + payloads []hooks.Payload | |
| 21 | + } | |
| 22 | + | |
| 23 | + func (r *recorder) Handle(_ context.Context, p hooks.Payload) error { | |
| 24 | + r.mu.Lock() | |
| 25 | + defer r.mu.Unlock() | |
| 26 | + r.payloads = append(r.payloads, p) | |
| 27 | + return nil | |
| 28 | + } | |
| 29 | + | |
| 30 | + func (r *recorder) events() []hooks.Event { | |
| 31 | + r.mu.Lock() | |
| 32 | + defer r.mu.Unlock() | |
| 33 | + out := make([]hooks.Event, 0, len(r.payloads)) | |
| 34 | + for _, p := range r.payloads { | |
| 35 | + out = append(out, p.Event) | |
| 36 | + } | |
| 37 | + return out | |
| 38 | + } | |
| 39 | + | |
| 40 | + func (r *recorder) find(ev hooks.Event) (hooks.Payload, bool) { | |
| 41 | + r.mu.Lock() | |
| 42 | + defer r.mu.Unlock() | |
| 43 | + for _, p := range r.payloads { | |
| 44 | + if p.Event == ev { | |
| 45 | + return p, true | |
| 46 | + } | |
| 47 | + } | |
| 48 | + return hooks.Payload{}, false | |
| 49 | + } | |
| 50 | + | |
| 51 | + // recordHooks attaches a recorder to the agent and returns it. | |
| 52 | + func recordHooks(a *Agent) *recorder { | |
| 53 | + rec := &recorder{} | |
| 54 | + a.Hooks = hooks.New("s1") | |
| 55 | + a.Hooks.Add(rec) | |
| 56 | + return rec | |
| 57 | + } | |
| 58 | + | |
| 59 | + func sameEvents(got, want []hooks.Event) bool { | |
| 60 | + if len(got) != len(want) { | |
| 61 | + return false | |
| 62 | + } | |
| 63 | + for i := range want { | |
| 64 | + if got[i] != want[i] { | |
| 65 | + return false | |
| 66 | + } | |
| 67 | + } | |
| 68 | + return true | |
| 69 | + } | |
| 70 | + | |
| 71 | + func TestRunFiresTheLifecycleInOrder(t *testing.T) { | |
| 72 | + root := t.TempDir() | |
| 73 | + os.WriteFile(filepath.Join(root, "a.go"), []byte("A\n"), 0o644) | |
| 74 | + f := newFakeAPI(t, | |
| 75 | + msg("tool_use", toolUseBlock("tu_1", "read_file", `{"path":"a.go"}`)), | |
| 76 | + msg("end_turn", textBlock("done")), | |
| 77 | + ) | |
| 78 | + a := newTestAgent(t, f, root, &bytes.Buffer{}) | |
| 79 | + rec := recordHooks(a) | |
| 80 | + | |
| 81 | + if _, err := a.Run(context.Background(), "go"); err != nil { | |
| 82 | + t.Fatalf("Run: %v", err) | |
| 83 | + } | |
| 84 | + | |
| 85 | + want := []hooks.Event{hooks.RunStart, hooks.TurnStart, hooks.ToolStart, hooks.ToolResult, hooks.TurnStart, hooks.RunEnd} | |
| 86 | + if got := rec.events(); !sameEvents(got, want) { | |
| 87 | + t.Errorf("events = %v, want %v", got, want) | |
| 88 | + } | |
| 89 | + if p, _ := rec.find(hooks.ToolStart); p.Tool != "read_file" || p.Turn != 1 { | |
| 90 | + t.Errorf("ToolStart = %+v, want read_file on turn 1", p) | |
| 91 | + } | |
| 92 | + // The session id is what lets one handler tell two tabs apart, and the | |
| 93 | + // loop never sets it itself — the Hooks it fires on must. | |
| 94 | + if p, _ := rec.find(hooks.RunStart); p.Session != "s1" { | |
| 95 | + t.Errorf("RunStart.Session = %q, want the id the hooks were built with", p.Session) | |
| 96 | + } | |
| 97 | + } | |
| 98 | + | |
| 99 | + // Whether a call failed is the one thing about a tool result a bystander gets | |
| 100 | + // to know; what it returned is not. | |
| 101 | + func TestToolResultReportsFailureWithoutTheOutput(t *testing.T) { | |
| 102 | + root := t.TempDir() | |
| 103 | + os.WriteFile(filepath.Join(root, "a.go"), []byte("package a\n"), 0o644) | |
| 104 | + tests := []struct { | |
| 105 | + name string | |
| 106 | + call string | |
| 107 | + tool string | |
| 108 | + wantErr string | |
| 109 | + }{ | |
| 110 | + {name: "success leaves Err empty", call: toolUseBlock("tu_1", "read_file", `{"path":"a.go"}`), tool: "read_file"}, | |
| 111 | + {name: "failure carries the error", call: toolUseBlock("tu_1", "boom", `{}`), tool: "boom", wantErr: "detonated"}, | |
| 112 | + } | |
| 113 | + for _, tc := range tests { | |
| 114 | + t.Run(tc.name, func(t *testing.T) { | |
| 115 | + f := newFakeAPI(t, msg("tool_use", tc.call), msg("end_turn", textBlock("ok"))) | |
| 116 | + a := newTestAgent(t, f, root, &bytes.Buffer{}) | |
| 117 | + rec := recordHooks(a) | |
| 118 | + if _, err := a.Run(context.Background(), "go"); err != nil { | |
| 119 | + t.Fatalf("Run: %v", err) | |
| 120 | + } | |
| 121 | + | |
| 122 | + p, ok := rec.find(hooks.ToolResult) | |
| 123 | + if !ok { | |
| 124 | + t.Fatalf("no ToolResult among %v", rec.events()) | |
| 125 | + } | |
| 126 | + if p.Tool != tc.tool { | |
| 127 | + t.Errorf("Tool = %q, want %q", p.Tool, tc.tool) | |
| 128 | + } | |
| 129 | + if p.Err != tc.wantErr { | |
| 130 | + t.Errorf("Err = %q, want %q", p.Err, tc.wantErr) | |
| 131 | + } | |
| 132 | + if p.Message != "" { | |
| 133 | + t.Errorf("Message = %q, want nothing: a tool's output is not a bystander's business", p.Message) | |
| 134 | + } | |
| 135 | + }) | |
| 136 | + } | |
| 137 | + } | |
| 138 | + | |
| 139 | + // A run that ends badly is still over. The sidebar that shows an agent as | |
| 140 | + // working because its run never said otherwise is the bug this guards. | |
| 141 | + func TestRunEndFiresOnEveryWayOut(t *testing.T) { | |
| 142 | + t.Run("turn cap", func(t *testing.T) { | |
| 143 | + f := newFakeAPI(t, | |
| 144 | + msg("tool_use", toolUseBlock("tu", "boom", `{}`)), | |
| 145 | + msg("tool_use", toolUseBlock("tu", "boom", `{}`)), | |
| 146 | + ) | |
| 147 | + a := newTestAgent(t, f, t.TempDir(), &bytes.Buffer{}) | |
| 148 | + a.opts.MaxTurns = 2 | |
| 149 | + rec := recordHooks(a) | |
| 150 | + | |
| 151 | + if _, err := a.Run(context.Background(), "go"); !errors.Is(err, ErrMaxTurns) { | |
| 152 | + t.Fatalf("error = %v, want ErrMaxTurns", err) | |
| 153 | + } | |
| 154 | + got := rec.events() | |
| 155 | + if len(got) == 0 || got[len(got)-1] != hooks.RunEnd { | |
| 156 | + t.Errorf("events = %v, want RunEnd last", got) | |
| 157 | + } | |
| 158 | + }) | |
| 159 | + | |
| 160 | + t.Run("cancelled while paused", func(t *testing.T) { | |
| 161 | + f := newPausableAPI(t, msg("end_turn", textBlock("hi"))) | |
| 162 | + a := newPausableAgent(t, f, t.TempDir()) | |
| 163 | + a.Pause() | |
| 164 | + rec := recordHooks(a) | |
| 165 | + | |
| 166 | + ctx, cancel := context.WithCancel(context.Background()) | |
| 167 | + errc := make(chan error, 1) | |
| 168 | + go func() { | |
| 169 | + _, err := a.Run(ctx, "go") | |
| 170 | + errc <- err | |
| 171 | + }() | |
| 172 | + time.Sleep(settle) | |
| 173 | + cancel() | |
| 174 | + select { | |
| 175 | + case err := <-errc: | |
| 176 | + if !errors.Is(err, context.Canceled) { | |
| 177 | + t.Fatalf("Run returned %v, want context.Canceled", err) | |
| 178 | + } | |
| 179 | + case <-time.After(2 * time.Second): | |
| 180 | + t.Fatal("a cancelled pause never unblocked") | |
| 181 | + } | |
| 182 | + | |
| 183 | + // The pause was never released, so Resumed must not appear: a watcher | |
| 184 | + // told the agent resumed would look for a turn that never came. | |
| 185 | + want := []hooks.Event{hooks.RunStart, hooks.Paused, hooks.RunEnd} | |
| 186 | + if got := rec.events(); !sameEvents(got, want) { | |
| 187 | + t.Errorf("events = %v, want %v", got, want) | |
| 188 | + } | |
| 189 | + }) | |
| 190 | + } | |
| 191 | + | |
| 192 | + func TestPauseAndResumeAreAnnounced(t *testing.T) { | |
| 193 | + f := newPausableAPI(t, msg("end_turn", textBlock("done"))) | |
| 194 | + a := newPausableAgent(t, f, t.TempDir()) | |
| 195 | + a.Pause() | |
| 196 | + rec := recordHooks(a) | |
| 197 | + | |
| 198 | + done := make(chan struct{}) | |
| 199 | + go func() { | |
| 200 | + if _, err := a.Run(context.Background(), "go"); err != nil { | |
| 201 | + t.Errorf("Run: %v", err) | |
| 202 | + } | |
| 203 | + close(done) | |
| 204 | + }() | |
| 205 | + time.Sleep(settle) | |
| 206 | + a.Resume() | |
| 207 | + select { | |
| 208 | + case <-done: | |
| 209 | + case <-time.After(2 * time.Second): | |
| 210 | + t.Fatal("the run never finished after Resume") | |
| 211 | + } | |
| 212 | + | |
| 213 | + want := []hooks.Event{hooks.RunStart, hooks.Paused, hooks.Resumed, hooks.TurnStart, hooks.RunEnd} | |
| 214 | + if got := rec.events(); !sameEvents(got, want) { | |
| 215 | + t.Errorf("events = %v, want %v", got, want) | |
| 216 | + } | |
| 217 | + if p, _ := rec.find(hooks.Paused); p.Turn != 1 { | |
| 218 | + t.Errorf("Paused.Turn = %d, want the turn being held", p.Turn) | |
| 219 | + } | |
| 220 | + } | |
| 221 | + | |
| 222 | + // Most sessions have nothing watching from outside, and the loop must not | |
| 223 | + // make them pay for it with a nil check at every fire point — or a panic. | |
| 224 | + func TestNilHooksRunsNormally(t *testing.T) { | |
| 225 | + f := newFakeAPI(t, | |
| 226 | + msg("tool_use", toolUseBlock("tu_1", "boom", `{}`)), | |
| 227 | + msg("end_turn", textBlock("fine")), | |
| 228 | + ) | |
| 229 | + a := newTestAgent(t, f, t.TempDir(), &bytes.Buffer{}) | |
| 230 | + if a.Hooks != nil { | |
| 231 | + t.Fatal("a fresh agent should have no hooks") | |
| 232 | + } | |
| 233 | + got, err := a.Run(context.Background(), "go") | |
| 234 | + if err != nil { | |
| 235 | + t.Fatalf("Run: %v", err) | |
| 236 | + } | |
| 237 | + if got != "fine" { | |
| 238 | + t.Errorf("answer = %q", got) | |
| 239 | + } | |
| 240 | + } | |
| @@ -9,6 +9,7 @@ import ( | ||
| 9 | 9 | "strings" |
| 10 | 10 | "sync" |
| 11 | 11 | |
| 12 | + "codey/internal/hooks" | |
| 12 | 13 | "codey/internal/llm" |
| 13 | 14 | "codey/internal/metrics" |
| 14 | 15 | "codey/internal/skills" |
| @@ -32,22 +33,216 @@ var ErrMaxTurns = errors.New("turn limit reached") | ||
| 32 | 33 | |
| 33 | 34 | // defaultSystem is deliberately static: it is the first thing in the cached |
| 34 | 35 | // prompt prefix, so a timestamp or session id here would cost a cache hit on |
| 35 | - // every single request. | |
| 36 | - const defaultSystem = `You are a coding agent operating on a local repository through tools. | |
| 36 | + // every single request. It also holds no digits, for the same reason — see | |
| 37 | + // TestRequestShape. | |
| 38 | + // | |
| 39 | + // Its shape follows the prompts Claude Code and Codex ship with: short headed | |
| 40 | + // sections on how to work, what not to touch, what to confirm, and how to | |
| 41 | + // report. Rules for tools this harness does not have — patch grammars, plan | |
| 42 | + // trackers — are left out on purpose. | |
| 43 | + const defaultSystem = `You are a coding agent running in Codey, a terminal harness, on a local | |
| 44 | + repository. You are expected to be precise, careful, and useful. The user is on | |
| 45 | + the same machine and can open any file you mention. | |
| 46 | + | |
| 47 | + # Harness | |
| 48 | + | |
| 49 | + Text you write outside of tool calls is shown to the user as Markdown in a | |
| 50 | + terminal. Tools run under a permission mode the user chose: some calls run | |
| 51 | + automatically, others stop and ask. A denied call means the user declined it — | |
| 52 | + adjust your approach, do not retry it verbatim. If the repository has a | |
| 53 | + CLAUDE.md or AGENTS.md, read it once at the start of the task and follow it: it | |
| 54 | + outranks these defaults on anything about this codebase. | |
| 55 | + | |
| 56 | + # How you work | |
| 37 | 57 | |
| 38 | 58 | Never guess at file paths. If you do not know where something lives, find it |
| 39 | 59 | with glob or grep first — one search costs less than a run of wrong guesses, |
| 40 | 60 | and a wrong guess tells you nothing about where the file actually is. Once you |
| 41 | 61 | know the path, read the specific file or region you need rather than dumping |
| 42 | - whole directories. | |
| 62 | + whole directories. Prefer the dedicated file and search tools over shell | |
| 63 | + equivalents when one fits; bash is for running things. | |
| 43 | 64 | |
| 44 | - When you change something, verify it by running the project's own tests rather | |
| 45 | - than asserting that it works. | |
| 65 | + Independent tool calls go in one response, in parallel. When a call depends on | |
| 66 | + the result of another, wait for that result first. Do not re-read a file after | |
| 67 | + editing it: edit_file returns a diff and fails loudly when it cannot apply. | |
| 68 | + | |
| 69 | + A command that does not return on its own — a dev server, a watcher, a tail — | |
| 70 | + is started with background: true and read with process_output; stop it with | |
| 71 | + process_kill. Never leave one in the foreground waiting for a timeout, and | |
| 72 | + never reach for pkill: a pattern you pass it can match the shell running your | |
| 73 | + own command. | |
| 74 | + | |
| 75 | + The file tools reach only the working root. Anything you need to read or write | |
| 76 | + with them — a verification script, a screenshot — goes under .codey/scratch | |
| 77 | + inside the project, which is ignored by git; bash itself can go anywhere, so a | |
| 78 | + throwaway dependency install for a browser driver can live in a temp directory. | |
| 79 | + | |
| 80 | + Carry a task through. Unless the user asked for a plan, asked a question about | |
| 81 | + the code, or is clearly thinking out loud, they want the change made — make it, | |
| 82 | + verify it, and report, rather than describing what you would do. Do not stop at | |
| 83 | + analysis or a partial fix. | |
| 46 | 84 | |
| 47 | 85 | If a tool returns an error, read it: the error text usually says exactly what to |
| 48 | - do differently. Do not repeat a failing call unchanged. | |
| 86 | + do differently. Do not repeat a failing call unchanged. Use git log and git | |
| 87 | + blame when you need to know why code is the way it is. | |
| 88 | + | |
| 89 | + Never claim the environment lacks something without having looked. "There is | |
| 90 | + no browser here" is a finding from which, ls and npx, not an assumption; the | |
| 91 | + same goes for compilers, databases and test runners. Check, then say what you | |
| 92 | + found. | |
| 93 | + | |
| 94 | + # Economy | |
| 95 | + | |
| 96 | + Every turn sends the whole conversation again, so the cost of a task is mostly | |
| 97 | + its turn count. Do as much as possible per response. Writing a new project | |
| 98 | + means writing several files in one response, not one file per turn: a | |
| 99 | + feature's component, its styles and its test go out together. Reading several | |
| 100 | + screenshots means several read_file calls in one response. A check that can | |
| 101 | + be one bash command — build, then test, then lint — is one command. | |
| 102 | + | |
| 103 | + Keep a plan with todo_write when a task has several distinct steps, when the | |
| 104 | + order matters, or when the user asked for more than one thing — one item | |
| 105 | + in_progress, marked done as you go. Skip it for anything you can finish in a | |
| 106 | + couple of actions. When you are unsure whether there is room for what is left, | |
| 107 | + context_status tells you how full the window is and how many turns you have | |
| 108 | + spent. | |
| 109 | + | |
| 110 | + Write the files you want directly. Do not run a scaffolding generator and then | |
| 111 | + delete and replace what it produced: that is the same work twice. Match the | |
| 112 | + number of files to the size of the project; a small app does not need a | |
| 113 | + stylesheet per component. Do not verify the same thing twice — a flow that | |
| 114 | + passed once in the browser is not run again after a change elsewhere; only the | |
| 115 | + piece that changed is looked at again. Keep tool output short: tail what you | |
| 116 | + need, do not cat what you just wrote. | |
| 117 | + | |
| 118 | + # Scope | |
| 119 | + | |
| 120 | + Do what was asked, at the scope it was asked, and no more. In an existing | |
| 121 | + codebase work with surgical precision: match its style, naming and comment | |
| 122 | + density, and do not rename, move or reformat things the task did not require. | |
| 123 | + Fix problems at the root rather than papering over symptoms. | |
| 124 | + | |
| 125 | + Scope cuts the other way on a new project. When the directory is empty or the | |
| 126 | + user is starting something from scratch, the quality of what you deliver is | |
| 127 | + the deliverable: be ambitious about it. Make it robust at the edges — browser | |
| 128 | + APIs that may be missing in some contexts, empty states, bad input, storage | |
| 129 | + that is unavailable — and make it look and feel finished. The rules below | |
| 130 | + about not adding things guard existing code against drift; they are not a | |
| 131 | + reason to ship a thin version of something new. | |
| 132 | + | |
| 133 | + Do not add features, abstractions, configuration, or error handling beyond what | |
| 134 | + the task needs. Three similar lines beat a premature helper; a bug fix does not | |
| 135 | + need surrounding cleanup. Do not validate what cannot happen — trust internal | |
| 136 | + code and the language's guarantees, and check only at system boundaries. Do | |
| 137 | + not leave backwards-compatibility shims, renamed unused variables, or | |
| 138 | + commented-out code for things you removed: if it is unused, delete it. | |
| 139 | + | |
| 140 | + Default to writing no comments. A comment states a constraint the code cannot | |
| 141 | + show; it never explains what the next line does or why your change is correct. | |
| 142 | + Do not add license or copyright headers. Prefer editing existing files to | |
| 143 | + creating new ones, and do not create planning or analysis documents unless the | |
| 144 | + user asked for one. | |
| 145 | + | |
| 146 | + Do not fix unrelated bugs or failing tests you come across. Mention them in | |
| 147 | + your report instead. Do not commit, create branches, or amend commits unless | |
| 148 | + the user asked you to. | |
| 149 | + | |
| 150 | + # Executing actions with care | |
| 151 | + | |
| 152 | + Weigh the reversibility and blast radius of every action. Local, reversible | |
| 153 | + steps — editing files, running tests, reading anything — are yours to take | |
| 154 | + freely. Anything hard to undo or visible beyond this machine you confirm first: | |
| 155 | + deleting files or branches, force pushes, git reset --hard, git checkout -- or | |
| 156 | + git restore on paths you did not touch, dropping data, killing processes you | |
| 157 | + did not start, modifying CI, pushing code, opening or commenting on pull | |
| 158 | + requests, sending messages, uploading content to third-party services. One | |
| 159 | + approval covers the scope it was given, not every later occurrence. | |
| 160 | + | |
| 161 | + You may be in a dirty worktree. Changes you did not make are the user's work in | |
| 162 | + progress: never revert them, and if they are in files you are editing, read | |
| 163 | + them and work with them. If you notice unexpected changes appearing while you | |
| 164 | + work, stop and ask before continuing. Run git status before any command that | |
| 165 | + could discard uncommitted work, and stash or commit what you find first. Do not | |
| 166 | + bypass safety checks such as commit hooks to get past an obstacle; find the | |
| 167 | + cause. When in doubt, a reversible step — move it aside, rename it, stash it — | |
| 168 | + beats deletion. | |
| 169 | + | |
| 170 | + # Verifying | |
| 49 | 171 | |
| 50 | - Be concise. When the task is done, say what you changed and why, briefly.` | |
| 172 | + When you change something, verify it by running the project's own tests rather | |
| 173 | + than asserting that it works. Start with the tests closest to what you changed, | |
| 174 | + then widen as confidence grows. Where the codebase has tests and an obvious | |
| 175 | + place for a new one, add it; do not introduce a test suite or a formatter to a | |
| 176 | + project that has none. Run the project's formatter if it has one. Do not fix | |
| 177 | + unrelated failures you meet on the way — report them. | |
| 178 | + | |
| 179 | + Verify the way the user will use it, not only the way that is convenient to | |
| 180 | + test. Test at the level the requirement lives: "survives a reload" is a test | |
| 181 | + that renders, reloads and looks again, not a test of the reducer underneath. | |
| 182 | + Passing unit tests on the logic say nothing about whether the thing works when | |
| 183 | + clicked. | |
| 184 | + | |
| 185 | + Anything with a user interface gets one browser pass before you call it done — | |
| 186 | + one, at the end, on the final build. Find a browser (which chromium | |
| 187 | + chromium-browser google-chrome firefox, the playwright cache under the home | |
| 188 | + directory, npx playwright); if there is one, serve the build and run a single | |
| 189 | + script that drives the real flow — the actions the user will take, in order, | |
| 190 | + against real storage — and saves three screenshots: desktop light, desktop | |
| 191 | + dark, one narrow mobile width. Have the script also report what a picture | |
| 192 | + cannot show, such as horizontal overflow at each width. Read the three | |
| 193 | + screenshots in one response, fix what you see, and re-shoot only the view that | |
| 194 | + changed. Only when there is genuinely no browser do you say so, in the first | |
| 195 | + sentence of your report, and say what you verified instead. | |
| 196 | + | |
| 197 | + On a new project the order is: build and test in place, then the clean-room | |
| 198 | + check — remove the build output and installed dependencies, install from | |
| 199 | + scratch, build and test once more — then the one browser pass on that clean | |
| 200 | + build. Not a browser pass before and another after. | |
| 201 | + | |
| 202 | + # Asking | |
| 203 | + | |
| 204 | + Use the ask_user tool when a decision is genuinely the user's: scope, risk | |
| 205 | + appetite, which of several defensible designs they want. Do not ask about | |
| 206 | + things you can settle by reading the code or by an obvious default. Bundle the | |
| 207 | + questions you have into one call rather than interrupting repeatedly, and ask | |
| 208 | + at the point the decision is needed, before building one of the answers. When | |
| 209 | + an exploratory question is asked of you — what could we do about this, how | |
| 210 | + should we approach that — answer in a few sentences with a recommendation and | |
| 211 | + its main trade-off, and do not implement until the user agrees. | |
| 212 | + | |
| 213 | + # Delegating | |
| 214 | + | |
| 215 | + Subagents cost time and context: each one re-explores, and you then read its | |
| 216 | + report. Delegate work that is genuinely independent, large enough to justify a | |
| 217 | + fresh context window, or that would otherwise flood your own with search | |
| 218 | + results. Do a small bounded sub-task — a few reads, one search, a short edit — | |
| 219 | + yourself. Do not fan several agents out over one modest job, and do not redo | |
| 220 | + or re-verify what you delegated while it runs. A well-briefed single agent | |
| 221 | + beats several loosely-briefed ones: the brief is all it will ever know. | |
| 222 | + | |
| 223 | + # Reporting | |
| 224 | + | |
| 225 | + The user reads your text, not your thinking and rarely your tool output. Say in | |
| 226 | + a sentence what you are about to do before your first tool call, and give a | |
| 227 | + short update when you find something that changes the picture or change | |
| 228 | + direction. Everything the user needs from a turn must be in your final message, | |
| 229 | + after the last tool call. | |
| 230 | + | |
| 231 | + Lead with the outcome. If a test failed, a step was skipped, or something came | |
| 232 | + back different from what you expected, that goes in the first sentence, even | |
| 233 | + when the rest succeeded. Report what you observed, not what a step should have | |
| 234 | + produced: done, fixed and verified are claims that rest on tool output you saw | |
| 235 | + this session. If you did not check, say so. Do not describe partial work as | |
| 236 | + finished, and do not quietly work around a failure so that it looks resolved. | |
| 237 | + | |
| 238 | + Match the size of the answer to the task. A simple question gets a direct | |
| 239 | + answer in plain prose; a small change gets a few sentences; a large one a short | |
| 240 | + walkthrough of what changed and why, grouped by file or concern. Reference code | |
| 241 | + as path:line so the user can jump to it. Do not paste back files you just | |
| 242 | + wrote, and do not tell the user to save or copy anything — they have the files. | |
| 243 | + Suggest natural next steps briefly when there are any; skip it when there are | |
| 244 | + not. No emojis unless asked. Avoid apologies and self-commentary: correct a | |
| 245 | + mistake in a sentence and move on.` | |
| 51 | 246 | |
| 52 | 247 | // ClaudeCodeIdentity is what an OAuth-authenticated request must present as |
| 53 | 248 | // its first system block. It is a fixed string, so it costs no cache hit: the |
| @@ -63,7 +258,7 @@ func skillListing(found []skills.Skill) string { | ||
| 63 | 258 | return "" |
| 64 | 259 | } |
| 65 | 260 | var b strings.Builder |
| 66 | - b.WriteString("\n\nSkills are instruction sets for specific kinds of work. Invoke one with\nthe skill tool when its description matches what you are doing — before\nstarting, not after. A user message beginning with /<name> is a request to\ninvoke that skill; everything after the name is its arguments.\n\nAvailable skills:") | |
| 261 | + b.WriteString("\n\n# Skills\n\nSkills are instruction sets for specific kinds of work. Invoke one with\nthe skill tool when its description matches what you are doing — before\nstarting, not after. A user message beginning with /<name> is a request to\ninvoke that skill; everything after the name is its arguments.\n\nAvailable skills:") | |
| 67 | 262 | for _, s := range found { |
| 68 | 263 | fmt.Fprintf(&b, "\n %s: %s", s.Name, s.Description) |
| 69 | 264 | } |
| @@ -88,7 +283,7 @@ func modelListing(models []llm.Model) string { | ||
| 88 | 283 | sort.SliceStable(sorted, func(i, j int) bool { return sorted[i].CreatedAt.After(sorted[j].CreatedAt) }) |
| 89 | 284 | |
| 90 | 285 | var b strings.Builder |
| 91 | - b.WriteString("\n\nModels this endpoint serves, newest first:") | |
| 286 | + b.WriteString("\n\n# Models\n\nModels this endpoint serves, newest first:") | |
| 92 | 287 | for i, m := range sorted { |
| 93 | 288 | fmt.Fprintf(&b, "\n %s", m.ID) |
| 94 | 289 | if i == 0 { |
| @@ -205,6 +400,14 @@ type Agent struct { | ||
| 205 | 400 | // Observe is called for every event. It may be nil. |
| 206 | 401 | Observe func(Event) |
| 207 | 402 | |
| 403 | + // Hooks publishes the lifecycle to anything outside the process that | |
| 404 | + // wants it — a multiplexer showing whether this pane is working or stuck, | |
| 405 | + // a notifier, a script. It is deliberately not Observe: that one carries | |
| 406 | + // text deltas and tool payloads for whoever is drawing the screen, and | |
| 407 | + // there is only ever one of it. A nil *Hooks drops everything, which is | |
| 408 | + // what a session nobody is watching from outside costs. | |
| 409 | + Hooks *hooks.Hooks | |
| 410 | + | |
| 208 | 411 | // Approve gates mutating tool calls. Nil means everything is allowed, |
| 209 | 412 | // which is only appropriate against a throwaway tree or in a sandbox. |
| 210 | 413 | Approve Approver |
| @@ -266,10 +469,12 @@ func (a *Agent) hold(ctx context.Context, turn int) error { | ||
| 266 | 469 | |
| 267 | 470 | a.emit(Event{Kind: EventPaused, Turn: turn}) |
| 268 | 471 | a.log.Log(metrics.Event{Type: "paused", Turn: turn}) |
| 472 | + a.fire(ctx, hooks.Paused, hooks.Payload{Turn: turn}) | |
| 269 | 473 | select { |
| 270 | 474 | case <-ch: |
| 271 | 475 | a.emit(Event{Kind: EventResumed, Turn: turn}) |
| 272 | 476 | a.log.Log(metrics.Event{Type: "resumed", Turn: turn}) |
| 477 | + a.fire(ctx, hooks.Resumed, hooks.Payload{Turn: turn}) | |
| 273 | 478 | return nil |
| 274 | 479 | case <-ctx.Done(): |
| 275 | 480 | // The agent is going away while held. Nothing resumed it, so the flag |
| @@ -333,12 +538,42 @@ func New(client *llm.Client, reg *tools.Registry, state *State, log *metrics.Log | ||
| 333 | 538 | // State exposes the conversation for callers that need to inspect or persist it. |
| 334 | 539 | func (a *Agent) State() *State { return a.state } |
| 335 | 540 | |
| 541 | + // ContextReport is what the context_status tool reports: how full the window | |
| 542 | + // is, how many turns it took and what that cost. The model cannot see any of | |
| 543 | + // it otherwise, and all three change how it should work — a session near the | |
| 544 | + // fold should finish and report rather than start something new. | |
| 545 | + // | |
| 546 | + // lastInputTokens is read without the mutex because a tool only runs from | |
| 547 | + // dispatch, which is on the turn loop's own goroutine — the same one that | |
| 548 | + // writes it. | |
| 549 | + func (a *Agent) ContextReport() tools.ContextReport { | |
| 550 | + window, at, _ := a.thresholds() | |
| 551 | + usage, turns := a.state.Usage() | |
| 552 | + cost, known := usage.Cost(a.client.Model()) | |
| 553 | + return tools.ContextReport{ | |
| 554 | + UsedTokens: a.lastInputTokens, | |
| 555 | + WindowTokens: window, | |
| 556 | + CompactAt: at, | |
| 557 | + Turns: turns, | |
| 558 | + CostUSD: cost, | |
| 559 | + CostKnown: known, | |
| 560 | + } | |
| 561 | + } | |
| 562 | + | |
| 336 | 563 | func (a *Agent) emit(ev Event) { |
| 337 | 564 | if a.Observe != nil { |
| 338 | 565 | a.Observe(ev) |
| 339 | 566 | } |
| 340 | 567 | } |
| 341 | 568 | |
| 569 | + // fire publishes a lifecycle event. It is nil-safe twice over — a nil Hooks | |
| 570 | + // and a Hooks with nothing registered both return immediately — so the loop | |
| 571 | + // calls it unconditionally rather than testing first. | |
| 572 | + func (a *Agent) fire(ctx context.Context, ev hooks.Event, p hooks.Payload) { | |
| 573 | + p.Event = ev | |
| 574 | + a.Hooks.Fire(ctx, p) | |
| 575 | + } | |
| 576 | + | |
| 342 | 577 | // Run sends one user message and drives the tool loop until the model stops |
| 343 | 578 | // asking for tools. It returns the final assistant text. |
| 344 | 579 | func (a *Agent) Run(ctx context.Context, prompt string) (string, error) { |
| @@ -379,6 +614,17 @@ func (a *Agent) drive(ctx context.Context) (string, error) { | ||
| 379 | 614 | // like a failed one to anything reading the return value — a subagent's |
| 380 | 615 | // report, most of all. |
| 381 | 616 | var last string |
| 617 | + | |
| 618 | + // A run is the unit an onlooker cares about: from here until the model | |
| 619 | + // stops asking for tools, this agent is busy. The end is deferred so that | |
| 620 | + // every way out — the turn cap, a refusal, an interrupt — still says so. | |
| 621 | + // An agent left marked busy because the run ended down an error path is | |
| 622 | + // exactly the wrong state to leave in someone's sidebar. | |
| 623 | + a.fire(ctx, hooks.RunStart, hooks.Payload{}) | |
| 624 | + defer func() { | |
| 625 | + a.fire(ctx, hooks.RunEnd, hooks.Payload{}) | |
| 626 | + }() | |
| 627 | + | |
| 382 | 628 | for turn := 1; a.opts.MaxTurns <= NoTurnLimit || turn <= a.opts.MaxTurns; turn++ { |
| 383 | 629 | // A pause is honoured before the turn is announced, let alone spent: |
| 384 | 630 | // a held agent has not started this turn, and anything counting turns |
| @@ -388,6 +634,7 @@ func (a *Agent) drive(ctx context.Context) (string, error) { | ||
| 388 | 634 | } |
| 389 | 635 | |
| 390 | 636 | a.emit(Event{Kind: EventTurnStart, Turn: turn}) |
| 637 | + a.fire(ctx, hooks.TurnStart, hooks.Payload{Turn: turn}) | |
| 391 | 638 | |
| 392 | 639 | // Compaction is checked before building the request, using the token |
| 393 | 640 | // count the API reported last turn — free and exact, where counting |
| @@ -489,6 +736,7 @@ func (a *Agent) dispatch(ctx context.Context, turn int, calls []llm.ContentBlock | ||
| 489 | 736 | results := make([]llm.ContentBlock, 0, len(calls)) |
| 490 | 737 | for _, call := range calls { |
| 491 | 738 | a.emit(Event{Kind: EventToolStart, Turn: turn, Tool: call.Name, Input: call.Input}) |
| 739 | + a.fire(ctx, hooks.ToolStart, hooks.Payload{Turn: turn, Tool: call.Name}) | |
| 492 | 740 | |
| 493 | 741 | out, err := a.authorizedExecute(ctx, call) |
| 494 | 742 | if ctx.Err() != nil { |
| @@ -496,7 +744,7 @@ func (a *Agent) dispatch(ctx context.Context, turn int, calls []llm.ContentBlock | ||
| 496 | 744 | } |
| 497 | 745 | |
| 498 | 746 | isErr := err != nil |
| 499 | - payload := out | |
| 747 | + payload := out.Text | |
| 500 | 748 | if isErr { |
| 501 | 749 | payload = err.Error() |
| 502 | 750 | } |
| @@ -512,8 +760,22 @@ func (a *Agent) dispatch(ctx context.Context, turn int, calls []llm.ContentBlock | ||
| 512 | 760 | } |
| 513 | 761 | } |
| 514 | 762 | |
| 515 | - results = append(results, llm.ToolResultBlock(call.ID, payload, isErr)) | |
| 763 | + // The model gets the blocks when there are any — a picture — and the | |
| 764 | + // text otherwise. The transcript and the hooks only ever see text. | |
| 765 | + if !isErr && len(out.Blocks) > 0 { | |
| 766 | + results = append(results, llm.ToolResultBlocks(call.ID, out.Blocks)) | |
| 767 | + } else { | |
| 768 | + results = append(results, llm.ToolResultBlock(call.ID, payload, isErr)) | |
| 769 | + } | |
| 516 | 770 | a.emit(Event{Kind: EventToolResult, Turn: turn, Tool: call.Name, Result: payload, IsError: isErr}) |
| 771 | + // The result itself is not published: a hook is a bystander, and what | |
| 772 | + // a tool read out of this repository is not a bystander's business. | |
| 773 | + // Whether it failed is, because that is what a watcher acts on. | |
| 774 | + done := hooks.Payload{Turn: turn, Tool: call.Name} | |
| 775 | + if isErr { | |
| 776 | + done.Err = firstLine(payload) | |
| 777 | + } | |
| 778 | + a.fire(ctx, hooks.ToolResult, done) | |
| 517 | 779 | } |
| 518 | 780 | return results, nil |
| 519 | 781 | } |
| @@ -596,14 +858,14 @@ func summarizeWebSearchResult(b llm.ContentBlock) (string, bool) { | ||
| 596 | 858 | // authorizedExecute runs a tool once the approval policy allows it. A denial |
| 597 | 859 | // comes back as an ordinary tool error: the model should read it and change |
| 598 | 860 | // course, not see Codey crash. |
| 599 | - func (a *Agent) authorizedExecute(ctx context.Context, call llm.ContentBlock) (string, error) { | |
| 861 | + func (a *Agent) authorizedExecute(ctx context.Context, call llm.ContentBlock) (tools.Output, error) { | |
| 600 | 862 | if a.Approve != nil { |
| 601 | 863 | if err := a.Approve.Approve(ctx, call.Name, call.Input); err != nil { |
| 602 | 864 | if ctx.Err() != nil { |
| 603 | - return "", ctx.Err() | |
| 865 | + return tools.Output{}, ctx.Err() | |
| 604 | 866 | } |
| 605 | 867 | a.log.Log(metrics.Event{Type: "tool_denied", Tool: call.Name, Err: err.Error()}) |
| 606 | - return "", err | |
| 868 | + return tools.Output{}, err | |
| 607 | 869 | } |
| 608 | 870 | } |
| 609 | 871 | return a.registry().Execute(ctx, call.Name, call.Input) |
| @@ -5,6 +5,8 @@ import ( | ||
| 5 | 5 | "context" |
| 6 | 6 | "encoding/json" |
| 7 | 7 | "errors" |
| 8 | + "image" | |
| 9 | + "image/png" | |
| 8 | 10 | "io" |
| 9 | 11 | "net/http" |
| 10 | 12 | "net/http/httptest" |
| @@ -168,6 +170,85 @@ func TestRunReadsAFileAndAnswers(t *testing.T) { | ||
| 168 | 170 | } |
| 169 | 171 | } |
| 170 | 172 | |
| 173 | + // A screenshot read with read_file reaches the model as an image block inside | |
| 174 | + // the tool_result, and the transcript sees the caption. | |
| 175 | + func TestImageReadReachesTheModelAsAnImage(t *testing.T) { | |
| 176 | + root := t.TempDir() | |
| 177 | + // The smallest valid PNG: 1×1, from image/png's own encoder. | |
| 178 | + var buf bytes.Buffer | |
| 179 | + png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 1, 1))) | |
| 180 | + os.WriteFile(filepath.Join(root, "shot.png"), buf.Bytes(), 0o644) | |
| 181 | + | |
| 182 | + f := newFakeAPI(t, | |
| 183 | + msg("tool_use", toolUseBlock("tu_1", "read_file", `{"path":"shot.png"}`)), | |
| 184 | + msg("end_turn", textBlock("looks fine")), | |
| 185 | + ) | |
| 186 | + a := newTestAgent(t, f, root, &bytes.Buffer{}) | |
| 187 | + var seen []Event | |
| 188 | + a.Observe = func(e Event) { | |
| 189 | + if e.Kind == EventToolResult { | |
| 190 | + seen = append(seen, e) | |
| 191 | + } | |
| 192 | + } | |
| 193 | + if _, err := a.Run(context.Background(), "look at the screenshot"); err != nil { | |
| 194 | + t.Fatalf("Run: %v", err) | |
| 195 | + } | |
| 196 | + | |
| 197 | + raw := f.rawRequest(1) | |
| 198 | + if !strings.Contains(raw, `"type":"image"`) || !strings.Contains(raw, `"media_type":"image/png"`) { | |
| 199 | + t.Errorf("second request carries no image block:\n%s", raw) | |
| 200 | + } | |
| 201 | + res := f.requests[1].Messages[2].Content[0] | |
| 202 | + if len(res.Blocks) != 2 || res.Blocks[1].Type != llm.BlockImage { | |
| 203 | + t.Errorf("tool_result blocks = %+v, want caption and image", res.Blocks) | |
| 204 | + } | |
| 205 | + if len(seen) != 1 || !strings.Contains(seen[0].Result, "PNG image, 1×1") { | |
| 206 | + t.Errorf("the transcript should see the caption, got %+v", seen) | |
| 207 | + } | |
| 208 | + } | |
| 209 | + | |
| 210 | + // Superseding a read of an image drops the picture too: the model must not | |
| 211 | + // keep reasoning from a screenshot of a file that has since changed. | |
| 212 | + func TestSupersedeDropsImageBlocks(t *testing.T) { | |
| 213 | + root := t.TempDir() | |
| 214 | + var buf bytes.Buffer | |
| 215 | + png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 1, 1))) | |
| 216 | + os.WriteFile(filepath.Join(root, "shot.png"), buf.Bytes(), 0o644) | |
| 217 | + | |
| 218 | + f := newFakeAPI(t, | |
| 219 | + msg("tool_use", toolUseBlock("tu_1", "read_file", `{"path":"shot.png"}`)), | |
| 220 | + msg("tool_use", toolUseBlock("tu_2", "read_file", `{"path":"shot.png"}`)), | |
| 221 | + msg("tool_use", toolUseBlock("tu_3", "write_file", `{"path":"shot.png","content":"not a png any more"}`)), | |
| 222 | + msg("end_turn", textBlock("done")), | |
| 223 | + ) | |
| 224 | + a := newTestAgent(t, f, root, &bytes.Buffer{}) | |
| 225 | + if _, err := a.Run(context.Background(), "redo the screenshot"); err != nil { | |
| 226 | + t.Fatalf("Run: %v", err) | |
| 227 | + } | |
| 228 | + var images, superseded int | |
| 229 | + for _, m := range a.State().History() { | |
| 230 | + for _, b := range m.Content { | |
| 231 | + if b.Type != llm.BlockToolResult { | |
| 232 | + continue | |
| 233 | + } | |
| 234 | + if strings.HasPrefix(b.Content, "[superseded:") { | |
| 235 | + superseded++ | |
| 236 | + if len(b.Blocks) != 0 { | |
| 237 | + t.Errorf("a superseded result still carries %d blocks", len(b.Blocks)) | |
| 238 | + } | |
| 239 | + } | |
| 240 | + for _, inner := range b.Blocks { | |
| 241 | + if inner.Type == llm.BlockImage { | |
| 242 | + images++ | |
| 243 | + } | |
| 244 | + } | |
| 245 | + } | |
| 246 | + } | |
| 247 | + if superseded != 1 || images != 1 { | |
| 248 | + t.Errorf("superseded=%d images=%d, want 1 and 1 (the later read keeps its picture)", superseded, images) | |
| 249 | + } | |
| 250 | + } | |
| 251 | + | |
| 171 | 252 | func TestParallelToolCallsReturnInOneUserMessage(t *testing.T) { |
| 172 | 253 | root := t.TempDir() |
| 173 | 254 | os.WriteFile(filepath.Join(root, "a.go"), []byte("A\n"), 0o644) |
| @@ -101,6 +101,9 @@ func (s *State) SupersedeReads(path string) int { | ||
| 101 | 101 | } |
| 102 | 102 | s.messages[i].Content[j].Content = stalePrefix + path + |
| 103 | 103 | " has been modified since this read. The contents shown here are out of date; read it again if you need them.]" |
| 104 | + // An image of the old file is as stale as its text, and far | |
| 105 | + // bigger; the notice above is all that goes back. | |
| 106 | + s.messages[i].Content[j].Blocks = nil | |
| 104 | 107 | n++ |
| 105 | 108 | } |
| 106 | 109 | } |
| @@ -0,0 +1,137 @@ | ||
| 1 | + package approval | |
| 2 | + | |
| 3 | + import ( | |
| 4 | + "context" | |
| 5 | + "encoding/json" | |
| 6 | + "errors" | |
| 7 | + "strings" | |
| 8 | + "testing" | |
| 9 | + | |
| 10 | + "codey/internal/hooks" | |
| 11 | + ) | |
| 12 | + | |
| 13 | + // recordHooks attaches a recorder and returns the payloads it will fill. | |
| 14 | + func recordHooks(p *Policy) *[]hooks.Payload { | |
| 15 | + var got []hooks.Payload | |
| 16 | + p.Hooks = hooks.New("s1") | |
| 17 | + p.Hooks.Add(hooks.HandlerFunc(func(_ context.Context, pl hooks.Payload) error { | |
| 18 | + got = append(got, pl) | |
| 19 | + return nil | |
| 20 | + })) | |
| 21 | + return &got | |
| 22 | + } | |
| 23 | + | |
| 24 | + func TestPromptIsBracketedByBlockedAndUnblocked(t *testing.T) { | |
| 25 | + tests := []struct { | |
| 26 | + name string | |
| 27 | + decision Decision | |
| 28 | + promptErr error | |
| 29 | + }{ | |
| 30 | + {name: "allowed once", decision: Once}, | |
| 31 | + {name: "denied", decision: Deny}, | |
| 32 | + {name: "prompt failed", promptErr: errors.New("turn cancelled")}, | |
| 33 | + } | |
| 34 | + for _, tc := range tests { | |
| 35 | + t.Run(tc.name, func(t *testing.T) { | |
| 36 | + // The prompt writes into the same sequence as the hooks, so the | |
| 37 | + // test can tell "Blocked before the human was asked" from | |
| 38 | + // "Blocked at some point". | |
| 39 | + var order []string | |
| 40 | + p := New(func(context.Context, Request) (Decision, error) { | |
| 41 | + order = append(order, "prompt") | |
| 42 | + return tc.decision, tc.promptErr | |
| 43 | + }) | |
| 44 | + var payloads []hooks.Payload | |
| 45 | + p.Hooks = hooks.New("s1") | |
| 46 | + p.Hooks.Add(hooks.HandlerFunc(func(_ context.Context, pl hooks.Payload) error { | |
| 47 | + order = append(order, string(pl.Event)) | |
| 48 | + payloads = append(payloads, pl) | |
| 49 | + return nil | |
| 50 | + })) | |
| 51 | + | |
| 52 | + err := p.ApproveAs(context.Background(), "a3", "bash", json.RawMessage(`{"command":"go test ./..."}`)) | |
| 53 | + if tc.promptErr != nil && !errors.Is(err, tc.promptErr) { | |
| 54 | + t.Fatalf("ApproveAs = %v, want the prompt's own error", err) | |
| 55 | + } | |
| 56 | + if want := "blocked prompt unblocked"; strings.Join(order, " ") != want { | |
| 57 | + t.Fatalf("order = %v, want %q", order, want) | |
| 58 | + } | |
| 59 | + for _, pl := range payloads { | |
| 60 | + if pl.Reason != hooks.ReasonApproval { | |
| 61 | + t.Errorf("%s.Reason = %q, want %q", pl.Event, pl.Reason, hooks.ReasonApproval) | |
| 62 | + } | |
| 63 | + if pl.Tool != "bash" { | |
| 64 | + t.Errorf("%s.Tool = %q, want bash", pl.Event, pl.Tool) | |
| 65 | + } | |
| 66 | + if pl.Agent != "a3" { | |
| 67 | + t.Errorf("%s.Agent = %q, want the asking subagent", pl.Event, pl.Agent) | |
| 68 | + } | |
| 69 | + } | |
| 70 | + // The command is what the prompt itself shows, and what lets a | |
| 71 | + // decision be made from a phone. | |
| 72 | + if msg := payloads[0].Message; !strings.Contains(msg, "go test ./...") { | |
| 73 | + t.Errorf("Blocked.Message = %q, want the command in it", msg) | |
| 74 | + } | |
| 75 | + }) | |
| 76 | + } | |
| 77 | + } | |
| 78 | + | |
| 79 | + // Blocked means a human is being waited on. A call nobody was asked about | |
| 80 | + // must not say so, or a sidebar flashes "blocked" on every read. | |
| 81 | + func TestNothingFiresWhenNoPromptIsNeeded(t *testing.T) { | |
| 82 | + tests := []struct { | |
| 83 | + name string | |
| 84 | + setup func(p *Policy) | |
| 85 | + tool string | |
| 86 | + input string | |
| 87 | + }{ | |
| 88 | + {name: "read-only tool", tool: "read_file", input: `{"path":"a.go"}`}, | |
| 89 | + {name: "YOLO", setup: func(p *Policy) { p.YOLO = true }, tool: "bash", input: `{"command":"rm -rf build"}`}, | |
| 90 | + { | |
| 91 | + name: "tool remembered as always allowed", | |
| 92 | + setup: func(p *Policy) { | |
| 93 | + // The remembering itself goes through the prompt, so its | |
| 94 | + // events are drained before the call under test. | |
| 95 | + approve(t, p, "bash", `{"command":"ls"}`) | |
| 96 | + }, | |
| 97 | + tool: "bash", input: `{"command":"go build ./..."}`, | |
| 98 | + }, | |
| 99 | + { | |
| 100 | + name: "command remembered as always allowed", | |
| 101 | + setup: func(p *Policy) { | |
| 102 | + p.Prompt = func(context.Context, Request) (Decision, error) { return AlwaysThisCommand, nil } | |
| 103 | + approve(t, p, "bash", `{"command":"go test ./..."}`) | |
| 104 | + }, | |
| 105 | + tool: "bash", input: `{"command":"go test ./..."}`, | |
| 106 | + }, | |
| 107 | + } | |
| 108 | + for _, tc := range tests { | |
| 109 | + t.Run(tc.name, func(t *testing.T) { | |
| 110 | + p := New(func(context.Context, Request) (Decision, error) { return AlwaysThisTool, nil }) | |
| 111 | + got := recordHooks(p) | |
| 112 | + if tc.setup != nil { | |
| 113 | + tc.setup(p) | |
| 114 | + } | |
| 115 | + *got = (*got)[:0] | |
| 116 | + | |
| 117 | + if err := approve(t, p, tc.tool, tc.input); err != nil { | |
| 118 | + t.Fatalf("%s was denied: %v", tc.tool, err) | |
| 119 | + } | |
| 120 | + if len(*got) != 0 { | |
| 121 | + t.Errorf("fired %+v for a call nobody was asked about", *got) | |
| 122 | + } | |
| 123 | + }) | |
| 124 | + } | |
| 125 | + } | |
| 126 | + | |
| 127 | + // A headless denial is instant, so there is no wait to announce. | |
| 128 | + func TestNilPromptFiresNothing(t *testing.T) { | |
| 129 | + p := New(nil) | |
| 130 | + got := recordHooks(p) | |
| 131 | + if err := approve(t, p, "bash", `{"command":"ls"}`); err == nil { | |
| 132 | + t.Fatal("a mutating call was allowed with no way to ask") | |
| 133 | + } | |
| 134 | + if len(*got) != 0 { | |
| 135 | + t.Errorf("fired %+v with no prompt installed", *got) | |
| 136 | + } | |
| 137 | + } | |
| @@ -7,6 +7,8 @@ import ( | ||
| 7 | 7 | "sort" |
| 8 | 8 | "strings" |
| 9 | 9 | "sync" |
| 10 | + | |
| 11 | + "codey/internal/hooks" | |
| 10 | 12 | ) |
| 11 | 13 | |
| 12 | 14 | // Decision is the outcome of an approval prompt. |
| @@ -55,6 +57,18 @@ var readOnly = map[string]bool{ | ||
| 55 | 57 | // ask_user changes nothing and is itself a prompt. Asking permission to |
| 56 | 58 | // ask a question would be two boxes for one decision. |
| 57 | 59 | "ask_user": true, |
| 60 | + // The plan and the budget are the agent talking about its own work. They | |
| 61 | + // touch nothing outside the conversation. | |
| 62 | + "todo_write": true, | |
| 63 | + "context_status": true, | |
| 64 | + // Reading a background process's output reaches no further than the | |
| 65 | + // command that produced it, which was approved when it started. | |
| 66 | + "process_output": true, | |
| 67 | + // Killing is the other half of starting, and the harness kills these | |
| 68 | + // anyway when the session ends. A prompt here would ask permission to | |
| 69 | + // tidy up — and the alternative the model reaches for without it is | |
| 70 | + // pkill, which is far worse aimed. | |
| 71 | + "process_kill": true, | |
| 58 | 72 | } |
| 59 | 73 | |
| 60 | 74 | // Policy decides whether a tool call proceeds, remembering what it is told. |
| @@ -69,6 +83,13 @@ type Policy struct { | ||
| 69 | 83 | // YOLO skips every prompt. Set it only where the blast radius is |
| 70 | 84 | // contained: a throwaway clone, a container, or the eval runner. |
| 71 | 85 | YOLO bool |
| 86 | + | |
| 87 | + // Hooks publishes the wait for a decision, for the same reason the asker | |
| 88 | + // does: an agent sitting on an approval prompt looks exactly like one | |
| 89 | + // thinking hard, and only one of the two will finish without you. It is | |
| 90 | + // fired around the prompt rather than around Approve, so a call allowed | |
| 91 | + // outright or remembered from earlier says nothing. | |
| 92 | + Hooks *hooks.Hooks | |
| 72 | 93 | } |
| 73 | 94 | |
| 74 | 95 | // New builds a policy. |
| @@ -104,7 +125,7 @@ func (p *Policy) ApproveAs(ctx context.Context, agent, tool string, input json.R | ||
| 104 | 125 | return fmt.Errorf("%s was not run: this session has no way to ask for approval, and %s can modify the working tree. Explain what you would do instead", tool, tool) |
| 105 | 126 | } |
| 106 | 127 | |
| 107 | - decision, err := p.Prompt(ctx, req) | |
| 128 | + decision, err := p.decide(ctx, req) | |
| 108 | 129 | if err != nil { |
| 109 | 130 | return err |
| 110 | 131 | } |
| @@ -128,6 +149,35 @@ func (p *Policy) ApproveAs(ctx context.Context, agent, tool string, input json.R | ||
| 128 | 149 | } |
| 129 | 150 | } |
| 130 | 151 | |
| 152 | + // decide puts the request to the user, announcing the wait to anything | |
| 153 | + // watching from outside and announcing its end however it ends. | |
| 154 | + func (p *Policy) decide(ctx context.Context, req Request) (Decision, error) { | |
| 155 | + p.Hooks.Fire(ctx, hooks.Payload{ | |
| 156 | + Event: hooks.Blocked, | |
| 157 | + Agent: req.Agent, | |
| 158 | + Tool: req.Tool, | |
| 159 | + Reason: hooks.ReasonApproval, | |
| 160 | + Message: blockMessage(req), | |
| 161 | + }) | |
| 162 | + defer p.Hooks.Fire(ctx, hooks.Payload{ | |
| 163 | + Event: hooks.Unblocked, | |
| 164 | + Agent: req.Agent, | |
| 165 | + Tool: req.Tool, | |
| 166 | + Reason: hooks.ReasonApproval, | |
| 167 | + }) | |
| 168 | + return p.Prompt(ctx, req) | |
| 169 | + } | |
| 170 | + | |
| 171 | + // blockMessage names what is waiting to be approved. The detail is the command | |
| 172 | + // line or the path the prompt itself shows, which is what makes the difference | |
| 173 | + // between "approve something" and a decision someone can make from a phone. | |
| 174 | + func blockMessage(req Request) string { | |
| 175 | + if req.Detail != "" { | |
| 176 | + return req.Tool + ": " + req.Detail | |
| 177 | + } | |
| 178 | + return "approve " + req.Tool | |
| 179 | + } | |
| 180 | + | |
| 131 | 181 | // Remembered lists what the session has been told to always allow, for showing |
| 132 | 182 | // the user what they have agreed to. |
| 133 | 183 | func (p *Policy) Remembered() []string { |
| @@ -162,23 +212,36 @@ func article(word string) string { | ||
| 162 | 212 | // describe turns a tool call into something a human can decide on in one read. |
| 163 | 213 | func describe(tool string, input json.RawMessage) Request { |
| 164 | 214 | var args struct { |
| 165 | - Command string `json:"command"` | |
| 166 | - Path string `json:"path"` | |
| 167 | - OldStr string `json:"old_str"` | |
| 168 | - NewStr string `json:"new_str"` | |
| 169 | - Content string `json:"content"` | |
| 170 | - Query string `json:"query"` | |
| 171 | - URL string `json:"url"` | |
| 172 | - Preset string `json:"preset"` | |
| 173 | - Task string `json:"task"` | |
| 174 | - Model string `json:"model"` | |
| 175 | - Effort string `json:"effort"` | |
| 215 | + Command string `json:"command"` | |
| 216 | + Background bool `json:"background"` | |
| 217 | + Path string `json:"path"` | |
| 218 | + OldStr string `json:"old_str"` | |
| 219 | + NewStr string `json:"new_str"` | |
| 220 | + Content string `json:"content"` | |
| 221 | + Query string `json:"query"` | |
| 222 | + URL string `json:"url"` | |
| 223 | + Preset string `json:"preset"` | |
| 224 | + Task string `json:"task"` | |
| 225 | + Model string `json:"model"` | |
| 226 | + Effort string `json:"effort"` | |
| 176 | 227 | } |
| 177 | 228 | json.Unmarshal(input, &args) |
| 178 | 229 | |
| 179 | 230 | switch tool { |
| 180 | 231 | case "bash": |
| 181 | 232 | info := Analyze(args.Command) |
| 233 | + if args.Background { | |
| 234 | + // Worth spelling out: agreeing to this is agreeing to something | |
| 235 | + // that is still running after the answer comes back. Never | |
| 236 | + // rememberable — "always allow" on a command that keeps running | |
| 237 | + // is a standing permission to leave processes behind. | |
| 238 | + return Request{ | |
| 239 | + Tool: tool, | |
| 240 | + Summary: "start a shell command in the background, left running — " + info.Describe(), | |
| 241 | + Detail: args.Command, | |
| 242 | + Rememberable: false, | |
| 243 | + } | |
| 244 | + } | |
| 182 | 245 | return Request{ |
| 183 | 246 | Tool: tool, |
| 184 | 247 | Summary: "run a shell command — " + info.Describe(), |
| @@ -15,7 +15,7 @@ func approve(t *testing.T, p *Policy, tool, input string) error { | ||
| 15 | 15 | func TestReadOnlyToolsNeverPrompt(t *testing.T) { |
| 16 | 16 | var asked int |
| 17 | 17 | p := New(func(context.Context, Request) (Decision, error) { asked++; return Deny, nil }) |
| 18 | - for _, tool := range []string{"read_file", "glob", "grep", "skill"} { | |
| 18 | + for _, tool := range []string{"read_file", "glob", "grep", "skill", "process_output", "process_kill", "todo_write", "context_status"} { | |
| 19 | 19 | if err := approve(t, p, tool, `{"path":"x"}`); err != nil { |
| 20 | 20 | t.Errorf("%s was denied: %v", tool, err) |
| 21 | 21 | } |
| @@ -25,6 +25,23 @@ func TestReadOnlyToolsNeverPrompt(t *testing.T) { | ||
| 25 | 25 | } |
| 26 | 26 | } |
| 27 | 27 | |
| 28 | + // A background start is still one prompt, but it has to say that the command | |
| 29 | + // will keep running after the call returns: that is the part a user agreeing | |
| 30 | + // to "run a shell command" would not otherwise know. | |
| 31 | + func TestBackgroundBashSaysItKeepsRunning(t *testing.T) { | |
| 32 | + got := describe("bash", []byte(`{"command":"npm run dev","background":true}`)) | |
| 33 | + if !strings.Contains(got.Summary, "background") { | |
| 34 | + t.Errorf("summary %q does not say the command keeps running", got.Summary) | |
| 35 | + } | |
| 36 | + if got.Rememberable { | |
| 37 | + t.Error("a background start should not be rememberable: the next one is a different server") | |
| 38 | + } | |
| 39 | + fg := describe("bash", []byte(`{"command":"ls"}`)) | |
| 40 | + if strings.Contains(fg.Summary, "background") { | |
| 41 | + t.Errorf("a foreground command should not mention background: %q", fg.Summary) | |
| 42 | + } | |
| 43 | + } | |
| 44 | + | |
| 28 | 45 | func TestMutatingToolsPrompt(t *testing.T) { |
| 29 | 46 | for _, tool := range []string{"bash", "edit_file", "write_file"} { |
| 30 | 47 | var asked int |
| @@ -20,6 +20,8 @@ import ( | ||
| 20 | 20 | "fmt" |
| 21 | 21 | "strings" |
| 22 | 22 | "sync" |
| 23 | + | |
| 24 | + "codey/internal/hooks" | |
| 23 | 25 | ) |
| 24 | 26 | |
| 25 | 27 | // The limits are the picker's, not the model's: four options fit on a screen |
| @@ -98,6 +100,14 @@ type Prompt func(context.Context, Request) (Response, error) | ||
| 98 | 100 | type Asker struct { |
| 99 | 101 | mu sync.Mutex |
| 100 | 102 | prompt Prompt |
| 103 | + | |
| 104 | + // Hooks publishes the wait itself. A question is the one moment an agent | |
| 105 | + // is neither working nor finished, and from outside the process the two | |
| 106 | + // are indistinguishable — the run has not returned and nothing is being | |
| 107 | + // printed. It lives here rather than around the Prompt a caller installs | |
| 108 | + // because a front end replaces that prompt when it takes over, and a | |
| 109 | + // wrapper put on at startup would be thrown away with it. | |
| 110 | + Hooks *hooks.Hooks | |
| 101 | 111 | } |
| 102 | 112 | |
| 103 | 113 | // NewAsker returns an asker with a starting prompt, which may be nil. |
| @@ -118,9 +128,41 @@ func (a *Asker) Ask(ctx context.Context, req Request) (Response, error) { | ||
| 118 | 128 | if p == nil { |
| 119 | 129 | return Response{}, errors.New("nothing in this session can reach the user") |
| 120 | 130 | } |
| 131 | + | |
| 132 | + a.Hooks.Fire(ctx, hooks.Payload{ | |
| 133 | + Event: hooks.Blocked, | |
| 134 | + Agent: req.Agent, | |
| 135 | + Reason: hooks.ReasonQuestion, | |
| 136 | + Message: summarize(req), | |
| 137 | + }) | |
| 138 | + // Unblocked on every path out, including the one where the user dismissed | |
| 139 | + // the box or the turn was cancelled underneath it. A watcher told an agent | |
| 140 | + // is waiting and never told otherwise is worse than one never told at all. | |
| 141 | + defer a.Hooks.Fire(ctx, hooks.Payload{ | |
| 142 | + Event: hooks.Unblocked, | |
| 143 | + Agent: req.Agent, | |
| 144 | + Reason: hooks.ReasonQuestion, | |
| 145 | + }) | |
| 146 | + | |
| 121 | 147 | return p(ctx, req) |
| 122 | 148 | } |
| 123 | 149 | |
| 150 | + // summarize is what a bystander is told the agent is waiting to hear about: | |
| 151 | + // the headers of the questions, which name the decisions without carrying the | |
| 152 | + // conversation they came out of. | |
| 153 | + func summarize(req Request) string { | |
| 154 | + headers := make([]string, 0, len(req.Questions)) | |
| 155 | + for _, q := range req.Questions { | |
| 156 | + if h := strings.TrimSpace(q.Header); h != "" { | |
| 157 | + headers = append(headers, h) | |
| 158 | + } | |
| 159 | + } | |
| 160 | + if len(headers) == 0 { | |
| 161 | + return "waiting for an answer" | |
| 162 | + } | |
| 163 | + return "asked about " + strings.Join(headers, ", ") | |
| 164 | + } | |
| 165 | + | |
| 124 | 166 | // Validate checks a request the model built. Every message it returns is read |
| 125 | 167 | // by the model and has to say what to send instead, because a rejected call |
| 126 | 168 | // is one the model gets to make again. |
| @@ -0,0 +1,105 @@ | ||
| 1 | + package ask | |
| 2 | + | |
| 3 | + import ( | |
| 4 | + "context" | |
| 5 | + "errors" | |
| 6 | + "strings" | |
| 7 | + "testing" | |
| 8 | + | |
| 9 | + "codey/internal/hooks" | |
| 10 | + ) | |
| 11 | + | |
| 12 | + // recordHooks attaches a recorder and returns the payloads it will fill. | |
| 13 | + func recordHooks(a *Asker) *[]hooks.Payload { | |
| 14 | + var got []hooks.Payload | |
| 15 | + a.Hooks = hooks.New("s1") | |
| 16 | + a.Hooks.Add(hooks.HandlerFunc(func(_ context.Context, p hooks.Payload) error { | |
| 17 | + got = append(got, p) | |
| 18 | + return nil | |
| 19 | + })) | |
| 20 | + return &got | |
| 21 | + } | |
| 22 | + | |
| 23 | + func TestAskBracketsThePromptWithBlockedAndUnblocked(t *testing.T) { | |
| 24 | + tests := []struct { | |
| 25 | + name string | |
| 26 | + promptErr error | |
| 27 | + }{ | |
| 28 | + {name: "answered"}, | |
| 29 | + {name: "prompt failed", promptErr: errors.New("turn cancelled")}, | |
| 30 | + } | |
| 31 | + for _, tc := range tests { | |
| 32 | + t.Run(tc.name, func(t *testing.T) { | |
| 33 | + // The prompt writes into the same sequence as the hooks, so the | |
| 34 | + // test can tell "Blocked before the human was asked" from | |
| 35 | + // "Blocked at some point". | |
| 36 | + var order []string | |
| 37 | + a := NewAsker(func(context.Context, Request) (Response, error) { | |
| 38 | + order = append(order, "prompt") | |
| 39 | + return Response{}, tc.promptErr | |
| 40 | + }) | |
| 41 | + a.Hooks = hooks.New("s1") | |
| 42 | + a.Hooks.Add(hooks.HandlerFunc(func(_ context.Context, p hooks.Payload) error { | |
| 43 | + order = append(order, string(p.Event)) | |
| 44 | + return nil | |
| 45 | + })) | |
| 46 | + | |
| 47 | + _, err := a.Ask(context.Background(), Request{Questions: []Question{twoOptions()}}) | |
| 48 | + if !errors.Is(err, tc.promptErr) { | |
| 49 | + t.Fatalf("Ask = %v, want the prompt's own error %v", err, tc.promptErr) | |
| 50 | + } | |
| 51 | + if want := "blocked prompt unblocked"; strings.Join(order, " ") != want { | |
| 52 | + t.Errorf("order = %v, want %q", order, want) | |
| 53 | + } | |
| 54 | + }) | |
| 55 | + } | |
| 56 | + } | |
| 57 | + | |
| 58 | + func TestBlockedNamesTheQuestionsWithoutRepeatingThem(t *testing.T) { | |
| 59 | + a := NewAsker(func(context.Context, Request) (Response, error) { return Response{}, nil }) | |
| 60 | + got := recordHooks(a) | |
| 61 | + | |
| 62 | + scope := twoOptions() | |
| 63 | + scope.Header = "Scope" | |
| 64 | + scope.Question = "Should this cover the TUI too?" | |
| 65 | + req := Request{Agent: "a3", Questions: []Question{twoOptions(), scope}} | |
| 66 | + if _, err := a.Ask(context.Background(), req); err != nil { | |
| 67 | + t.Fatalf("Ask: %v", err) | |
| 68 | + } | |
| 69 | + | |
| 70 | + blocked := (*got)[0] | |
| 71 | + if blocked.Reason != hooks.ReasonQuestion { | |
| 72 | + t.Errorf("Reason = %q, want %q", blocked.Reason, hooks.ReasonQuestion) | |
| 73 | + } | |
| 74 | + for _, header := range []string{"Storage", "Scope"} { | |
| 75 | + if !strings.Contains(blocked.Message, header) { | |
| 76 | + t.Errorf("Message %q does not name the %s question", blocked.Message, header) | |
| 77 | + } | |
| 78 | + } | |
| 79 | + // The headers name the decisions; the questions carry the conversation, | |
| 80 | + // which is not a bystander's to have. | |
| 81 | + if strings.Contains(blocked.Message, "How should sessions be stored?") { | |
| 82 | + t.Errorf("Message %q repeats the question text", blocked.Message) | |
| 83 | + } | |
| 84 | + for _, p := range *got { | |
| 85 | + if p.Agent != "a3" { | |
| 86 | + t.Errorf("%s.Agent = %q, want the asking subagent", p.Event, p.Agent) | |
| 87 | + } | |
| 88 | + } | |
| 89 | + if (*got)[1].Reason != hooks.ReasonQuestion { | |
| 90 | + t.Errorf("Unblocked.Reason = %q, want %q so a watcher can pair it with the Blocked", (*got)[1].Reason, hooks.ReasonQuestion) | |
| 91 | + } | |
| 92 | + } | |
| 93 | + | |
| 94 | + // With nothing to ask, nobody is waiting: a Blocked with no Unblocked to | |
| 95 | + // follow would be exactly the stuck state the events exist to prevent. | |
| 96 | + func TestNoPromptFiresNothing(t *testing.T) { | |
| 97 | + a := NewAsker(nil) | |
| 98 | + got := recordHooks(a) | |
| 99 | + if _, err := a.Ask(context.Background(), Request{Questions: []Question{twoOptions()}}); err == nil { | |
| 100 | + t.Fatal("an asker with nothing installed answered") | |
| 101 | + } | |
| 102 | + if len(*got) != 0 { | |
| 103 | + t.Errorf("fired %v with no prompt installed", *got) | |
| 104 | + } | |
| 105 | + } | |
| @@ -11,8 +11,11 @@ import ( | ||
| 11 | 11 | "errors" |
| 12 | 12 | "fmt" |
| 13 | 13 | "io/fs" |
| 14 | + "maps" | |
| 14 | 15 | "os" |
| 15 | 16 | "path/filepath" |
| 17 | + "reflect" | |
| 18 | + "slices" | |
| 16 | 19 | "strings" |
| 17 | 20 | |
| 18 | 21 | "codey/internal/llm" |
| @@ -79,6 +82,42 @@ const ( | ||
| 79 | 82 | // them: least capable and least surprising first. |
| 80 | 83 | var WebSearchModes = []string{WebSearchNone, WebSearchClaude, WebSearchCustom} |
| 81 | 84 | |
| 85 | + // What to do about the skills Claude Code has on this machine — its own, and | |
| 86 | + // the ones its plugins ship. | |
| 87 | + // | |
| 88 | + // The empty string means ClaudeSkillsOff, and for the same reason the empty | |
| 89 | + // web-search mode means none: a skill is a page of instructions the model | |
| 90 | + // follows, so it gets loaded because someone said so, not because a directory | |
| 91 | + // happened to exist in the home directory. | |
| 92 | + const ( | |
| 93 | + // ClaudeSkillsOff leaves them alone. Codey reads only its own roots. | |
| 94 | + ClaudeSkillsOff = "off" | |
| 95 | + // ClaudeSkillsUse reads them where they lie, on every run, so an | |
| 96 | + // updated plugin is an updated skill with nothing to re-run. | |
| 97 | + ClaudeSkillsUse = "use" | |
| 98 | + // ClaudeSkillsCopy copies them once into ~/.agents/skills, where they | |
| 99 | + // become ordinary personal skills: yours to edit, and unaffected by the | |
| 100 | + // next plugin update. Nothing is read from Claude's directories after | |
| 101 | + // that, which is why this behaves like "off" at startup. | |
| 102 | + ClaudeSkillsCopy = "copy" | |
| 103 | + ) | |
| 104 | + | |
| 105 | + // ClaudeSkillsModes are the settings the wizard offers, in the order it offers | |
| 106 | + // them: least surprising first, as with the web-search modes. | |
| 107 | + var ClaudeSkillsModes = []string{ClaudeSkillsOff, ClaudeSkillsUse, ClaudeSkillsCopy} | |
| 108 | + | |
| 109 | + // Diagnostics settings, as written to the file. | |
| 110 | + // | |
| 111 | + // Empty means DiagnosticsOn. This is the one default that leans the other way | |
| 112 | + // from web search, and deliberately: a diagnostic is produced by a tool | |
| 113 | + // already installed on this machine, reading a file the agent has just | |
| 114 | + // written. Nothing leaves the machine, and an agent that is not told it broke | |
| 115 | + // the build finds out three tool calls later, if at all. | |
| 116 | + const ( | |
| 117 | + DiagnosticsOn = "on" | |
| 118 | + DiagnosticsOff = "off" | |
| 119 | + ) | |
| 120 | + | |
| 82 | 121 | // DefaultSearchCommand is the command WebSearchCustom runs when nothing else |
| 83 | 122 | // is configured: ddgs, the DuckDuckGo CLI, which needs no account and no API |
| 84 | 123 | // key. {{query}} and {{count}} are substituted as whole argv elements, never |
| @@ -119,6 +158,26 @@ type Config struct { | ||
| 119 | 158 | // Empty means DefaultSearchCommand, so someone who never edits it picks up |
| 120 | 159 | // a better default if the shipped one changes. |
| 121 | 160 | SearchCommand string `json:"search_command,omitempty"` |
| 161 | + // ClaudeSkills is one of ClaudeSkillsModes. Empty means | |
| 162 | + // ClaudeSkillsOff. | |
| 163 | + ClaudeSkills string `json:"claude_skills,omitempty"` | |
| 164 | + // Diagnostics is DiagnosticsOn or DiagnosticsOff: whether a file the | |
| 165 | + // agent edits is handed to whatever language server or linter this | |
| 166 | + // machine has for it, and what they say handed back. Empty means on. | |
| 167 | + Diagnostics string `json:"diagnostics,omitempty"` | |
| 168 | + // Hooks maps a lifecycle event name to the shell commands run when it | |
| 169 | + // happens; the names are the ones in hooks.All. It is the one setting | |
| 170 | + // here the wizard never asks about and never writes: a hook is a command | |
| 171 | + // this machine runs on your behalf, so it gets into the file because you | |
| 172 | + // typed it there. | |
| 173 | + Hooks map[string][]string `json:"hooks,omitempty"` | |
| 174 | + // MCPServers maps a server name to how to start it. The name is what the | |
| 175 | + // server's tools are filed under — mcp__<name>__<tool> — so it is chosen | |
| 176 | + // by the user, not read from the server, and stays stable when the server | |
| 177 | + // is upgraded. Like Hooks it is never written by the wizard: each entry is | |
| 178 | + // a program this machine runs with your environment, so it gets into the | |
| 179 | + // file because you typed it there. | |
| 180 | + MCPServers map[string]MCPServer `json:"mcp_servers,omitempty"` | |
| 122 | 181 | // Setup is SetupDone once the questions have been answered to the end. |
| 123 | 182 | // |
| 124 | 183 | // It is what decides whether the wizard opens by itself, in place of the |
| @@ -129,6 +188,82 @@ type Config struct { | ||
| 129 | 188 | Setup string `json:"setup,omitempty"` |
| 130 | 189 | } |
| 131 | 190 | |
| 191 | + // MCPServer is one MCP server: a program that speaks the protocol on its | |
| 192 | + // stdin and stdout. It is the same shape Claude Code and Codex use for the | |
| 193 | + // same thing, so an entry can be carried across from either without being | |
| 194 | + // rewritten. | |
| 195 | + type MCPServer struct { | |
| 196 | + // Command is the program to run, looked up on PATH if it is not a path. | |
| 197 | + Command string `json:"command"` | |
| 198 | + Args []string `json:"args,omitempty"` | |
| 199 | + // Env is added to Codey's own environment for this server only; it is | |
| 200 | + // where an API key the server needs goes. | |
| 201 | + Env map[string]string `json:"env,omitempty"` | |
| 202 | + } | |
| 203 | + | |
| 204 | + // Equal reports whether two configs say the same thing. | |
| 205 | + // | |
| 206 | + // It exists because Config stopped being comparable with == when hooks | |
| 207 | + // arrived: one map field makes the whole struct uncomparable, and the settings | |
| 208 | + // panel decides whether to rewrite the file by asking whether anything | |
| 209 | + // changed. Spelling the comparison out is the price of the field, and it is a | |
| 210 | + // cheaper price than a settings panel that saves on every close. | |
| 211 | + func (c Config) Equal(other Config) bool { | |
| 212 | + // The copies are local, so the fields that need a rule of their own are | |
| 213 | + // lifted out and everything else compared wholesale. Reflection rather | |
| 214 | + // than a list of field names: a list is a thing to forget when the next | |
| 215 | + // setting arrives, and this runs once, when a settings panel closes. | |
| 216 | + mine, theirs := c.Hooks, other.Hooks | |
| 217 | + myServers, theirServers := c.MCPServers, other.MCPServers | |
| 218 | + c.Hooks, other.Hooks = nil, nil | |
| 219 | + c.MCPServers, other.MCPServers = nil, nil | |
| 220 | + return reflect.DeepEqual(c, other) && hooksEqual(mine, theirs) && mcpServersEqual(myServers, theirServers) | |
| 221 | + } | |
| 222 | + | |
| 223 | + // mcpServersEqual compares two server maps, treating a missing map and an | |
| 224 | + // empty one as the same thing, and a server with no args and one with an | |
| 225 | + // empty args list likewise. Both distinctions are invisible in the file once | |
| 226 | + // it has been saved, so a panel that saw one would be rewriting the file to | |
| 227 | + // say what it already says. | |
| 228 | + func mcpServersEqual(a, b map[string]MCPServer) bool { | |
| 229 | + if len(a) != len(b) { | |
| 230 | + return false | |
| 231 | + } | |
| 232 | + for name, as := range a { | |
| 233 | + bs, ok := b[name] | |
| 234 | + if !ok || as.Command != bs.Command || !slices.Equal(as.Args, bs.Args) || !maps.Equal(as.Env, bs.Env) { | |
| 235 | + return false | |
| 236 | + } | |
| 237 | + } | |
| 238 | + return true | |
| 239 | + } | |
| 240 | + | |
| 241 | + // hooksEqual compares two hook maps, treating a missing event and an empty | |
| 242 | + // list of commands for it as the same thing. | |
| 243 | + func hooksEqual(a, b map[string][]string) bool { | |
| 244 | + count := func(m map[string][]string) int { | |
| 245 | + n := 0 | |
| 246 | + for _, v := range m { | |
| 247 | + if len(v) > 0 { | |
| 248 | + n++ | |
| 249 | + } | |
| 250 | + } | |
| 251 | + return n | |
| 252 | + } | |
| 253 | + if count(a) != count(b) { | |
| 254 | + return false | |
| 255 | + } | |
| 256 | + for k, av := range a { | |
| 257 | + if len(av) == 0 { | |
| 258 | + continue | |
| 259 | + } | |
| 260 | + if !slices.Equal(av, b[k]) { | |
| 261 | + return false | |
| 262 | + } | |
| 263 | + } | |
| 264 | + return true | |
| 265 | + } | |
| 266 | + | |
| 132 | 267 | // WebSearchMode is the setting in force, resolving the empty string to |
| 133 | 268 | // WebSearchNone so callers never have to spell that reading out themselves. |
| 134 | 269 | func (c Config) WebSearchMode() string { |
| @@ -147,6 +282,23 @@ func (c Config) SearchCommandLine() string { | ||
| 147 | 282 | // ValidWebSearch reports whether mode is one this build offers. |
| 148 | 283 | func ValidWebSearch(mode string) bool { return contains(WebSearchModes, mode) } |
| 149 | 284 | |
| 285 | + // ClaudeSkillsMode is the setting in force, resolving the empty string to | |
| 286 | + // ClaudeSkillsOff. | |
| 287 | + func (c Config) ClaudeSkillsMode() string { | |
| 288 | + if !ValidClaudeSkills(c.ClaudeSkills) { | |
| 289 | + return ClaudeSkillsOff | |
| 290 | + } | |
| 291 | + return c.ClaudeSkills | |
| 292 | + } | |
| 293 | + | |
| 294 | + // ValidClaudeSkills reports whether mode is one this build offers. | |
| 295 | + func ValidClaudeSkills(mode string) bool { return contains(ClaudeSkillsModes, mode) } | |
| 296 | + | |
| 297 | + // DiagnosticsEnabled reports whether an edited file should be checked by | |
| 298 | + // whatever language server or linter is installed for it. Only an explicit | |
| 299 | + // "off" turns it off. | |
| 300 | + func (c Config) DiagnosticsEnabled() bool { return c.Diagnostics != DiagnosticsOff } | |
| 301 | + | |
| 150 | 302 | // AutoCompactEnabled reports whether the conversation should fold itself when |
| 151 | 303 | // the window fills. Only an explicit "off" turns it off. |
| 152 | 304 | func (c Config) AutoCompactEnabled() bool { return c.AutoCompact != AutoCompactOff } |
| @@ -274,6 +426,15 @@ func sanitize(c Config) Config { | ||
| 274 | 426 | if !ValidWebSearch(c.WebSearch) { |
| 275 | 427 | c.WebSearch = "" |
| 276 | 428 | } |
| 429 | + // Same reading of the same kind of doubt: an unrecognised word here | |
| 430 | + // leaves Claude's skills unread rather than loading instructions nobody | |
| 431 | + // in this session asked for. | |
| 432 | + if !ValidClaudeSkills(c.ClaudeSkills) { | |
| 433 | + c.ClaudeSkills = "" | |
| 434 | + } | |
| 435 | + if c.Diagnostics != DiagnosticsOn && c.Diagnostics != DiagnosticsOff { | |
| 436 | + c.Diagnostics = "" | |
| 437 | + } | |
| 277 | 438 | c.SearchCommand = strings.TrimSpace(c.SearchCommand) |
| 278 | 439 | // Anything other than the one word means the marker was not written by a |
| 279 | 440 | // wizard that finished, so it does not count as one. |
| @@ -114,7 +114,7 @@ func TestLoadReportsMissingWithoutError(t *testing.T) { | ||
| 114 | 114 | if found { |
| 115 | 115 | t.Error("reported a config that does not exist") |
| 116 | 116 | } |
| 117 | - if cfg != (Config{}) { | |
| 117 | + if !cfg.Equal(Config{}) { | |
| 118 | 118 | t.Errorf("a missing config should come back zeroed, got %+v", cfg) |
| 119 | 119 | } |
| 120 | 120 | } |
| @@ -138,7 +138,7 @@ func TestSaveThenLoadRoundTrips(t *testing.T) { | ||
| 138 | 138 | t.Fatal("Load did not find the config Save just wrote") |
| 139 | 139 | } |
| 140 | 140 | want.Version = Version // stamped on write |
| 141 | - if got != want { | |
| 141 | + if !got.Equal(want) { | |
| 142 | 142 | t.Errorf("round trip changed the config:\n got %+v\nwant %+v", got, want) |
| 143 | 143 | } |
| 144 | 144 | } |
| @@ -162,7 +162,7 @@ func TestLoadRejectsMalformedWithoutDiscardingIt(t *testing.T) { | ||
| 162 | 162 | if found { |
| 163 | 163 | t.Error("a malformed config was reported as usable") |
| 164 | 164 | } |
| 165 | - if cfg != (Config{}) { | |
| 165 | + if !cfg.Equal(Config{}) { | |
| 166 | 166 | t.Errorf("a malformed config produced values: %+v", cfg) |
| 167 | 167 | } |
| 168 | 168 | if b, _ := os.ReadFile(path); string(b) != "{not json" { |
| @@ -400,3 +400,134 @@ func TestSearchCommandFallsBackToTheDefault(t *testing.T) { | ||
| 400 | 400 | t.Errorf("SearchCommandLine() = %q, want %q", got.SearchCommandLine(), custom) |
| 401 | 401 | } |
| 402 | 402 | } |
| 403 | + | |
| 404 | + func TestHooksSurviveASaveAndLoad(t *testing.T) { | |
| 405 | + isolate(t) | |
| 406 | + | |
| 407 | + want := Config{ | |
| 408 | + Model: "claude-sonnet-5", | |
| 409 | + Hooks: map[string][]string{ | |
| 410 | + "blocked": {"notify-send codey \"$CODEY_MESSAGE\""}, | |
| 411 | + "run_end": {"true", "echo done"}, | |
| 412 | + }, | |
| 413 | + } | |
| 414 | + if err := Save(want); err != nil { | |
| 415 | + t.Fatalf("Save: %v", err) | |
| 416 | + } | |
| 417 | + | |
| 418 | + got, _, err := Load() | |
| 419 | + if err != nil { | |
| 420 | + t.Fatalf("Load: %v", err) | |
| 421 | + } | |
| 422 | + want.Version = Version // stamped on write | |
| 423 | + if !got.Equal(want) { | |
| 424 | + t.Errorf("round trip changed the hooks:\n got %+v\nwant %+v", got.Hooks, want.Hooks) | |
| 425 | + } | |
| 426 | + } | |
| 427 | + | |
| 428 | + // The settings panel decides whether to rewrite the file by asking whether | |
| 429 | + // anything changed, and it must not lose a hook it never knew about. | |
| 430 | + func TestEqual(t *testing.T) { | |
| 431 | + base := Config{Model: "m", Theme: "iris", Hooks: map[string][]string{"run_end": {"a"}}} | |
| 432 | + | |
| 433 | + cases := []struct { | |
| 434 | + name string | |
| 435 | + a, b Config | |
| 436 | + equal bool | |
| 437 | + }{ | |
| 438 | + {"the same config", base, base, true}, | |
| 439 | + {"a changed setting", base, Config{Model: "n", Theme: "iris", Hooks: base.Hooks}, false}, | |
| 440 | + { | |
| 441 | + name: "the same hooks in another map", | |
| 442 | + a: base, | |
| 443 | + b: Config{Model: "m", Theme: "iris", Hooks: map[string][]string{"run_end": {"a"}}}, | |
| 444 | + equal: true, | |
| 445 | + }, | |
| 446 | + { | |
| 447 | + name: "a changed hook", | |
| 448 | + a: base, | |
| 449 | + b: Config{Model: "m", Theme: "iris", Hooks: map[string][]string{"run_end": {"b"}}}, | |
| 450 | + equal: false, | |
| 451 | + }, | |
| 452 | + { | |
| 453 | + name: "a hook dropped", | |
| 454 | + a: base, | |
| 455 | + b: Config{Model: "m", Theme: "iris"}, | |
| 456 | + equal: false, | |
| 457 | + }, | |
| 458 | + { | |
| 459 | + name: "an event with no commands is no event at all", | |
| 460 | + a: Config{Model: "m"}, | |
| 461 | + b: Config{Model: "m", Hooks: map[string][]string{"run_end": {}}}, | |
| 462 | + // A panel that rewrote the file over this would be saving nothing. | |
| 463 | + equal: true, | |
| 464 | + }, | |
| 465 | + {"two zero values", Config{}, Config{}, true}, | |
| 466 | + } | |
| 467 | + for _, c := range cases { | |
| 468 | + t.Run(c.name, func(t *testing.T) { | |
| 469 | + if got := c.a.Equal(c.b); got != c.equal { | |
| 470 | + t.Errorf("Equal = %v, want %v", got, c.equal) | |
| 471 | + } | |
| 472 | + if got := c.b.Equal(c.a); got != c.equal { | |
| 473 | + t.Errorf("Equal is not symmetric: reversed gave %v, want %v", got, c.equal) | |
| 474 | + } | |
| 475 | + }) | |
| 476 | + } | |
| 477 | + } | |
| 478 | + | |
| 479 | + func TestMCPServersSurviveASaveAndLoad(t *testing.T) { | |
| 480 | + isolate(t) | |
| 481 | + | |
| 482 | + want := Config{ | |
| 483 | + MCPServers: map[string]MCPServer{ | |
| 484 | + "docs": {Command: "npx", Args: []string{"-y", "@example/docs-mcp"}, Env: map[string]string{"DOCS_KEY": "k"}}, | |
| 485 | + "db": {Command: "/usr/local/bin/db-mcp"}, | |
| 486 | + }, | |
| 487 | + } | |
| 488 | + if err := Save(want); err != nil { | |
| 489 | + t.Fatalf("Save: %v", err) | |
| 490 | + } | |
| 491 | + | |
| 492 | + got, _, err := Load() | |
| 493 | + if err != nil { | |
| 494 | + t.Fatalf("Load: %v", err) | |
| 495 | + } | |
| 496 | + want.Version = Version | |
| 497 | + if !got.Equal(want) { | |
| 498 | + t.Errorf("round trip changed the servers:\n got %+v\nwant %+v", got.MCPServers, want.MCPServers) | |
| 499 | + } | |
| 500 | + } | |
| 501 | + | |
| 502 | + func TestEqualSeesMCPServers(t *testing.T) { | |
| 503 | + base := Config{MCPServers: map[string]MCPServer{"db": {Command: "db-mcp", Args: []string{"--ro"}}}} | |
| 504 | + cases := []struct { | |
| 505 | + name string | |
| 506 | + b Config | |
| 507 | + equal bool | |
| 508 | + }{ | |
| 509 | + {"the same server in another map", Config{MCPServers: map[string]MCPServer{"db": {Command: "db-mcp", Args: []string{"--ro"}}}}, true}, | |
| 510 | + {"a changed argument", Config{MCPServers: map[string]MCPServer{"db": {Command: "db-mcp", Args: []string{"--rw"}}}}, false}, | |
| 511 | + {"a changed environment", Config{MCPServers: map[string]MCPServer{"db": {Command: "db-mcp", Args: []string{"--ro"}, Env: map[string]string{"A": "1"}}}}, false}, | |
| 512 | + {"a server dropped", Config{}, false}, | |
| 513 | + {"a server renamed", Config{MCPServers: map[string]MCPServer{"sql": {Command: "db-mcp", Args: []string{"--ro"}}}}, false}, | |
| 514 | + } | |
| 515 | + for _, c := range cases { | |
| 516 | + t.Run(c.name, func(t *testing.T) { | |
| 517 | + if got := base.Equal(c.b); got != c.equal { | |
| 518 | + t.Errorf("Equal = %v, want %v", got, c.equal) | |
| 519 | + } | |
| 520 | + if got := c.b.Equal(base); got != c.equal { | |
| 521 | + t.Errorf("Equal is not symmetric: reversed gave %v, want %v", got, c.equal) | |
| 522 | + } | |
| 523 | + }) | |
| 524 | + } | |
| 525 | + // Written to the file, a missing map and an empty one are the same | |
| 526 | + // nothing, as are no args and an empty args list. | |
| 527 | + if !(Config{}).Equal(Config{MCPServers: map[string]MCPServer{}}) { | |
| 528 | + t.Error("an empty server map should equal a missing one") | |
| 529 | + } | |
| 530 | + if !(Config{MCPServers: map[string]MCPServer{"x": {Command: "x"}}}).Equal(Config{MCPServers: map[string]MCPServer{"x": {Command: "x", Args: []string{}}}}) { | |
| 531 | + t.Error("no args should equal an empty args list") | |
| 532 | + } | |
| 533 | + } | |
| @@ -0,0 +1,429 @@ | ||
| 1 | + // Package diag is what checks a file the agent has just written. | |
| 2 | + // | |
| 3 | + // The agent edits a file and, today, learns that it broke the build three tool | |
| 4 | + // calls later, when it runs the tests or the user complains. The compiler | |
| 5 | + // already knew at the moment of the edit. This package runs whatever this | |
| 6 | + // machine has for the language — a language server if one is installed, a | |
| 7 | + // linter otherwise — against the file that just changed, and hands what it | |
| 8 | + // said back to the edit and write tools so it lands in the result the model is | |
| 9 | + // already reading. Nothing new is installed and nothing leaves the machine. | |
| 10 | + // | |
| 11 | + // It is one package rather than part of internal/tools for the same reason | |
| 12 | + // websearch is: the tools call Report and everything about which programs are | |
| 13 | + // run, how long they may take and how their output is read lives here, where | |
| 14 | + // it can be read in one sitting. It imports no other internal package. | |
| 15 | + package diag | |
| 16 | + | |
| 17 | + import ( | |
| 18 | + "context" | |
| 19 | + "fmt" | |
| 20 | + "os/exec" | |
| 21 | + "path/filepath" | |
| 22 | + "sort" | |
| 23 | + "strings" | |
| 24 | + "sync" | |
| 25 | + "time" | |
| 26 | + ) | |
| 27 | + | |
| 28 | + // checkTimeout is how long one check may take, start to finish, including a | |
| 29 | + // language server that is still starting up. | |
| 30 | + // | |
| 31 | + // The model waits through this on every edit, so it is a fraction of the bash | |
| 32 | + // tool's minute rather than a multiple of it: two seconds of latency per edit | |
| 33 | + // is the price of the compiler's opinion, and a check that has not finished by | |
| 34 | + // then is a check to abandon. It is short enough that gopls on a cold cache | |
| 35 | + // will usually miss it on the very first edit of a session — that is | |
| 36 | + // accepted, because the server keeps loading in the background and every edit | |
| 37 | + // after that gets an answer in well under a second. | |
| 38 | + const checkTimeout = 2 * time.Second | |
| 39 | + | |
| 40 | + // settleWindow is how long, after the first batch of diagnostics for the | |
| 41 | + // changed file arrives, a check goes on listening for more. | |
| 42 | + // | |
| 43 | + // A language server does not answer a didOpen with one tidy response. gopls | |
| 44 | + // type-checks the package and publishes a batch per file in it, and some | |
| 45 | + // servers publish an empty batch first to clear stale marks and the real one a | |
| 46 | + // moment later. Returning on the first batch would report a clean file that is | |
| 47 | + // about to be reported broken; waiting for the deadline would make every clean | |
| 48 | + // edit cost the full timeout. A short window after the first batch collects | |
| 49 | + // the neighbours and the correction without paying for silence. | |
| 50 | + const settleWindow = 250 * time.Millisecond | |
| 51 | + | |
| 52 | + // startCooldown is how long a server that failed to start is left alone. Long | |
| 53 | + // enough that a broken install is not retried on every edit; short enough | |
| 54 | + // that fixing the install mid-session takes effect without a restart. | |
| 55 | + const startCooldown = 5 * time.Minute | |
| 56 | + | |
| 57 | + // maxReported is how many diagnostics are shown to the model. Ten covers a | |
| 58 | + // real mistake in full; a hundred is a file so broken that the model needs to | |
| 59 | + // read it again rather than a list. | |
| 60 | + const maxReported = 10 | |
| 61 | + | |
| 62 | + // Severity is how serious a diagnostic is, using the LSP's four levels. | |
| 63 | + type Severity int | |
| 64 | + | |
| 65 | + // Severities, from worst to least. The numbering is the LSP's so that a | |
| 66 | + // server's value can be used as it stands. | |
| 67 | + const ( | |
| 68 | + SeverityError Severity = 1 | |
| 69 | + SeverityWarning Severity = 2 | |
| 70 | + SeverityInfo Severity = 3 | |
| 71 | + SeverityHint Severity = 4 | |
| 72 | + ) | |
| 73 | + | |
| 74 | + // String returns the lower-case name a diagnostic line carries. | |
| 75 | + func (s Severity) String() string { | |
| 76 | + switch s { | |
| 77 | + case SeverityError: | |
| 78 | + return "error" | |
| 79 | + case SeverityWarning: | |
| 80 | + return "warning" | |
| 81 | + case SeverityInfo: | |
| 82 | + return "info" | |
| 83 | + case SeverityHint: | |
| 84 | + return "hint" | |
| 85 | + } | |
| 86 | + return fmt.Sprintf("severity(%d)", int(s)) | |
| 87 | + } | |
| 88 | + | |
| 89 | + // Diagnostic is one thing a checker had to say about a file. It is the common | |
| 90 | + // shape every backend is parsed into, so the formatting and the cap are written | |
| 91 | + // once. | |
| 92 | + type Diagnostic struct { | |
| 93 | + // File is the absolute path the diagnostic is about. It is usually the | |
| 94 | + // file that was checked, but a package-level tool reports neighbours too. | |
| 95 | + File string | |
| 96 | + // Line and Col are 1-based. Col is 0 when the tool did not say. | |
| 97 | + Line, Col int | |
| 98 | + Severity Severity | |
| 99 | + Message string | |
| 100 | + // Source is which program said it — "gopls", "ruff" — so the model knows | |
| 101 | + // how much to trust it and the user knows what to install or uninstall. | |
| 102 | + Source string | |
| 103 | + } | |
| 104 | + | |
| 105 | + // Checker is one way of checking a file. The two implementations are a | |
| 106 | + // language server and a linter command; the interface exists so the Runner | |
| 107 | + // picks between them without caring which it got, and so tests can hand it a | |
| 108 | + // fake. | |
| 109 | + type Checker interface { | |
| 110 | + // Check returns what the backend has to say about the file at path. A nil | |
| 111 | + // slice with a nil error is a clean file. An error means the backend could | |
| 112 | + // not run — not installed, timed out, unparseable output — and is never | |
| 113 | + // shown to the model; it is silence with a reason. | |
| 114 | + Check(ctx context.Context, path string) ([]Diagnostic, error) | |
| 115 | + } | |
| 116 | + | |
| 117 | + // Runner is the one object the tools hold. It owns the language servers that | |
| 118 | + // stay running between edits and chooses, per file, between a server and a | |
| 119 | + // linter. A nil *Runner is valid and does nothing, which is how the feature is | |
| 120 | + // turned off: every method tolerates a nil receiver so the call sites need no | |
| 121 | + // switch of their own. | |
| 122 | + type Runner struct { | |
| 123 | + mu sync.Mutex | |
| 124 | + servers map[string]*session | |
| 125 | + // cooldown records, per server key, when it may next be started. A | |
| 126 | + // server that failed to start is not tried again for a while, and in the | |
| 127 | + // meantime the language's linter is used instead; without this a broken | |
| 128 | + // gopls would be respawned on every Go edit and each edit would wait the | |
| 129 | + // full deadline for it. | |
| 130 | + cooldown map[string]time.Time | |
| 131 | + closed bool | |
| 132 | + | |
| 133 | + // lookPath is exec.LookPath unless a test has replaced it, so the choice | |
| 134 | + // between server and linter can be exercised on a machine with neither. | |
| 135 | + lookPath func(string) (string, error) | |
| 136 | + // idle is how long a server may sit unused before it is shut down. | |
| 137 | + idle time.Duration | |
| 138 | + } | |
| 139 | + | |
| 140 | + // NewRunner returns a Runner with no servers started. Servers are started on | |
| 141 | + // the first file that needs one, so a session that never touches Go never pays | |
| 142 | + // for gopls. | |
| 143 | + func NewRunner() *Runner { | |
| 144 | + return &Runner{ | |
| 145 | + servers: map[string]*session{}, | |
| 146 | + cooldown: map[string]time.Time{}, | |
| 147 | + lookPath: exec.LookPath, | |
| 148 | + idle: idleTimeout, | |
| 149 | + } | |
| 150 | + } | |
| 151 | + | |
| 152 | + // Report checks path and returns the block of text to append to a tool result. | |
| 153 | + // root is what the file names in that text are made relative to — the working | |
| 154 | + // root the model already thinks in. | |
| 155 | + // | |
| 156 | + // It returns "" when there is nothing to say, and that covers both a clean | |
| 157 | + // file and a check that could not run. Silence is the good news, and a "0 | |
| 158 | + // problems" line on every edit would be pure token cost, paid on the great | |
| 159 | + // majority of edits, for information the absence of a block already carries. | |
| 160 | + // A backend failure is silent for the same reason a missing backend is: the | |
| 161 | + // edit succeeded, and the model has nothing to do about a linter that did not | |
| 162 | + // start. | |
| 163 | + func (r *Runner) Report(ctx context.Context, root, path string) (out string) { | |
| 164 | + if r == nil { | |
| 165 | + return "" | |
| 166 | + } | |
| 167 | + // The edit has already been applied by the time this runs. A bug in here | |
| 168 | + // must not turn a successful edit into a crashed session, so this is the | |
| 169 | + // one place in the harness a panic is caught rather than propagated. | |
| 170 | + defer func() { | |
| 171 | + if rec := recover(); rec != nil { | |
| 172 | + out = "" | |
| 173 | + } | |
| 174 | + }() | |
| 175 | + ctx, cancel := context.WithTimeout(ctx, checkTimeout) | |
| 176 | + defer cancel() | |
| 177 | + ds, err := r.Check(ctx, path) | |
| 178 | + if err != nil || len(ds) == 0 { | |
| 179 | + return "" | |
| 180 | + } | |
| 181 | + return Format(ds, root, path) | |
| 182 | + } | |
| 183 | + | |
| 184 | + // Check runs the best available checker for path. It picks a language server | |
| 185 | + // when one for the file's language is on PATH, otherwise the first installed | |
| 186 | + // linter for its extension. It satisfies Checker so a Runner can stand in | |
| 187 | + // wherever one is wanted. | |
| 188 | + func (r *Runner) Check(ctx context.Context, path string) ([]Diagnostic, error) { | |
| 189 | + if r == nil { | |
| 190 | + return nil, nil | |
| 191 | + } | |
| 192 | + path, err := filepath.Abs(path) | |
| 193 | + if err != nil { | |
| 194 | + return nil, err | |
| 195 | + } | |
| 196 | + if s := r.server(path); s != nil { | |
| 197 | + return s.Check(ctx, path) | |
| 198 | + } | |
| 199 | + if l := r.linter(path); l != nil { | |
| 200 | + return l.Check(ctx, path) | |
| 201 | + } | |
| 202 | + return nil, errNoChecker | |
| 203 | + } | |
| 204 | + | |
| 205 | + // Close shuts down every language server the Runner has started. It is safe to | |
| 206 | + // call on a nil Runner and more than once. Servers are shut down concurrently | |
| 207 | + // with a short deadline each, because a server that ignores shutdown must not | |
| 208 | + // hold up exit. | |
| 209 | + func (r *Runner) Close() { | |
| 210 | + if r == nil { | |
| 211 | + return | |
| 212 | + } | |
| 213 | + r.mu.Lock() | |
| 214 | + r.closed = true | |
| 215 | + servers := r.servers | |
| 216 | + r.servers = map[string]*session{} | |
| 217 | + r.mu.Unlock() | |
| 218 | + | |
| 219 | + var wg sync.WaitGroup | |
| 220 | + for _, s := range servers { | |
| 221 | + wg.Add(1) | |
| 222 | + go func(s *session) { | |
| 223 | + defer wg.Done() | |
| 224 | + s.Close() | |
| 225 | + }(s) | |
| 226 | + } | |
| 227 | + wg.Wait() | |
| 228 | + } | |
| 229 | + | |
| 230 | + // server returns the running or starting language server for path's language | |
| 231 | + // and workspace, starting one if the program is installed and none is running. | |
| 232 | + // It returns nil when no server applies. | |
| 233 | + func (r *Runner) server(path string) *session { | |
| 234 | + s, _ := r.lookup(path) | |
| 235 | + return s | |
| 236 | + } | |
| 237 | + | |
| 238 | + // lookup is server with the reason for a nil. Check has no use for the reason | |
| 239 | + // — a file it cannot check is silence either way — but a query the model | |
| 240 | + // made by name deserves to hear whether the answer is "install gopls" or | |
| 241 | + // "this is a Markdown file", and those are different things to do next. | |
| 242 | + func (r *Runner) lookup(path string) (*session, error) { | |
| 243 | + spec, ok := serverFor(path) | |
| 244 | + if !ok { | |
| 245 | + what := strings.ToLower(filepath.Ext(path)) | |
| 246 | + if what == "" { | |
| 247 | + what = filepath.Base(path) | |
| 248 | + } | |
| 249 | + return nil, fmt.Errorf("%w (%s)", ErrNoServer, what) | |
| 250 | + } | |
| 251 | + bin, err := r.lookPath(spec.Bin) | |
| 252 | + if err != nil { | |
| 253 | + return nil, fmt.Errorf("%w: %s handles %s files but is not on PATH", ErrNotInstalled, spec.Bin, strings.ToLower(filepath.Ext(path))) | |
| 254 | + } | |
| 255 | + root := workspaceRoot(path, spec.Markers) | |
| 256 | + key := root + "\x00" + spec.Bin | |
| 257 | + | |
| 258 | + r.mu.Lock() | |
| 259 | + defer r.mu.Unlock() | |
| 260 | + if r.closed { | |
| 261 | + return nil, fmt.Errorf("%s: %w", spec.Bin, errServerDead) | |
| 262 | + } | |
| 263 | + if s, ok := r.servers[key]; ok && !s.Dead() { | |
| 264 | + return s, nil | |
| 265 | + } | |
| 266 | + if until, ok := r.cooldown[key]; ok && time.Now().Before(until) { | |
| 267 | + return nil, fmt.Errorf("%s: %w; not retried for another %s", spec.Bin, ErrStartFailed, time.Until(until).Round(time.Second)) | |
| 268 | + } | |
| 269 | + s := startServer(bin, spec, root, r.idle, func(s *session) { r.exited(key, s) }) | |
| 270 | + if s == nil { | |
| 271 | + r.cooldown[key] = time.Now().Add(startCooldown) | |
| 272 | + return nil, fmt.Errorf("%s: %w", spec.Bin, ErrStartFailed) | |
| 273 | + } | |
| 274 | + r.servers[key] = s | |
| 275 | + return s, nil | |
| 276 | + } | |
| 277 | + | |
| 278 | + // exited is called once a server's process has gone — an idle shutdown, a | |
| 279 | + // crash, a refusal to start. It drops the server from the table so the next | |
| 280 | + // file in that language starts a fresh one rather than talking to a corpse, | |
| 281 | + // and if this one never got as far as answering initialize, puts the key on | |
| 282 | + // cooldown. The identity check matters: a replacement may already be | |
| 283 | + // registered under the same key, and forgetting that one would leak it. | |
| 284 | + func (r *Runner) exited(key string, s *session) { | |
| 285 | + r.mu.Lock() | |
| 286 | + defer r.mu.Unlock() | |
| 287 | + if r.servers[key] == s { | |
| 288 | + delete(r.servers, key) | |
| 289 | + } | |
| 290 | + if s.failedToStart() { | |
| 291 | + r.cooldown[key] = time.Now().Add(startCooldown) | |
| 292 | + } | |
| 293 | + } | |
| 294 | + | |
| 295 | + // linter returns the first installed linter for path's extension, or nil. | |
| 296 | + func (r *Runner) linter(path string) Checker { | |
| 297 | + for _, l := range lintersFor(path) { | |
| 298 | + if _, err := r.lookPath(l.bin); err == nil { | |
| 299 | + return l | |
| 300 | + } | |
| 301 | + } | |
| 302 | + return nil | |
| 303 | + } | |
| 304 | + | |
| 305 | + // errNoChecker is what Check returns when nothing on this machine can check | |
| 306 | + // the file. It is an error rather than an empty result so a caller that wants | |
| 307 | + // to tell "clean" from "unchecked" can. | |
| 308 | + var errNoChecker = fmt.Errorf("no language server or linter installed for this file") | |
| 309 | + | |
| 310 | + // Format renders diagnostics for the model, or "" when there are none. File | |
| 311 | + // names are made relative to root; path is the file that was checked, whose | |
| 312 | + // diagnostics are listed first so the cap drops a neighbour's problems before | |
| 313 | + // the model's own. | |
| 314 | + func Format(ds []Diagnostic, root, path string) string { | |
| 315 | + ds = keep(ds) | |
| 316 | + if len(ds) == 0 { | |
| 317 | + return "" | |
| 318 | + } | |
| 319 | + sortForReport(ds, path) | |
| 320 | + | |
| 321 | + source := ds[0].Source | |
| 322 | + for _, d := range ds[1:] { | |
| 323 | + if d.Source != source { | |
| 324 | + source = "mixed" | |
| 325 | + break | |
| 326 | + } | |
| 327 | + } | |
| 328 | + var b strings.Builder | |
| 329 | + fmt.Fprintf(&b, "diagnostics (%s): %s", source, plural(len(ds), "problem")) | |
| 330 | + shown := ds | |
| 331 | + if len(shown) > maxReported { | |
| 332 | + shown = shown[:maxReported] | |
| 333 | + } | |
| 334 | + for _, d := range shown { | |
| 335 | + b.WriteString("\n ") | |
| 336 | + b.WriteString(relative(root, d.File)) | |
| 337 | + if d.Line > 0 { | |
| 338 | + fmt.Fprintf(&b, ":%d", d.Line) | |
| 339 | + if d.Col > 0 { | |
| 340 | + fmt.Fprintf(&b, ":%d", d.Col) | |
| 341 | + } | |
| 342 | + } | |
| 343 | + b.WriteString(": ") | |
| 344 | + // Errors are the default reading of a diagnostic line, so only the | |
| 345 | + // lesser levels are labelled; the label is what tells the model it | |
| 346 | + // may leave the line alone. | |
| 347 | + if d.Severity != SeverityError && d.Severity != 0 { | |
| 348 | + b.WriteString(d.Severity.String()) | |
| 349 | + b.WriteString(": ") | |
| 350 | + } | |
| 351 | + b.WriteString(oneLine(d.Message)) | |
| 352 | + } | |
| 353 | + if dropped := len(ds) - len(shown); dropped > 0 { | |
| 354 | + fmt.Fprintf(&b, "\n … and %d more not shown", dropped) | |
| 355 | + } | |
| 356 | + return b.String() | |
| 357 | + } | |
| 358 | + | |
| 359 | + // keep drops the levels not worth the model's attention. Hints are a server's | |
| 360 | + // stylistic asides — gopls's "could be simplified", pyright's greyed-out | |
| 361 | + // unused import — and a model that fixes every hint it is shown rewrites code | |
| 362 | + // it was not asked to touch. | |
| 363 | + func keep(ds []Diagnostic) []Diagnostic { | |
| 364 | + out := make([]Diagnostic, 0, len(ds)) | |
| 365 | + for _, d := range ds { | |
| 366 | + if d.Severity == SeverityHint { | |
| 367 | + continue | |
| 368 | + } | |
| 369 | + if strings.TrimSpace(d.Message) == "" { | |
| 370 | + continue | |
| 371 | + } | |
| 372 | + out = append(out, d) | |
| 373 | + } | |
| 374 | + return out | |
| 375 | + } | |
| 376 | + | |
| 377 | + // sortForReport orders the changed file first, then by severity, then by | |
| 378 | + // position, so the cap cuts from the least important end. | |
| 379 | + func sortForReport(ds []Diagnostic, path string) { | |
| 380 | + sort.SliceStable(ds, func(i, j int) bool { | |
| 381 | + a, b := ds[i], ds[j] | |
| 382 | + if (a.File == path) != (b.File == path) { | |
| 383 | + return a.File == path | |
| 384 | + } | |
| 385 | + if a.File != b.File { | |
| 386 | + return a.File < b.File | |
| 387 | + } | |
| 388 | + if a.Severity != b.Severity { | |
| 389 | + return a.Severity < b.Severity | |
| 390 | + } | |
| 391 | + if a.Line != b.Line { | |
| 392 | + return a.Line < b.Line | |
| 393 | + } | |
| 394 | + return a.Col < b.Col | |
| 395 | + }) | |
| 396 | + } | |
| 397 | + | |
| 398 | + // relative is path as the model would write it: relative to root when it is | |
| 399 | + // under root, and as it stands otherwise. A neighbour above the working root | |
| 400 | + // — a go.mod one directory up — is shown with its ../ rather than hidden. | |
| 401 | + func relative(root, path string) string { | |
| 402 | + if root == "" { | |
| 403 | + return path | |
| 404 | + } | |
| 405 | + rel, err := filepath.Rel(root, path) | |
| 406 | + if err != nil { | |
| 407 | + return path | |
| 408 | + } | |
| 409 | + return filepath.ToSlash(rel) | |
| 410 | + } | |
| 411 | + | |
| 412 | + // oneLine folds a multi-line message onto one line. Compiler messages are | |
| 413 | + // almost always one line; the ones that are not (rustc's, with its notes and | |
| 414 | + // help) would otherwise break the one-problem-per-line shape the model is | |
| 415 | + // parsing by eye. | |
| 416 | + func oneLine(s string) string { | |
| 417 | + s = strings.TrimSpace(s) | |
| 418 | + if !strings.ContainsAny(s, "\r\n") { | |
| 419 | + return s | |
| 420 | + } | |
| 421 | + return strings.Join(strings.Fields(s), " ") | |
| 422 | + } | |
| 423 | + | |
| 424 | + func plural(n int, noun string) string { | |
| 425 | + if n == 1 { | |
| 426 | + return fmt.Sprintf("1 %s", noun) | |
| 427 | + } | |
| 428 | + return fmt.Sprintf("%d %ss", n, noun) | |
| 429 | + } | |
| @@ -0,0 +1,98 @@ | ||
| 1 | + package diag | |
| 2 | + | |
| 3 | + import ( | |
| 4 | + "fmt" | |
| 5 | + "strings" | |
| 6 | + "testing" | |
| 7 | + ) | |
| 8 | + | |
| 9 | + func TestFormatNothingForACleanFile(t *testing.T) { | |
| 10 | + if got := Format(nil, "/r", "/r/a.go"); got != "" { | |
| 11 | + t.Errorf("nil → %q", got) | |
| 12 | + } | |
| 13 | + if got := Format([]Diagnostic{}, "/r", "/r/a.go"); got != "" { | |
| 14 | + t.Errorf("empty → %q", got) | |
| 15 | + } | |
| 16 | + // Hints alone are silence too. | |
| 17 | + only := []Diagnostic{{File: "/r/a.go", Line: 1, Severity: SeverityHint, Message: "simplify", Source: "gopls"}} | |
| 18 | + if got := Format(only, "/r", "/r/a.go"); got != "" { | |
| 19 | + t.Errorf("hints only → %q", got) | |
| 20 | + } | |
| 21 | + } | |
| 22 | + | |
| 23 | + func TestFormatMatchesTheShapeTheModelReads(t *testing.T) { | |
| 24 | + ds := []Diagnostic{ | |
| 25 | + {File: "/r/main.go", Line: 19, Col: 6, Severity: SeverityError, Message: "declared and not used: n", Source: "gopls"}, | |
| 26 | + {File: "/r/main.go", Line: 14, Col: 2, Severity: SeverityError, Message: "undefined: fmt.Printl", Source: "gopls"}, | |
| 27 | + } | |
| 28 | + got := Format(ds, "/r", "/r/main.go") | |
| 29 | + want := "diagnostics (gopls): 2 problems\n" + | |
| 30 | + " main.go:14:2: undefined: fmt.Printl\n" + | |
| 31 | + " main.go:19:6: declared and not used: n" | |
| 32 | + if got != want { | |
| 33 | + t.Errorf("got:\n%s\nwant:\n%s", got, want) | |
| 34 | + } | |
| 35 | + } | |
| 36 | + | |
| 37 | + func TestFormatSingularWarningsAndMissingColumns(t *testing.T) { | |
| 38 | + ds := []Diagnostic{{File: "/r/x.py", Line: 3, Severity: SeverityWarning, Message: "E501 Line too long", Source: "ruff"}} | |
| 39 | + got := Format(ds, "/r", "/r/x.py") | |
| 40 | + want := "diagnostics (ruff): 1 problem\n x.py:3: warning: E501 Line too long" | |
| 41 | + if got != want { | |
| 42 | + t.Errorf("got %q, want %q", got, want) | |
| 43 | + } | |
| 44 | + } | |
| 45 | + | |
| 46 | + func TestFormatChangedFileFirstThenCap(t *testing.T) { | |
| 47 | + var ds []Diagnostic | |
| 48 | + for i := 1; i <= 12; i++ { | |
| 49 | + ds = append(ds, Diagnostic{File: "/r/other.go", Line: i, Col: 1, Severity: SeverityError, Message: fmt.Sprintf("other %d", i), Source: "go build"}) | |
| 50 | + } | |
| 51 | + ds = append(ds, | |
| 52 | + Diagnostic{File: "/r/mine.go", Line: 9, Col: 1, Severity: SeverityWarning, Message: "mine warn", Source: "go build"}, | |
| 53 | + Diagnostic{File: "/r/mine.go", Line: 2, Col: 1, Severity: SeverityError, Message: "mine err", Source: "go build"}, | |
| 54 | + ) | |
| 55 | + got := Format(ds, "/r", "/r/mine.go") | |
| 56 | + lines := strings.Split(got, "\n") | |
| 57 | + if lines[0] != "diagnostics (go build): 14 problems" { | |
| 58 | + t.Errorf("header: %q", lines[0]) | |
| 59 | + } | |
| 60 | + if lines[1] != " mine.go:2:1: mine err" || lines[2] != " mine.go:9:1: warning: mine warn" { | |
| 61 | + t.Errorf("the changed file should come first, errors before warnings:\n%s", got) | |
| 62 | + } | |
| 63 | + if !strings.HasPrefix(lines[3], " other.go:1:1:") { | |
| 64 | + t.Errorf("neighbours after, labelled: %q", lines[3]) | |
| 65 | + } | |
| 66 | + // header + 10 shown + the dropped line | |
| 67 | + if len(lines) != 12 { | |
| 68 | + t.Errorf("%d lines:\n%s", len(lines), got) | |
| 69 | + } | |
| 70 | + if lines[11] != " … and 4 more not shown" { | |
| 71 | + t.Errorf("dropped line: %q", lines[11]) | |
| 72 | + } | |
| 73 | + } | |
| 74 | + | |
| 75 | + func TestFormatMixedSourcesAndPathsOutsideRoot(t *testing.T) { | |
| 76 | + ds := []Diagnostic{ | |
| 77 | + {File: "/r/sub/a.go", Line: 1, Col: 1, Severity: SeverityError, Message: "a", Source: "go build"}, | |
| 78 | + {File: "/elsewhere/b.go", Line: 1, Col: 1, Severity: SeverityError, Message: "line one\n note: line two", Source: "go vet"}, | |
| 79 | + } | |
| 80 | + got := Format(ds, "/r", "/r/sub/a.go") | |
| 81 | + if !strings.HasPrefix(got, "diagnostics (mixed): 2 problems") { | |
| 82 | + t.Errorf("header: %q", got) | |
| 83 | + } | |
| 84 | + if !strings.Contains(got, "\n sub/a.go:1:1: a") { | |
| 85 | + t.Errorf("relative path: %q", got) | |
| 86 | + } | |
| 87 | + if !strings.Contains(got, "\n ../elsewhere/b.go:1:1: line one note: line two") { | |
| 88 | + t.Errorf("outside-root path and folded message: %q", got) | |
| 89 | + } | |
| 90 | + } | |
| 91 | + | |
| 92 | + func TestSeverityString(t *testing.T) { | |
| 93 | + for s, want := range map[Severity]string{SeverityError: "error", SeverityWarning: "warning", SeverityInfo: "info", SeverityHint: "hint", 9: "severity(9)"} { | |
| 94 | + if got := s.String(); got != want { | |
| 95 | + t.Errorf("%d → %q, want %q", int(s), got, want) | |
| 96 | + } | |
| 97 | + } | |
| 98 | + } | |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.