diff --git a/docs/plan.md b/docs/plan.md index e13e63a..12264f4 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -54,8 +54,8 @@ The first milestone is a walking skeleton (`skeleton`): one feature on a throwaw | `bash` | `internal/bash`: the commands that a bash script runs, each as the words that bash passes it, read with mvdan.cc/sh's parser: those in substitutions, here-documents and functions too, the command that a wrapper such as `env`, `nohup` or `xargs` runs, and the commands of the scripts of `sh -c` and `eval` (decision 50, fact 33) | Done (#31) | | `guard` | `internal/guard`: the commands of a bash call that the guard watches, by program: git's commands that send to another host, such as `git push`, `gh`'s commands but those that only read, `ssh`, `scp`, `sftp` and Jira's CLIs; `tmux` and `osascript`; another agent's CLI; `baton`, with its command; and a program that only the run decides (decision 51, fact 34) | Done (#32) | | `guard-http` | `internal/guard` reads the options of `curl`, `wget` and `gh api`, and watches only those that send data (decision 51, fact 35) | Done (#33) | -| `guard-hook` | `baton hook guard` with its `preToolUse` hook in the plugin: in a role's pane, it denies, with a reason for the role, a bash call that the guard can't read or that runs a command that the guard watches, but baton's commands for a role (`baton wait`, `handover`, `ask`, `status` and help), and a `create` or `edit` of a file in baton's folder, however its path names it; it says nothing in the orchestrator's pane yet (decisions 9, 17, 48 and 52; fact 36, risk 11) | In review (#34) | -| `plugin-check` | `baton pane` checks the files of the plugin folder that it finds in its cache before it uses it, and `baton hook guard` denies a role's `create` and `edit` in baton's cache, so that a role can't turn the guard off for the panes that start after it (decisions 36 and 52, risk 12) | Todo | +| `guard-hook` | `baton hook guard` with its `preToolUse` hook in the plugin: in a role's pane, it denies, with a reason for the role, a bash call that the guard can't read or that runs a command that the guard watches, but baton's commands for a role (`baton wait`, `handover`, `ask`, `status` and help), and a `create` or `edit` of a file in baton's folder, however its path names it; it says nothing in the orchestrator's pane yet (decisions 9, 17, 48 and 52; fact 36, risk 11) | Done (#34) | +| `plugin-check` | `baton pane` checks the files of the plugin folder that it finds in its cache before each `copilot` starts, a role's after its wait too, and writes the plugin under the next name beside a changed folder; `baton hook guard` denies a role's `create` and `edit` in baton's cache, so that a role can't turn the guard off for the panes that start after it (decisions 36, 52 and 53, risk 12) | In review (#35) | | `guard-orchestrator` | In the orchestrator's pane, `baton hook guard` asks the user before outward actions, and before `baton feature start` while plan approval is on, and denies `edit` and `create` outside its session's folder; the orchestrator then runs with `--allow-all` (decisions 8, 19 and 45) | Todo | | `belt` | Risk 2's second belt, if it works: the role panes' credentials taken away, such as an empty `GH_CONFIG_DIR` and a git `pushInsteadOf` that fails | Todo | | `gates` | The engineer's handover runs the repo's gate command and refuses a red one | Todo | @@ -172,7 +172,7 @@ copilot --agent baton: --plugin-dir /plugin- --model -i "" ``` -`copilot` keeps the pane's terminal and environment, so the role's `baton` commands read `BATON_ROLE` and `BATON_FEATURE` there, and gets `COPILOT_ALLOW_ALL=true` too, which trusts the worktree for that session (decision 39), and `BATON_BIN`, the path of the pane's baton, which the plugin's `agentStop` hook runs (decision 47). The plugin comes from the binary: `baton pane` writes it into ``, the `baton` folder in the user's cache folder (`~/Library/Caches/baton` on a Mac), unless it's there already, in a folder named by a hash of its files (decision 36). baton keeps each role's session ID, so it can find the role's events and resume the session. +`copilot` keeps the pane's terminal and environment, so the role's `baton` commands read `BATON_ROLE` and `BATON_FEATURE` there, and gets `COPILOT_ALLOW_ALL=true` too, which trusts the worktree for that session (decision 39), and `BATON_BIN`, the path of the pane's baton, which the plugin's `agentStop` hook runs (decision 47). The plugin comes from the binary: `baton pane` writes it into ``, the `baton` folder in the user's cache folder (`~/Library/Caches/baton` on a Mac), unless it's there already, unchanged, in a folder named by a hash of its files (decision 36); a role's pane checks it again after the wait (decision 53). baton keeps each role's session ID, so it can find the role's events and resume the session. **The orchestrator's session** (decisions 44, 45 and 49). `baton start`, or plain `baton`, in the user's shell, makes the tmux session `baton-` in the repo's main worktree, unless it runs already, with one pane titled orchestrator, which runs `baton pane` with `BATON_ROLE=orchestrator` and no `BATON_FEATURE`. It reads the config file before it makes the session, so a mistake there shows in the user's terminal, not in a pane that takes its session with it when it fails. Then it shows the session: outside tmux, baton becomes `tmux attach-session`, and inside tmux, it switches the user's client to the session. The orchestrator's `baton pane` writes the plugin, reads its settings, and becomes its `copilot` at once, with no first prompt, since the user speaks first, and with the user's environment, plus `BATON_BIN` (decision 47): @@ -189,7 +189,7 @@ It keeps copilot's own prompts, since the user watches that pane, `ask_user`, si **Questions** (decisions 18 and 46). A role never asks the user. The holder starts `baton ask ""` in a background shell and ends its turn, as with `baton wait`: baton stores the question in the feature's file and logs it, then reads the file every second until the answer comes, prints it and exits, and its end wakes the role (fact 5). The orchestrator's `baton wait` picks up the question, as it picks up its work, and prints it with the command that answers it. The orchestrator answers from the plan with `baton answer ""`, or asks the user first, in its pane or through nudge. A role asks one question at a time, and can't hand over while it waits for the answer. Asking the same question again doesn't ask it twice: it waits for the answer, or prints it if it came, so a role whose `baton ask` ended early still gets its answer. The role's handover clears its question, which the journal keeps. -**Safety** (decisions 9, 17, 19 and 39). The roles run with every permission (`--allow-all`), in a worktree that their `copilot` trusts, and no `ask_user`, so nothing waits on a prompt in a pane nobody watches, and with the tool allow-list, so no hidden agent and none of the user's skills or MCP tools. The guard hook (`preToolUse` → `baton hook guard`) denies outward actions in a role's pane: `git push`, pull request and merge commands, `gh api` writes, HTTP writes with `curl` and `wget`, Jira writes and `ssh`; and `tmux`, `osascript`, another agent's CLI, baton's commands but a role's own (`baton wait`, `handover`, `ask` and `status`), a program that only the run decides, and `edit` and `create` in baton's folder (decisions 51 and 52). It stops a role's mistakes, not a role that sets out to get past it (risks 2, 11 and 12). In the orchestrator's pane, it answers `ask` for outward actions, so the user approves each one, and for `baton feature start` while plan approval is on (decision 8); and it denies `edit` and `create` outside its session's folder, where it writes its briefs and notes (decision 48), so the orchestrator plans and dispatches but doesn't write the code. The guard reads the commands of a bash call with a bash parser (decision 50), not its text, and quickly, because a hook that times out allows (fact 4). Until the guard comes, the orchestrator runs without `--allow-all`, so `copilot` asks the user before each of its tools acts (decision 45). +**Safety** (decisions 9, 17, 19 and 39). The roles run with every permission (`--allow-all`), in a worktree that their `copilot` trusts, and no `ask_user`, so nothing waits on a prompt in a pane nobody watches, and with the tool allow-list, so no hidden agent and none of the user's skills or MCP tools. The guard hook (`preToolUse` → `baton hook guard`) denies outward actions in a role's pane: `git push`, pull request and merge commands, `gh api` writes, HTTP writes with `curl` and `wget`, Jira writes and `ssh`; and `tmux`, `osascript`, another agent's CLI, baton's commands but a role's own (`baton wait`, `handover`, `ask` and `status`), a program that only the run decides, and `edit` and `create` in baton's folder and in its cache, which holds the plugin (decisions 51 to 53). It stops a role's mistakes, not a role that sets out to get past it (risks 2, 11 and 12). In the orchestrator's pane, it answers `ask` for outward actions, so the user approves each one, and for `baton feature start` while plan approval is on (decision 8); and it denies `edit` and `create` outside its session's folder, where it writes its briefs and notes (decision 48), so the orchestrator plans and dispatches but doesn't write the code. The guard reads the commands of a bash call with a bash parser (decision 50), not its text, and quickly, because a hook that times out allows (fact 4). Until the guard comes, the orchestrator runs without `--allow-all`, so `copilot` asks the user before each of its tools acts (decision 45). **State** (decision 15). Everything lives in `$(git rev-parse --git-common-dir)/baton/`, which every worktree of the repo shares and git never commits: @@ -255,7 +255,7 @@ Decisions marked **lead's call** are the lead's, for the user's end review. The 33. **tmux from mise on Linux only** (lead's call): CI's runner has no tmux (fact 11), so mise installs tmux-builds there. On a Mac, mise leaves `tmux` alone, so that in the repo it stays the user's own, the one that runs the user's server (decision 27). 34. **The one place that runs each tool carries `//nolint:gosec` too, with its reason** (the user's calls, 2026-10-31), as does each tool's test helper, the tests' own place that runs it: tmux's and git's now, copilot's and gh's later. gosec's G204 ("subprocess launched with variable") wants constant arguments, but each tool needs the feature's own: a checked name, IDs and text that reach the tool as arguments, which no shell reads (fact 22). A test helper runs the tests' own arguments. 35. **`repo.Worktree.Add` makes a feature's branch and worktree, both or neither** (lead's call; fact 23). It refuses a relative path, and a path that's taken, even by an empty folder, which git would take. It makes the branch on its own first, since `worktree add -b` keeps the branch it made when the path is taken, and without an upstream (`--no-track`), so that `baton deliver` sets one when it pushes. If the worktree fails, a hook of the user's included, or Ctrl-C stops it, baton removes the worktree, locked or not, and deletes the branch, once git's child has ended; its error says what it couldn't undo. A stop interrupts git, as Ctrl-C does, rather than killing it, so that git, which a Ctrl-C reaches too, isn't killed halfway through removing its worktree. Ctrl-C during `git branch` itself may leave the branch. A feature's worktree goes next to the repo's main worktree, which `git worktree list` gives first, from any of the repo's worktrees. The caller's names reach git after `--end-of-options`. -36. **The plugin's folder is named by its files, and appears whole** (lead's call; fact 24). baton writes the plugin from the binary into its cache, as `plugin-`, the first 16 hex digits of a SHA-256 of the files' paths and contents. It copies the files into a new temporary folder, which then takes that name, unless another baton has given a folder that name first; then baton uses that one, which holds the same files. So a pane never loads half a plugin, the panes of one baton version share one folder, and Copilot CLI keeps one plugin-data folder per version, not per pane. Two versions, an old pane's and a new one's after an upgrade (fact 17), each keep their own folder, and baton never removes one, since a running pane may still use it; a baton killed while it writes leaves its temporary folder, a few kilobytes, which nothing removes either. A fixed folder would mix two versions' files; a folder per pane would leave a plugin-data folder per pane in `~/.copilot` once the plugin has hooks. baton trusts a folder that has the name: the cache is the user's, and baton makes it readable by the user alone. A role runs as the user, though, and can change it (risk 12). +36. **The plugin's folder is named by its files, and appears whole** (lead's call; fact 24). baton writes the plugin from the binary into its cache, as `plugin-`, the first 16 hex digits of a SHA-256 of the files' paths and contents. It copies the files into a new temporary folder, which then takes that name, unless another baton has given a folder that name first; then baton uses that one, if it holds the same files (decision 53). So a pane never loads half a plugin, the panes of one baton version share one folder, and Copilot CLI keeps one plugin-data folder per version, not per pane. Two versions, an old pane's and a new one's after an upgrade (fact 17), each keep their own folder, and baton never removes one, since a running pane may still use it; a baton killed while it writes leaves its temporary folder, a few kilobytes, which nothing removes either. A fixed folder would mix two versions' files; a folder per pane would leave a plugin-data folder per pane in `~/.copilot` once the plugin has hooks. The cache is the user's, and baton makes it readable by the user alone; a role runs as the user, though, and can change a folder there (risk 12), so baton checks a folder's files before it uses it (decision 53). 37. **The plugin holds only what baton runs** (lead's call): for now, the agents of the orchestrator, the lead, the engineer, QA and the reviewer. Each hook comes with its command (`agentStop` with `stop-hook`, `preToolUse` with `guard-hook`), since a `preToolUse` hook whose command fails denies every tool (fact 4). A prompt names only commands that baton has, so the step that gives a role a command adds it to the role's prompt (`ask` adds `baton ask`). A prompt says how to do the role's job and how to hand over, and leaves the next role to baton (decision 13). The prompts are plain files, each whole, so the text they share is written in each, rather than put together from templates, which would be more code to get wrong; a test checks that each names the commands it needs. 38. **`baton pane` becomes the role's `copilot`** (lead's call). It waits as baton, then replaces itself with `copilot` (`execve`), so `copilot` gets the pane's terminal and signals as they are, and its environment with `COPILOT_ALLOW_ALL=true` (decision 39): Ctrl-C reaches `copilot` alone, and the pane ends when `copilot` does (fact 25). A child process would share the terminal with baton, which would then have to ignore Ctrl-C while `copilot` runs, since baton's second Ctrl-C ends it at once (decision 28). Before it waits, `baton pane` finds `copilot`, writes the plugin and reads the config file, so a pane that can't start `copilot`, or not with the user's settings, never picks up work that nobody would then do. It picks up the work as `baton wait` does, under the feature's lock, and gives `copilot` the text that `baton wait` prints as its first prompt. While it waits, it reads the terminal and drops what the user types, so it stops when the terminal's input ends, as on Ctrl-D, or when the terminal is gone, even without a hangup (facts 22 and 25). A `copilot` that fails to start leaves the work picked up, for the `resume` step. The orchestrator's pane doesn't wait: its work comes through its own `baton wait`, so `baton pane` becomes its `copilot` at once (decision 45). `main` passes `syscall.Exec` in, so gosec sees no command to flag, and `copilot`'s start carries no `//nolint:gosec` (decision 34). Each role gets its model, reasoning effort and AI credits cap from the config file (decision 49), and `copilot` gets no `--session-id` until the `cost` step, which keeps it. 39. **A role's `copilot` trusts its worktree** (lead's call, flagged; fact 9). `baton pane` gives `copilot` its environment with `COPILOT_ALLOW_ALL=true`, in place of any value of the user's, which `copilot` would read first. That value also trusts the folder that `copilot` starts in, the feature's worktree, for that session alone, and saves nothing. Without it, a worktree outside the folders that the user trusts would stop the role on copilot's "Confirm folder trust" dialog, in a pane nobody watches (decision 9). The worktree holds the user's own repo, whose skills, plugins, MCP servers and hooks then load, as they do in the main worktree once the user trusts it; the tool allow-list still keeps their tools from the role (decision 10). The variable reaches the role's shells too, so a `copilot` that a role starts trusts its own folder as well; the `guard` step can deny that. Rejected: copilot's dialog; `--add-dir`, which doesn't trust the folder; `--trust-folder`, which needs `-p`; adding each worktree to the user's trusted folders, which would change `~/.copilot/` (decision 11). @@ -271,7 +271,8 @@ Decisions marked **lead's call** are the lead's, for the user's end review. The 49. **One config file, in layers** (lead's call, flagged; decision 16, fact 31). `config.json` in baton's folder under the user's config folder holds the settings for every repo at its top, and each repo's section in `repos`, keyed by the remote's URL as its host and path: `git@github.com:codedogapp/baton.git`, `https://github.com/codedogapp/baton` and `ssh://git@github.com:22/codedogapp/baton.git` all give `github.com/codedogapp/baton`, so a repo's HTTPS and SSH clones share a section; a remote that is a local path keeps its path. baton refuses a key in another form, and says the form; a key keeps the URL's case. The repo's section lies over the top, and the top over baton's own settings, one setting at a time: a setting that a layer leaves out, or gives its zero value, keeps the one below, so a `roundCap` or a `maxAiCredits` of 0 keeps the layer's below, and the branch prefix can't be empty; the orchestrator's `extraTools`, a list, replaces the list below whole, and `[]` gives none. baton's own: Claude Opus 5.5 for every role (decision 7), copilot's own reasoning effort, no cap, nudge's two tools for the orchestrator (decision 45), three rounds (decision 13) and `feature/` (decision 41). baton reads and checks the whole file, the other repos' sections too, so that a mistake shows at once, with the file's path, and not in a `copilot` that it stops: it refuses a member that it doesn't know (names match case: `roundCap`, not `RoundCap`), a member given twice, a role that it doesn't know, a model or a tool that starts with a dash, which would read as one of copilot's flags, a reasoning effort that copilot doesn't take (CLI 1.0.105's, so a new level needs a new baton), a cap below copilot's least, 30 (fact 10), a negative round cap, and extra tools for any role but the orchestrator (decision 10). A missing file gives baton's own settings. The step came in two: `config` reads the file, and `config-cli` gives its settings to `baton pane` and `baton feature start`. They find the repo's section by its `origin` remote's URL, as `git remote get-url origin` gives it, with the user's `insteadOf` rewrites (fact 32): a repo can have many remotes, and `origin` is the one that `git clone` makes. A repo without an `origin` gets the top's settings, even with other remotes, since baton can't tell which of them is the repo's own. `baton pane` reads the file after it writes the plugin and before it waits (decision 38), the orchestrator's pane too, so a mistake in the file stops a pane before its role picks up work; `baton feature start` reads it before it makes the feature; and `baton start` reads it before it makes the orchestrator's session, since a pane that fails takes its session, and its reason, with it, but not to show a session that runs, which has its settings already. A role's settings come after its plugin on copilot's command line: `--model`, then `--reasoning-effort` and `--max-ai-credits` only when the file sets them, the cap in plain digits, as `1000000.5`, never as `1e+06`. The other settings come with their steps. 50. **The guard reads bash with mvdan.cc/sh** (the user's call, 2026-11-03, for the module; lead's call, flagged, for the rest). The guard needs the commands that a bash call runs: a match on the text would miss `git -C x push`, `nohup git push` and `$(git push)`, and catch `echo 'git push'`. The standard library has no bash parser, so `internal/bash`, and only it, imports `mvdan.cc/sh/v3`, the parser of `shfmt`, pinned at exactly v3.14.1 in `go.mod`, which Renovate updates as it does the rest; its license is BSD-3, whose notice the release carries (`homebrew`). `bash.Calls` gives each command that a script runs as its words, as bash passes them: quotes and backslashes taken off, and each part that only the run decides, such as `$HOME` or `$(…)`, as a marker, `bash.Dynamic`. It finds the commands in substitutions, here-documents, functions and conditions too; for a wrapper (`builtin`, `caffeinate`, `command`, `env`, `exec`, `nice`, `nohup`, `stdbuf`, `sudo`, `time`, `timeout`, `xargs` and Homebrew's `g` names, such as `gtimeout`), the command that it runs, past its options, `NAME=value` words and `timeout`'s duration; and for `eval`, and `bash`, `dash`, `ksh`, `sh` and `zsh` with `-c`, the commands of their script. A program's name ignores case, as a Mac's file system does. A script that doesn't parse is an error, which the guard denies, and that is rare (fact 33). It can't see what only the run decides: an alias, a script file or `source`, a variable as the program (`"$X" push`), a script on standard input (`sh -s`, `| sh`, `bash < .git/baton/…` (risk 11), and to the binary that `BATON_BIN` names, which the hooks run. It misses a hard link, outside baton's folder, to a file in it, which the walk up the link's folders can't see; data that `web_fetch` sends in a URL, as it only reads; other repos' baton folders; and what decisions 50 and 51 list. And it misses writes to the plugin's cache, where baton trusts a folder by its name (decision 36), so a role that changes the hooks there turns the guard off in the panes that start after it (risk 12), until `plugin-check`. Risk 2's second belt (`belt`) is for the rest. +52. **`baton hook guard` denies a role's tools what the guard watches** (lead's call, flagged; decisions 9, 17, 48 and 51, facts 4 and 36, risks 2, 4, 11 and 12). The plugin's `preToolUse` hook runs `baton hook guard` before each tool that a pane's `copilot` calls. In a role's pane, it denies a `bash` call whose script the guard can't read (decision 50), or that runs a command that the guard watches, but baton's commands for a role: `baton wait`, `baton handover`, `baton ask`, `baton status` and baton's help (`baton help`, `-h`, `-help` and `--help`); the guard names a `baton` command by its first word, so `baton wait -h` passes and `baton answer -h` doesn't. It denies a `create` or an `edit` of a file in baton's folder, which only baton writes (decision 48): it makes the path absolute from the input's `cwd`, resolves the symlinks of the part of the path that is there, and walks up its folders, comparing each with baton's folder as a file (`os.SameFile`), so that neither a symlink, nor a disk that ignores case, nor a new file or folder gets past; in a repo without baton's folder, no path is baton's. Each reason starts with "baton's guard stopped this call, and none of it ran." and says why: the first command that the guard watches, by its name, with its kind's rule (an outward action is the orchestrator's, with the user's approval, so the role keeps its work local and says in its handover note what should go out; a role never drives tmux or the Mac's apps, nor starts another agent; a program must be named in the script itself), the parser's error, with a plea for plain bash, or the file and baton's folder, with the session's `files/` folder for notes. The hook denies the whole call, since a bash call is one script, and the role can try again. It answers only `deny`, and otherwise says nothing, which leaves the tool to `copilot`'s own permissions; the role's other tools only read or look after its shells (fact 36). It says nothing in the orchestrator's pane until `guard-orchestrator`. When it can't decide, as for input that isn't a JSON object, arguments that don't fit the tool, a pane without `BATON_ROLE` or a folder outside a git repo, it fails, so `copilot` denies the tool (fact 4). It reads only its input and, for a file tool, git's common folder, so it answers well within the hook's timeout, past which `copilot` would allow (risk 4). The guard stops a role's mistakes, not a role that sets out to get past it. It misses `bash`'s own writes, which it doesn't follow: in baton's folder, such as `echo … > .git/baton/…` (risk 11), and to the binary that `BATON_BIN` names, which the hooks run. It misses a hard link, outside baton's folder, to a file in it, which the walk up the link's folders can't see; data that `web_fetch` sends in a URL, as it only reads; other repos' baton folders; and what decisions 50 and 51 list. It denies the file tools in the plugin's cache too, and `baton pane` checks the plugin's files there before each `copilot` starts (decision 53), but `bash` can still change them while a `copilot` starts or runs (risk 12). Risk 2's second belt (`belt`) is for the rest. +53. **baton checks the plugin's folder before each `copilot` starts** (lead's call, flagged; decisions 36, 38 and 52, risk 12). `plugin.Write` hashes the files of a folder that has the plugin's name, by their paths in the folder and their contents, as it hashes the binary's, and uses the folder only if the hashes match and it is a folder, not a symlink to one. A folder with other files, more, fewer or changed ones, or with anything but folders and regular files, such as a symlink, and anything else under the name, it leaves as it is, and writes the plugin under the next name, `plugin--1`, then `-2`, up to `-7`; with all eight names taken, the pane fails and says why. baton never replaces a folder (Go's `os.Rename` refuses to), so a starting `copilot` never misses the plugin's folder, as it could while a swap ran, and goes on without the hooks and with the default agent (risk 10). Panes that start at once take the same new name, since a folder appears whole (decision 36). A role's pane checks again after its wait, which can last hours, just before it becomes `copilot`; a failure there leaves the work picked up, as a `copilot` that fails to start does (decision 38). The guard denies a role's `create` and `edit` in baton's cache folder, as in baton's folder, and fails without the user's cache folder (decision 52). `bash` can still change the plugin's files after a pane's check, while its `copilot` starts or runs, or through a hard link (risk 2), and a Finder `.DS_Store` in a folder costs one more folder. Rejected: rewriting a changed folder, which a starting `copilot` could see half written; removing it, which a running pane may still use; and making a checked folder read-only, which a role that runs as the user can undo. ## 5. Risks @@ -286,7 +287,7 @@ Decisions marked **lead's call** are the lead's, for the user's end review. The 9. **Floating tool versions**: mise tools use `latest` with no lockfile, so a new release (a new jactionlint rule, say) can turn CI red with no change in the repo, and a local run may use an older version than CI until `mise upgrade --local`. When CI turns red for no reason, check the tool versions first. 10. **A wrong agent doesn't stop `copilot`** (fact 24): an interactive session that can't find its agent goes on with the default agent, and `-i` hands that agent the role's work. baton's IDs come from the plugin that it writes (decision 37), and `live-run` saw each pane run its role's agent (facts 26 and 27). 11. **A role can write in baton's folder.** Under `--allow-all`, a role writes anywhere: in the live run, the reviewer put its notes there (fact 29). A stray note there misleads whoever reads the notes, and a role that edited the feature's file or its journal would mislead baton. `baton handover` refuses a note there, and the prompts send the notes to the role's session folder (decision 48), and the guard's `preToolUse` hook keeps the file tools out of it (decision 52), but a role's `bash` can still write there, as `echo … > .git/baton/…` does, which the guard doesn't see. -12. **A role can turn the guard off for the panes that start later** (decisions 36 and 52). baton trusts a plugin folder in its cache by its name, and a role runs as the user, so a role that changes the folder's files, such as `hooks.json`, with `bash` or a file tool, can take the guard away from each pane that loads that folder after it. `plugin-check` makes `baton pane` check the folder's files before it uses them, and the guard deny the file tools there; `bash`'s writes stay with risk 2. +12. **A role can turn the guard off for the panes that start later** (decisions 36, 52 and 53). A role runs as the user, so it can change the files of a plugin folder in baton's cache, such as `hooks.json`, with `bash` or a file tool, and take the guard away from each pane that loads that folder after it. `baton pane` checks the folder's files before each `copilot` starts, and the guard denies the file tools there (decision 53); `bash`'s writes after a check stay with risk 2. ## 6. Open questions diff --git a/internal/cli/guard.go b/internal/cli/guard.go index f0fbf8b..ab6ec09 100644 --- a/internal/cli/guard.go +++ b/internal/cli/guard.go @@ -53,8 +53,9 @@ type guardOutput struct { // hookGuard is the plugin's preToolUse hook, which runs before each tool that a pane's copilot // calls. In a role's pane, it denies a bash script that the guard can't read, or that runs a // command that the guard watches other than baton's commands for a role, and a create or edit of -// a file in baton's folder. It says nothing in the orchestrator's pane yet. Copilot denies the -// tool when the hook fails, so baton then reports why and exits with 1 (decision 52). +// a file in baton's folder or in baton's cache folder, which holds the plugin. It says nothing in +// the orchestrator's pane yet. Copilot denies the tool when the hook fails, so baton then reports +// why and exits with 1 (decisions 52 and 53). func hookGuard(ctx context.Context, env Env, args []string) error { if err := parse(newFlags(env, "hook guard"), args); err != nil { return err @@ -128,8 +129,14 @@ func scriptReason(script string) string { } // pathReason returns the reason to deny a file tool's write in a role's pane, or "" to let it run: -// a file in baton's folder, which only baton writes (decision 48). +// a file in baton's folder, which only baton writes (decision 48), or in baton's cache folder, +// whose plugin runs this guard (decision 53). func pathReason(ctx context.Context, dir, path string) (string, error) { + cache, err := batonCache() + if err != nil { + return "", err + } + store, err := openStore(ctx, dir) if err != nil { return "", err @@ -144,12 +151,16 @@ func pathReason(ctx context.Context, dir, path string) (string, error) { path = abs } - if !within(path, store.Dir) { - return "", nil + switch { + case within(path, store.Dir): + return fmt.Sprintf("%s%s is in baton's folder %s, which only baton writes: write your "+ + "notes in your session's files/ folder.", stopped, path, store.Dir), nil + case within(path, cache): + return fmt.Sprintf("%s%s is in baton's cache folder %s, which only baton writes: it "+ + "holds the plugin whose hook runs this guard.", stopped, path, cache), nil } - return fmt.Sprintf("%s%s is in baton's folder %s, which only baton writes: write your notes "+ - "in your session's files/ folder.", stopped, path, store.Dir), nil + return "", nil } // within reports whether the path is the folder or lies in it, by the files that they name, so diff --git a/internal/cli/guard_files_test.go b/internal/cli/guard_files_test.go index 5ed2019..8f0d620 100644 --- a/internal/cli/guard_files_test.go +++ b/internal/cli/guard_files_test.go @@ -8,6 +8,7 @@ import ( "github.com/codedogapp/baton/internal/cli" "github.com/codedogapp/baton/internal/flow" + "github.com/codedogapp/baton/internal/plugin" ) // TestGuardHookKeepsTheFileToolsOutOfBatonsFolder denies a role's create and edit of a file in @@ -69,6 +70,68 @@ func TestGuardHookKeepsTheFileToolsOutOfBatonsFolder(t *testing.T) { } } +// TestGuardHookKeepsTheFileToolsOutOfBatonsCache denies a role's create and edit of a file in +// baton's cache folder, which holds the plugin whose hook runs the guard, however the path names +// it (decision 53). It lets them write beside it. +func TestGuardHookKeepsTheFileToolsOutOfBatonsCache(t *testing.T) { + _, cache := standIn(t) + _, linked := repos(t) + + dir, err := plugin.Write(cache) + if err != nil { + t.Fatal(err) + } + + link := filepath.Join(linked, "cache") + if err := os.Symlink(cache, link); err != nil { + t.Fatal(err) + } + + hooks := filepath.Join(dir, hooksFile) + cases := []struct { + name string + path string // The path in the tool's arguments. + want string // The path that the reason names, or "" to let the tool run. + }{ + {"the plugin's hooks", hooks, hooks}, + {"baton's cache folder", cache, cache}, + {"a file in a new folder", cache + "/plugin-0/x.md", cache + "/plugin-0/x.md"}, + {"a symlink", link + "/x.md", link + "/x.md"}, + {"a folder beside it", cache + "-old/x.md", ""}, + } + + for _, test := range cases { + for _, tool := range []string{"create", "edit"} { + t.Run(tool+" "+test.name, func(t *testing.T) { + want := "" + if test.want != "" { + want = guardStopped + test.want + " is in baton's cache folder " + cache + + ", which only baton writes: it holds the plugin whose hook runs this guard." + } + + input := guardInput(t, linked, tool, map[string]any{"path": test.path}) + wantGuard(t, guardHook(t, linked, paneOf(flow.RoleEngineer), input), want) + }) + } + } +} + +// TestGuardHookFailsWithoutTheCacheFolder fails, and so denies a file tool, when it can't find +// the user's cache folder, which holds the plugin. +func TestGuardHookFailsWithoutTheCacheFolder(t *testing.T) { + _, linked := repos(t) + t.Setenv("HOME", "") + t.Setenv("XDG_CACHE_HOME", "") + + input := guardInput(t, linked, "create", map[string]any{"path": linked + "/x.md"}) + got := guardHook(t, linked, paneOf(flow.RoleLead), input) + + want := "baton hook guard: find the cache folder: " + if got.code != 1 || got.stdout != "" || !strings.HasPrefix(got.stderr, want) { + t.Errorf("got %+v; want exit code 1 and %q", got, want) + } +} + // TestGuardHookFindsBatonsFolderFromItsOwnFolder finds baton's folder from the hook's own folder // when the input names no folder, and a relative path starts there. func TestGuardHookFindsBatonsFolderFromItsOwnFolder(t *testing.T) { diff --git a/internal/cli/pane.go b/internal/cli/pane.go index bbb2174..303b010 100644 --- a/internal/cli/pane.go +++ b/internal/cli/pane.go @@ -32,7 +32,8 @@ const envBin = "BATON_BIN" // The orchestrator's copilot starts at once, since the user talks to it. A role's pane waits for // the role's first work, then becomes the role's copilot, with that work as its first prompt // (decision 38). It finds copilot, writes the plugin and reads the settings before it waits, so -// that a pane that can't start copilot never picks up work that nobody would then do. +// that a pane that can't start copilot never picks up work that nobody would then do. After the +// wait it checks the plugin again, which a role may have changed meanwhile (decision 53). func pane(ctx context.Context, env Env, args []string) error { if err := parse(newFlags(env, "pane"), args); err != nil { return err @@ -68,6 +69,10 @@ func pane(ctx context.Context, env Env, args []string) error { return err } + if dir, err = writePlugin(); err != nil { + return err + } + return becomeCopilot(env, program, command(role, dir, own, work), environment(bin, allowAll+"=true")) } @@ -108,25 +113,41 @@ func becomeCopilot(env Env, program string, line, vars []string) error { return nil } -// prepare finds copilot and writes the plugin into baton's cache folder, and returns copilot's -// path and the plugin's folder. +// prepare finds copilot and writes the plugin, and returns copilot's path and the plugin's folder. func prepare() (program, dir string, err error) { program, err = exec.LookPath("copilot") if err != nil { return "", "", err } - cache, err := os.UserCacheDir() + dir, err = writePlugin() if err != nil { - return "", "", fmt.Errorf("find the cache folder: %w", err) + return "", "", err } - dir, err = plugin.Write(filepath.Join(cache, cacheFolder)) + return program, dir, nil +} + +// writePlugin writes the plugin into baton's cache folder, unless a folder there holds it +// already, unchanged, and returns the plugin's folder (decisions 36 and 53). +func writePlugin() (string, error) { + cache, err := batonCache() if err != nil { - return "", "", err + return "", err } - return program, dir, nil + return plugin.Write(cache) +} + +// batonCache returns baton's folder in the user's cache folder, which holds the plugin (decision +// 36). +func batonCache() (string, error) { + cache, err := os.UserCacheDir() + if err != nil { + return "", fmt.Errorf("find the cache folder: %w", err) + } + + return filepath.Join(cache, cacheFolder), nil } // watch reads the pane's terminal, and drops what the user types there, until the terminal's diff --git a/internal/cli/pane_plugin_test.go b/internal/cli/pane_plugin_test.go new file mode 100644 index 0000000..5719d38 --- /dev/null +++ b/internal/cli/pane_plugin_test.go @@ -0,0 +1,86 @@ +package cli_test + +import ( + "os" + "path/filepath" + "testing" + "testing/synctest" + + "github.com/codedogapp/baton/internal/flow" + "github.com/codedogapp/baton/internal/state" +) + +// hooksFile is the plugin's hooks file, in the plugin's folder. +var hooksFile = filepath.Join("com.github.copilot", "hooks", "hooks.json") + +// TestPaneChecksThePluginAgainAfterTheWait has the engineer's pane, whose plugin a role changes +// while it waits, start the engineer's copilot with the plugin written again, under the folder's +// next name, and leave the changed folder as it is (decision 53). +func TestPaneChecksThePluginAgainAfterTheWait(t *testing.T) { + copilot, cache := standIn(t) + + synctest.Test(t, func(t *testing.T) { + var first string + + got, store := handedOverAfter(t, func() { + first = pluginIn(t, cache) + write(t, first, hooksFile, "{}") + }) + + wantCopilot(t, got, flow.RoleEngineer, execed{ + path: copilot, + args: copilotLine(flow.RoleEngineer, first+"-1", + "Picked up demo-1 as engineer: round 1 of 3, from lead.\nNote: "+ + notePath(store, "02-lead-to-engineer.md")+"\n"), + env: copilotEnv(), + }) + + hooks := filepath.Clean(filepath.Join(first, hooksFile)) + if data, err := os.ReadFile(hooks); string(data) != "{}" { + t.Errorf("the changed hooks file holds %q, %v; want it as the role left it", data, err) + } + }) +} + +// TestPaneFailsWhenItCantWriteThePluginAfterTheWait has the engineer's pane, whose cache folder +// a file takes the place of while it waits, fail after it picks up the engineer's work, and start +// no copilot: the work stays picked up, as when copilot fails to start (decision 38). +func TestPaneFailsWhenItCantWriteThePluginAfterTheWait(t *testing.T) { + _, cache := standIn(t) + + synctest.Test(t, func(t *testing.T) { + got, store := handedOverAfter(t, func() { fileFor(t, cache) }) + + wantRefused(t, got, "engineer in demo-1: waiting for work.\n", + "make the cache folder: mkdir "+cache+": not a directory\n") + wantWork(t, store, state.Work{ + Note: "02-lead-to-engineer.md", From: flow.RoleLead, Picked: true, + }) + }) +} + +// handedOverAfter starts the engineer's pane, which waits for its work, and makes the change while +// it waits; the lead then picks up its own work and hands over to the engineer. It returns what +// the engineer's pane did, and the feature's store. +func handedOverAfter(t *testing.T, change func()) (ran, state.Store) { + t.Helper() + + mainDir, linked := repos(t) + store := started(t, mainDir, fresh()) + + env, _ := inPane(t, linked, paneOf(flow.RoleEngineer)) + engineer := make(chan ran, 1) + + go func() { engineer <- paneFor(t, env) }() + + synctest.Wait() + change() + + if got := waitAs(t, mainDir, flow.RoleLead); got.code != 0 { + t.Fatalf("the lead's wait: %+v", got) + } + + handOver(t, mainDir, flow.RoleLead, flow.VerdictNone) + + return <-engineer, store +} diff --git a/internal/plugin/changed_test.go b/internal/plugin/changed_test.go new file mode 100644 index 0000000..8efca12 --- /dev/null +++ b/internal/plugin/changed_test.go @@ -0,0 +1,129 @@ +package plugin + +import ( + "fmt" + "io/fs" + "maps" + "os" + "path/filepath" + "testing" +) + +// hooksFile and agentsFolder are paths in the plugin's folder. +var ( + hooksFile = filepath.Join("com.github.copilot", "hooks", "hooks.json") + agentsFolder = filepath.Join("com.github.copilot", "agents") +) + +// TestWriteTakesTheNextNameForAChangedFolder has Write find the plugin's folder changed, as a role +// could change it (risk 12), and write the plugin under the next name, which serves from then on, +// and leave the changed folder as it is (decision 53). +func TestWriteTakesTheNextNameForAChangedFolder(t *testing.T) { + for _, test := range []struct { + name string + change func(t *testing.T, dir string) // Changes the plugin's folder. + }{ + {"a changed file", func(t *testing.T, dir string) { + must(t, os.WriteFile(filepath.Join(dir, hooksFile), []byte("{}"), 0o600)) + }}, + {"a file more", func(t *testing.T, dir string) { + put(t, filepath.Join(dir, agentsFolder, "helper.agent.md")) + }}, + {"a file less", func(t *testing.T, dir string) { + must(t, os.Remove(filepath.Join(dir, hooksFile))) + }}, + {"a file more that it can't read", func(t *testing.T, dir string) { + if os.Geteuid() == 0 { + t.Skip("root reads any file") + } + + put(t, filepath.Join(dir, "notes.md")) + chmod(t, filepath.Join(dir, "notes.md"), 0) + }}, + {"a symlink to a copy of a file", func(t *testing.T, dir string) { + moveAway(t, filepath.Join(dir, hooksFile), filepath.Join(t.TempDir(), "hooks.json")) + }}, + {"a symlink to a copy of the folder", func(t *testing.T, dir string) { + moveAway(t, dir, filepath.Join(t.TempDir(), "plugin")) + }}, + {"an empty folder", func(t *testing.T, dir string) { + must(t, os.RemoveAll(dir)) + must(t, os.Mkdir(dir, 0o700)) + }}, + {"a file in the folder's place", func(t *testing.T, dir string) { + must(t, os.RemoveAll(dir)) + put(t, dir) + }}, + } { + t.Run(test.name, func(t *testing.T) { + cache := t.TempDir() + first := take(Write(cache))(t) + test.change(t, first) + before := listing(t, first) + + next := take(Write(cache))(t) + if next != first+"-1" { + t.Errorf("Write = %s, want %s-1", next, first) + } + + if again := take(Write(cache))(t); again != next { + t.Errorf("the next Write = %s, want %s", again, next) + } + + same(t, next, thePlugin(t)) + only(t, cache, filepath.Base(first), filepath.Base(next)) + + if after := listing(t, first); !maps.Equal(after, before) { + t.Errorf("Write changed %s to %v, from %v", first, after, before) + } + }) + } +} + +func TestWriteByManyPanesAtOnceAfterAChange(t *testing.T) { + cache := t.TempDir() + first := take(Write(cache))(t) + must(t, os.WriteFile(filepath.Join(first, hooksFile), []byte("{}"), 0o600)) + + if dir := atOnce(t, cache); dir != first+"-1" { + t.Errorf("the panes' Write = %s, want %s-1", dir, first) + } + + only(t, cache, filepath.Base(first), filepath.Base(first)+"-1") +} + +// moveAway moves the file or folder to the new path, and puts a symlink to it in its place. +func moveAway(t *testing.T, path, to string) { + t.Helper() + + must(t, os.Rename(path, to)) + must(t, os.Symlink(to, path)) +} + +// listing returns the paths in the folder, or the file, without following a symlink, each with its +// mode, size and time of change, which change if anything writes there. +func listing(t *testing.T, path string) map[string]string { + t.Helper() + + found := map[string]string{} + + err := filepath.WalkDir(path, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + + info, err := entry.Info() + if err != nil { + return err + } + + found[path] = fmt.Sprint(info.Mode(), info.Size(), info.ModTime()) + + return nil + }) + if err != nil { + t.Fatal(err) + } + + return found +} diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index 1a48dd4..d5501fd 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -1,18 +1,19 @@ // Package plugin holds baton's plugin for Copilot CLI, which the binary embeds and writes to its // cache for --plugin-dir: the agents of the orchestrator and the roles, the preToolUse hook, which // runs baton hook guard, and the agentStop hook, which runs baton hook stop (decisions 11, 36, 37, -// 47 and 52). +// 47, 52 and 53). package plugin import ( "crypto/sha256" "embed" "encoding/hex" - "errors" "fmt" "io/fs" "os" "path/filepath" + "strconv" + "strings" "github.com/codedogapp/baton/internal/flow" ) @@ -24,6 +25,7 @@ const ( root = "files" // The folder in the binary that holds the plugin's files. hashSize = 8 // The bytes of the files' hash that name the plugin's folder. privateFolder = 0o700 // The cache folder's mode: only the user's. + names = 8 // The names that the plugin's folder may take: plugin-, -1 to -7. ) //go:embed files @@ -35,8 +37,8 @@ func Agent(role flow.Role) string { return Name + ":" + string(role) } -// Write writes the plugin into baton's cache folder, unless it's there already, and returns the -// plugin's folder (decision 36). +// Write writes the plugin into baton's cache folder, unless it's there already, unchanged, and +// returns the plugin's folder (decisions 36 and 53). func Write(cache string) (string, error) { return write(cache, files) } @@ -45,8 +47,13 @@ func Write(cache string) (string, error) { // hash, so that each version of the plugin gets a folder of its own. The folder appears whole: // the files go into a new temporary folder, which then takes the folder's name, unless another // baton has given it that name first. +// +// A folder that has the name already serves only if it holds the same files, since a role runs as +// the user and could have changed them, and with them the hooks that guard the panes (risk 12). +// Else the files take the next name: write never replaces a folder, which a copilot may be +// loading (decision 53). func write(cache string, tree fs.FS) (string, error) { - sum, err := digest(tree) + sum, err := digest(tree, root) if err != nil { return "", fmt.Errorf("hash the plugin: %w", err) } @@ -66,23 +73,54 @@ func write(cache string, tree fs.FS) (string, error) { return "", fmt.Errorf("write the plugin: %w", err) } - // A folder with the name already holds the same files: another baton has written them. - dir := filepath.Join(cache, "plugin-"+sum) - if err := os.Rename(filepath.Join(temp, root), dir); err != nil && - !errors.Is(err, fs.ErrExist) { - return "", fmt.Errorf("write the plugin: %w", err) + return place(filepath.Join(temp, root), filepath.Join(cache, "plugin-"+sum), sum) +} + +// place gives the folder the name, or else the first of the next names, name-1 to name-7, that it +// can take, and returns that name. A name whose folder holds the files whose hash is the sum +// serves as it is. +func place(folder, name, sum string) (string, error) { + var err error + + for try := range names { + dir := name + if try > 0 { + dir += "-" + strconv.Itoa(try) + } + + if err = os.Rename(folder, dir); err == nil || intact(dir, sum) { + return dir, nil + } + } + + return "", fmt.Errorf("write the plugin under any of %d names: %w", names, err) +} + +// intact reports whether the folder holds the files whose hash is the sum, and is a folder, not +// a symlink to one, which could lead anywhere. +func intact(dir, sum string) bool { + info, err := os.Lstat(dir) + if err != nil || !info.IsDir() { + return false } - return dir, nil + found, err := digest(os.DirFS(dir), ".") + + return err == nil && found == sum } -// digest returns a hash of the files in the tree's root folder: their paths and their contents. -func digest(tree fs.FS) (string, error) { +// digest returns a hash of the files in the tree's top folder: their paths in that folder and their +// contents. It fails on an entry that is neither a folder nor a regular file, such as a symlink, +// which could lead anywhere, or a named pipe, which could keep it waiting. +func digest(tree fs.FS, top string) (string, error) { hash := sha256.New() - err := fs.WalkDir(tree, root, func(path string, entry fs.DirEntry, err error) error { - if err != nil || entry.IsDir() { + err := fs.WalkDir(tree, top, func(path string, entry fs.DirEntry, err error) error { + switch { + case err != nil || entry.IsDir(): return err + case !entry.Type().IsRegular(): + return fmt.Errorf("%s isn't a regular file", path) } data, err := fs.ReadFile(tree, path) @@ -90,7 +128,7 @@ func digest(tree fs.FS) (string, error) { return err } - _, _ = fmt.Fprintf(hash, "%s\x00%d\x00", path, len(data)) + _, _ = fmt.Fprintf(hash, "%s\x00%d\x00", strings.TrimPrefix(path, top+"/"), len(data)) _, _ = hash.Write(data) return nil diff --git a/internal/plugin/refusals_test.go b/internal/plugin/refusals_test.go index 19a5c9b..cfa4f3c 100644 --- a/internal/plugin/refusals_test.go +++ b/internal/plugin/refusals_test.go @@ -4,6 +4,7 @@ import ( "io/fs" "os" "path/filepath" + "strconv" "strings" "testing" "testing/fstest" @@ -23,10 +24,10 @@ func TestWriteRefusals(t *testing.T) { cache: emptyCache, want: []string{"hash the plugin: open files: file does not exist"}, }, { - name: "a file it can't read", + name: "a symlink", tree: fstest.MapFS{root + "/agent.md": {Mode: fs.ModeSymlink, Data: []byte("gone")}}, cache: emptyCache, - want: []string{"hash the plugin: open files/agent.md: file does not exist"}, + want: []string{"hash the plugin: files/agent.md isn't a regular file"}, }, { name: "a file in the cache's place", tree: plugin, @@ -53,20 +54,28 @@ func TestWriteRefusals(t *testing.T) { }, want: []string{"write the plugin: mkdir ", "/plugin-", ".tmp: permission denied"}, }, { - name: "a file it can't copy", - tree: fstest.MapFS{root + "/hook": {Mode: fs.ModeNamedPipe}}, + name: "a file whose name is too long for the disk", + tree: fstest.MapFS{root + "/" + strings.Repeat("a", 256): {Data: []byte("{}")}}, cache: emptyCache, - want: []string{"write the plugin: CopyFS files/hook: invalid argument"}, + want: []string{"write the plugin: open ", "/aaaa", ": file name too long"}, }, { - name: "a file in the folder's place", + name: "a file in the place of each name for the folder", tree: plugin, cache: func(t *testing.T, tree fs.FS) string { cache := t.TempDir() - put(t, filepath.Join(cache, filepath.Base(take(write(t.TempDir(), tree))(t)))) + name := filepath.Join(cache, filepath.Base(take(write(t.TempDir(), tree))(t))) + + put(t, name) + + for try := 1; try < names; try++ { + put(t, name+"-"+strconv.Itoa(try)) + } return cache }, - want: []string{"write the plugin: rename ", "/plugin-", ": not a directory"}, + want: []string{ + "write the plugin under any of 8 names: rename ", "/plugin-", "-7: not a directory", + }, }} { t.Run(test.name, func(t *testing.T) { cache := test.cache(t, test.tree) diff --git a/internal/plugin/write_test.go b/internal/plugin/write_test.go index 58706f8..a95d5aa 100644 --- a/internal/plugin/write_test.go +++ b/internal/plugin/write_test.go @@ -36,21 +36,16 @@ func TestWriteWritesThePlugin(t *testing.T) { t.Errorf("the cache folder's mode is %v, want 0700: only the user's", info.Mode().Perm()) } - plugin, err := fs.Sub(files, root) - if err != nil { - t.Fatal(err) - } - - same(t, dir, plugin) + same(t, dir, thePlugin(t)) only(t, cache, filepath.Base(dir)) } -func TestWriteKeepsTheFolderItFinds(t *testing.T) { +func TestWriteKeepsTheFolderThatHoldsThePlugin(t *testing.T) { cache := t.TempDir() first := take(Write(cache))(t) - mark := filepath.Join(first, "mark") - if err := os.WriteFile(mark, nil, 0o600); err != nil { + before, err := os.Stat(first) + if err != nil { t.Fatal(err) } @@ -58,40 +53,20 @@ func TestWriteKeepsTheFolderItFinds(t *testing.T) { t.Errorf("the second Write = %s, want the first's, %s", again, first) } - if _, err := os.Stat(mark); err != nil { + if after, err := os.Stat(first); err != nil || !os.SameFile(after, before) { t.Errorf("the second Write replaced the folder: %v", err) } + same(t, first, thePlugin(t)) only(t, cache, filepath.Base(first)) } func TestWriteByManyPanesAtOnce(t *testing.T) { cache := t.TempDir() + dir := atOnce(t, cache) - const panes = 8 - - dirs, errs := make([]string, panes), make([]error, panes) - - var group sync.WaitGroup - for pane := range panes { - group.Go(func() { dirs[pane], errs[pane] = Write(cache) }) - } - - group.Wait() - - for pane := range panes { - if errs[pane] != nil || dirs[pane] != dirs[0] { - t.Errorf("Write %d = %s, %v; want %s, nil", pane, dirs[pane], errs[pane], dirs[0]) - } - } - - plugin, err := fs.Sub(files, root) - if err != nil { - t.Fatal(err) - } - - same(t, dirs[0], plugin) - only(t, cache, filepath.Base(dirs[0])) + same(t, dir, thePlugin(t)) + only(t, cache, filepath.Base(dir)) } func TestWriteNamesTheFolderByItsFiles(t *testing.T) { @@ -109,9 +84,9 @@ func TestWriteNamesTheFolderByItsFiles(t *testing.T) { for number, pair := range [][2]fstest.MapFS{ {tree("a.md", "one"), tree("a.md", "two")}, // Other contents. {tree("a.md", "one"), tree("b.md", "one")}, // Another path. - {tree("a1", "xx", "b", "z"), tree("a", "xxfiles/b1\x00z")}, // A path's end. - {tree("a", "2", "bcd", "y"), tree("a", "files/bcd\x001y")}, // A length's end. - {tree("a", "files/x", "b", "y"), tree("a", "", "xfiles/b", "y")}, // A file's end. + {tree("a1", "xx", "b", "zzzzzzz"), tree("a", "xxb7\x00zzzzzzz")}, // A path's end. + {tree("a", "2", "bcd", "yyyyyyy"), tree("a", "bcd\x007yyyyyyy")}, // A length's end. + {tree("a", "x", "b", "y"), tree("a", "", "xb", "y")}, // A file's end. } { if one, other := take( write(cache, pair[0]), @@ -127,6 +102,52 @@ func TestWriteNamesTheFolderByItsFiles(t *testing.T) { } } +// atOnce has many panes write the plugin into the cache at once, and returns the folder that each +// of them got: the same for each. +func atOnce(t *testing.T, cache string) string { + t.Helper() + + const panes = 8 + + dirs, errs := make([]string, panes), make([]error, panes) + + var group sync.WaitGroup + for pane := range panes { + group.Go(func() { dirs[pane], errs[pane] = Write(cache) }) + } + + group.Wait() + + for pane := range panes { + if errs[pane] != nil || dirs[pane] != dirs[0] { + t.Errorf("Write %d = %s, %v; want %s, nil", pane, dirs[pane], errs[pane], dirs[0]) + } + } + + return dirs[0] +} + +// thePlugin returns the plugin's files, as the binary holds them. +func thePlugin(t *testing.T) fs.FS { + t.Helper() + + plugin, err := fs.Sub(files, root) + if err != nil { + t.Fatal(err) + } + + return plugin +} + +// must fails the test on the error. +func must(t *testing.T, err error) { + t.Helper() + + if err != nil { + t.Fatal(err) + } +} + // tree returns a tree with the files in its root folder, given as pairs of a path and a text. func tree(pairs ...string) fstest.MapFS { files := fstest.MapFS{}