diff --git a/docs/plan.md b/docs/plan.md index aab8fd7..e13e63a 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -53,8 +53,9 @@ The first milestone is a walking skeleton (`skeleton`): one feature on a throwaw | `config-cli` | `baton pane` and `baton feature start` take their settings from the config, for the repo of the `origin` remote's URL: each role's model, reasoning effort and AI credits cap, and the orchestrator's extra tools, in place of the pane's model and nudge's tools, and the round cap and the branch prefix in place of the start's (decision 49, fact 32); the README's config section | Done (#30) | | `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) | In review (#33) | -| `guard-hook` | `baton hook guard` with its `preToolUse` hook in the plugin: in a role's pane, it denies the commands that the guard watches, but baton's commands that a role runs, a script that doesn't parse, and the file tools' writes in baton's folder (decisions 9, 17 and 48; risk 11) | Todo | +| `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-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 | @@ -130,6 +131,7 @@ Checked on 2026-10-31 with Copilot CLI 1.0.105 and tmux 3.8, unless a fact says 33. **mvdan.cc/sh reads the agents' bash** (v3.14.1, on 2026-11-03). Of the 32,473 distinct commands of the bash tool calls in the user's Copilot CLI sessions, its bash parser read all but one into 181,550 commands, in 0.2 seconds; the one that failed holds `$[…]`, bash's old arithmetic, on which bash fails too when it runs it. The parser drops a NUL byte without an error, and keeps a backslash that ends a word. 34. **git's options and gh's commands** (git 2.56.0 and gh 2.103.2, on 2026-11-03). Before its command, git takes the next word as the value of `-C`, `-c`, `--attr-source`, `--config-env`, `--git-dir`, `--namespace`, `--shallow-file` and `--work-tree`, unless the option holds it after `=`, and fails on an option that it doesn't know, `--super-prefix` among them, which older gits take with a value. `--` ends git's options. git's and gh's commands match case: `git PUSH` fails with `cannot handle PUSH as a builtin`, and `gh PR view` with `unknown command "PR" for "gh"`. 35. **curl's, wget's and gh api's options** (curl 8.7.1, GNU Wget 1.25.0 and gh 2.103.2, on 2026-11-03, from their help and their requests to a server on 127.0.0.1). curl sends data with `-d` (`--data`, and its `-ascii`, `-binary`, `-raw` and `-urlencode` forms), `-F` (`--form`, `--form-string`), `--json` and `-T` (`--upload-file`), which, with no method (`-X`, `--request`), make a POST, or a PUT for `-T`. With `-X GET`, `-d` sends a GET with a body. `-G` puts the data of `-d` and `--json` into the URL, but leaves `-F` a POST and `-T` a PUT, and `--no-get` undoes it; `-I` with `-d` fails. `--expand-` before an option's name fills in variables (`--expand-data` POSTs), and `--next` (`-:`) starts another request with options of its own. curl refuses a long option's abbreviation; wget takes the start of a name (`--meth=POST`, `--post-d=x`). wget sends data with `--post-data` and `--post-file`, and, with `--method`, `--body-data` and `--body-file`; `-e` (`--execute`) gives it a line of its settings, whose name ignores case, `-` and `_` (`-e Post_Data=x` POSTs). gh api POSTs once it has a field (`-f`, `-F`, `--raw-field`, `--field`) or `--input`, unless `-X GET` (`--method`) puts the fields into the URL, and it reads `-X=GET` as `GET`. A short option that takes a value, by each tool's help, takes the rest of its word, or else the next word: curl's `AbcCdDeEFhHKmoPQrtTuUwxXyYz`, wget's `aABDeiIlnoOPQRtTUwX` (`-nv` is `-n` with `v`) and gh api's `fFHpqtX`. curl also reads its options from `~/.curlrc` and from `-K` (`--config`) files, and wget its settings from `~/.wgetrc` and the file that `WGETRC` names. +36. **The tools' arguments in a `preToolUse` hook** (Copilot CLI 1.0.105, on 2026-11-03, checked live). The hook's input names the tool in `toolName` and gives its arguments in `toolArgs`, which the hooks reference types as `unknown`, as a JSON object: for `bash`, `command` and `description`, with `mode` and, for an async call, `initial_wait` and `shellId` (the agent-wake spike's hook log); for `create`, `path` and `file_text`; for `edit`, `path`, `old_str` and `new_str`, the paths absolute, as the CLI's tool schemas ask. The input also has `sessionId`, `timestamp` and `cwd`. The answer `{"permissionDecision":"deny","permissionDecisionReason":…}` stops the tool, under `--allow-all` too, and the agent gets `Denied by preToolUse hook: ` with the reason, which a denial needs. The live check ran `copilot -p` with Claude Haiku 5.5, `--allow-all`, the plugin of `guard-hook` and a throwaway `COPILOT_HOME`, in a throwaway repo, for 0.21 AI credits: `baton hook guard` denied `tmux -V` and a `create` in baton's folder, and said nothing to a `create` and an `edit` of a file in the repo, which ran. Of a role's tools (decision 10), `bash`, `create` and `edit` write; `view`, `grep`, `glob` and `web_fetch` read; and `read_bash`, `stop_bash` and `list_bash` look after the shells that `bash` starts. ## 3. Design @@ -187,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, `ssh`, `baton deliver`, `tmux`, another agent's CLI, and a program that only the run decides (decision 51). 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 (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). **State** (decision 15). Everything lives in `$(git rev-parse --git-common-dir)/baton/`, which every worktree of the repo shares and git never commits: @@ -253,8 +255,8 @@ 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. -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`), 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. +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). +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). 40. **A failed start leaves nothing behind** (lead's call). `baton feature start` makes the feature first, which claims its slug, then its branch and worktree, then its tmux session, and when a part fails, it undoes the parts it made. `repo.Worktree.Undo` removes the worktree, with any changes in it, and deletes the branch, as `Add` does when it fails (decision 35). `state.Store.Remove` takes the feature's lock and removes the feature's file, which ends the feature, then its event log and its notes. It keeps the lock file, which baton never replaces, so a process that waits for the lock finds no feature once it gets it, and a new feature can take the slug and start afresh. A crash in between can leave the old event log and notes, which a new feature with that slug would then keep. @@ -265,10 +267,11 @@ Decisions marked **lead's call** are the lead's, for the user's end review. The 45. **`baton start` opens the orchestrator in the user's terminal, and the orchestrator keeps copilot's own prompts** (lead's call, flagged). `baton start`, or plain `baton`, runs in the user's own shell, and refuses a pane that baton started, by `BATON_ROLE` (decision 29). From any of the repo's worktrees, it names the session `baton-` after the main worktree's folder, as a feature's session is named (decision 41), and makes it in the main worktree, unless it runs already. A session of that name in another folder belongs to another repo with a folder of the same name, or to the user, so baton refuses it rather than show it. Then baton shows the session in the user's terminal: outside tmux, it becomes `tmux attach-session` (`execve`, as `baton pane` becomes `copilot`, decision 38), so tmux keeps the terminal until the user detaches; inside tmux, it switches the user's client, the one that shows baton's pane, to the session (fact 22). With no client to switch, baton fails with tmux's error, and the session stays for the next `baton start`. Two `baton start`s at once in one repo: one makes the session, and the other fails on tmux's `duplicate session`. The orchestrator's `copilot` starts at once, with no first prompt, since the user speaks first. It keeps copilot's own prompts: no `--allow-all`, `--no-ask-user` or `COPILOT_ALLOW_ALL`, since the user watches that pane and approves each tool, until the `guard` step decides what the orchestrator may do unasked; so copilot's folder-trust question for the main worktree is the user's to answer too, once. It gets the roles' tools (decision 10), plus `ask_user` and its extra tools from the config file, by default the user's nudge tools, `nudge-ask` and `nudge-report` (decision 49); for a user without nudge, `copilot` only says that those names are unknown (fact 7), and `"extraTools": []` leaves them out. Its prompt, like the roles', names only the commands that baton has (decision 37): it plans with the user, starts features, keeps one `baton wait` running, reports each ready or escalated feature, and hands features back on the user's word. 46. **A role's question waits in its feature's file, one at a time** (lead's call, flagged). The holder's last question sits at the top of `feature.json`, so it keeps the format (decision 24): the role that asked, the text, whether the orchestrator's `baton wait` picked it up, and the answer once it comes. Not in `questions/.json`, as planned: a question belongs to its feature, so it takes the feature's lock and goes in its journal, with its text, as every change does (decision 26), and the slug names it, so `baton answer ` needs no ID. Only the holder asks, since only the holder works; the orchestrator asks the user. A role asks one question at a time, and baton refuses its handover until the answer comes, since the answer goes to the role that asked, which should still hold the feature when it acts on it. `baton ask` stores the question under the lock, then reads the file every second without it, as any reader may, until the answer comes. An ask with the text of the role's last question doesn't ask it again: it waits for the answer, or prints it if it came, so a role whose ask ended early, in a crash or a restart, gets the answer by asking again, and the orchestrator doesn't get the question twice. The holder's handover clears its question, which the journal keeps, so the same question in a later turn is a new one; the cost is that a role that means to ask the same question twice in one turn gets the first answer. If another question takes the place of an ask's question before the ask reads the answer, as two asks at once could, that ask fails rather than print an answer to another question, and so does an ask whose question a handover cleared before it read the answer. The orchestrator's `baton wait` picks up a question in any feature that a role holds, in the order of the slugs, as it picks up its work (decision 44): it marks the question picked under the lock and logs a pickup, so its next wait doesn't print it again; a question that it picked up but didn't answer, as when its pane stopped, waits for the `resume` step to bring it back. `baton answer` runs only in the orchestrator's pane, while a question waits for its answer. The texts are the commands' arguments, each one whole and not blank, so the prompts tell the roles and the orchestrator to quote them for the shell. 47. **A stop hook sends back a `copilot` that stops while nothing of its role waits** (lead's call, flagged; facts 4 and 5). The plugin's `agentStop` hook runs `"$BATON_BIN" hook stop`, and `baton pane` gives each pane's `copilot` `BATON_BIN`, the path of its own baton, in place of any value of the user's, so the hook runs the baton that started the pane, not one from the `PATH` (decision 42). `baton wait` and `baton ask`, and `baton pane` while it waits, mark their role as waiting: a shared `flock` on `.wait` in the feature's folder, or on `orchestrator.wait` in baton's folder, files that baton never removes, as it never removes a lock (decision 40). The kernel releases the mark when its process ends, so a wait that crashed leaves none, and two waits of one role share it. A wait takes the mark only when its first look finds nothing, so a wait that refuses, or picks its work up at once, writes nothing; that look takes milliseconds, and a model's turn far longer, so the hook sees the mark of a wait that the turn started. `baton hook stop` lets the `copilot` stop while its role's mark is held, since the wait's end wakes it (fact 5). Otherwise it sends it back, with a reason that becomes its next prompt: to start `baton wait` when work can come to it, because another role holds the feature or its work waits there; to ask its question again when that waits for its answer, since baton doesn't ask it twice (decision 46); and otherwise to finish its part and hand over, or just to end its turn when a command of its own runs in the background. The orchestrator gets the first reason while a role holds a feature or its work waits in one, and may stop once it holds every feature with its work picked up, since those wait on the user, or when there is none. The hook changes nothing in baton's folder, and lets the `copilot` stop when its block has sent this turn back already (`stop_hook_active`), so a `copilot` that can't do what the reason says stops one turn later; that turn is also the cost for a role that stops to wait for a command of its own. It finds the repo from the `copilot`'s folder in its input, or from its own folder without one. A hook that fails lets the `copilot` stop (fact 4), so baton reports why on stderr and exits with 1, and copilot logs it. When two hooks look at one mark at the same moment, one can take the other's look for a wait and let its `copilot` stop, as a hook that fails does; only two `copilot`s of one role could do that. Rejected: looking for a `baton wait` in the process table, which needs `ps` and can't tell one pane's wait from another's; a PID file, which a crash leaves behind; and a mark that every wait takes at its start, which would write files for waits that refuse. -48. **Only baton writes in its folder** (lead's call, flagged; fact 29, risk 11). `baton handover` refuses a note, and `baton feature start` a brief, that lies in baton's folder, once a relative path is made absolute from the working folder, so that the check and the read look at the same file, and the symlinks of both paths are resolved; it says to move the file to the session's `files/` folder, and the refusal changes nothing. In the live run the reviewer wrote its notes there, named as baton names the next one: a stray file there misleads whoever reads the notes, and baton numbers a new note after the folder's highest one (decision 26), so a stray shifts the numbers. The prompts send each role's notes, and the orchestrator's briefs and notes, to its session's `files/` folder (`$COPILOT_HOME/session-state//files/`), which Copilot CLI names in each session's context, never to the repo's worktrees, where they could get committed, or to baton's folder. They also tell the roles that the folder of the notes that baton hands them is baton's, and to run git and their other commands in the feature's worktree, where their pane starts, since the notes' paths run through the main worktree. So decision 19's guard lets the orchestrator `edit` and `create` only in its session's folder, no longer in baton's. On a disk that ignores case, as a Mac's does by default, a path that spells baton's folder in other case gets through, and the refusal stops only the note that a handover names: the guard's `preToolUse` hook can keep the file tools out of baton's folder (risk 11). baton doesn't refuse a note in a worktree, which only risks a commit, and doesn't remove a stray that is there already. +48. **Only baton writes in its folder** (lead's call, flagged; fact 29, risk 11). `baton handover` refuses a note, and `baton feature start` a brief, that lies in baton's folder, once a relative path is made absolute from the working folder, so that the check and the read look at the same file, and the symlinks of both paths are resolved; it says to move the file to the session's `files/` folder, and the refusal changes nothing. In the live run the reviewer wrote its notes there, named as baton names the next one: a stray file there misleads whoever reads the notes, and baton numbers a new note after the folder's highest one (decision 26), so a stray shifts the numbers. The prompts send each role's notes, and the orchestrator's briefs and notes, to its session's `files/` folder (`$COPILOT_HOME/session-state//files/`), which Copilot CLI names in each session's context, never to the repo's worktrees, where they could get committed, or to baton's folder. They also tell the roles that the folder of the notes that baton hands them is baton's, and to run git and their other commands in the feature's worktree, where their pane starts, since the notes' paths run through the main worktree. So decision 19's guard lets the orchestrator `edit` and `create` only in its session's folder, no longer in baton's. On a disk that ignores case, as a Mac's does by default, a path that spells baton's folder in other case gets past this refusal, which stops only the note that a handover names; the guard's `preToolUse` hook keeps the file tools out of baton's folder, in any case (decision 52), but not `bash` (risk 11). baton doesn't refuse a note in a worktree, which only risks a commit, and doesn't remove a stray that is there already. 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. ## 5. Risks @@ -282,7 +285,8 @@ Decisions marked **lead's call** are the lead's, for the user's end review. The 8. **Processes outlive an upgrade** (fact 17): a running `baton wait` or `baton pane` keeps the old binary, so the feature's file carries its format, and an older baton refuses a newer format and keeps the fields it doesn't know (decision 24). 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), but nothing stops a role's other writes there; the guard's `preToolUse` hook, which sees each tool's arguments (fact 4), can keep the file tools out of baton's folder. +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. ## 6. Open questions diff --git a/internal/cli/guard.go b/internal/cli/guard.go new file mode 100644 index 0000000..f0fbf8b --- /dev/null +++ b/internal/cli/guard.go @@ -0,0 +1,184 @@ +package cli + +import ( + "cmp" + "context" + "encoding/json/jsontext" + "encoding/json/v2" + "fmt" + "os" + "path/filepath" + "slices" + + "github.com/codedogapp/baton/internal/flow" + "github.com/codedogapp/baton/internal/guard" +) + +// stopped starts each reason that baton hook guard gives for a tool that it denies, which copilot +// shows the role's agent (decision 52). +const stopped = "baton's guard stopped this call, and none of it ran. " + +// why says why the guard denies a role a command of each kind, and what the role can do instead. +var why = map[guard.Kind]string{ + guard.Outward: "can act outside the Mac, which only the orchestrator does, with the user's " + + "approval. Keep your work local, and say in your handover note what should go out.", + guard.Control: "drives the user's tmux panes or the Mac's apps, which a role never does.", + guard.Agent: "starts another agent, which a role never does: do the work yourself.", + guard.Baton: "isn't a role's command: a role runs `baton wait`, `baton handover`, " + + "`baton ask` and `baton status`.", + guard.Unknown: "runs a program that only the run decides, which the guard can't check: name " + + "the program in the script itself.", +} + +// roleCommands are baton's commands that a role runs, by the names that the guard gives them. +var roleCommands = []string{ + "baton ask", "baton handover", "baton status", "baton wait", + "baton help", "baton -h", "baton -help", "baton --help", +} + +// guardInput is what baton reads of the preToolUse hook's input: the copilot's folder, and the +// tool that the copilot calls, with its arguments (facts 4 and 36). +type guardInput struct { + Dir string `json:"cwd"` + Tool string `json:"toolName"` + Args jsontext.Value `json:"toolArgs"` +} + +// guardOutput is the preToolUse hook's answer that denies the tool, with the reason. +type guardOutput struct { + Decision string `json:"permissionDecision"` + Reason string `json:"permissionDecisionReason"` +} + +// 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). +func hookGuard(ctx context.Context, env Env, args []string) error { + if err := parse(newFlags(env, "hook guard"), args); err != nil { + return err + } + + var input guardInput + if err := readInput(env, &input); err != nil { + return err + } + + role, _, err := fromPane(env.Getenv) + if err != nil || role == flow.RoleOrchestrator { + return err + } + + reason, err := guardReason(ctx, cmp.Or(input.Dir, env.Dir), input) + if err != nil || reason == "" { + return err + } + + return json.MarshalEncode(jsontext.NewEncoder(env.Stdout), guardOutput{"deny", reason}) +} + +// guardReason returns the reason to deny the tool in a role's pane, or "" to let it run: bash by +// its script, and create and edit by the file that they write. The role's other tools read, or +// look after its shells (fact 36). A relative path starts in the folder. +func guardReason(ctx context.Context, dir string, input guardInput) (string, error) { + switch input.Tool { + case "bash": + var args struct { + Command string `json:"command"` + } + + if err := json.Unmarshal(input.Args, &args); err != nil { + return "", fmt.Errorf("read the arguments of bash: %w", err) + } + + return scriptReason(args.Command), nil + case "create", "edit": + var args struct { + Path string `json:"path"` + } + + if err := json.Unmarshal(input.Args, &args); err != nil { + return "", fmt.Errorf("read the arguments of %s: %w", input.Tool, err) + } + + return pathReason(ctx, dir, args.Path) + default: + return "", nil + } +} + +// scriptReason returns the reason to deny a bash script in a role's pane, or "" to let it run: a +// script that the guard can't read, or its first command that the guard watches other than +// baton's commands for a role. +func scriptReason(script string) string { + commands, err := guard.Commands(script) + if err != nil { + return fmt.Sprintf("%sThe guard can't read the script (%v): write it in plain bash.", + stopped, err) + } + + for _, command := range commands { + if !slices.Contains(roleCommands, command.Name) { + return fmt.Sprintf("%s`%s` %s", stopped, command.Name, why[command.Kind]) + } + } + + return "" +} + +// 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). +func pathReason(ctx context.Context, dir, path string) (string, error) { + store, err := openStore(ctx, dir) + if err != nil { + return "", err + } + + if !filepath.IsAbs(path) { + path = filepath.Join(dir, path) + } + + // An empty dir is the process's own working folder, as in main. + if abs, err := filepath.Abs(path); err == nil { + path = abs + } + + if !within(path, store.Dir) { + return "", 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 +} + +// within reports whether the path is the folder or lies in it, by the files that they name, so +// that neither a symlink nor a disk that ignores case hides it. A path that isn't there yet, such +// as a new file's, counts by the part of it that is. A folder that isn't there holds nothing. +func within(path, folder string) bool { + want, err := os.Stat(folder) + if err != nil { + return false + } + + for dir := resolve(filepath.Clean(path)); ; dir = filepath.Dir(dir) { + if info, err := os.Stat(dir); err == nil && os.SameFile(info, want) { + return true + } + + if dir == filepath.Dir(dir) { + return false + } + } +} + +// resolve returns the path with the symlinks resolved in the longest part of it that is there, +// and the rest as it is. +func resolve(path string) string { + resolved, err := filepath.EvalSymlinks(path) + if err == nil || path == filepath.Dir(path) { + return cmp.Or(resolved, path) + } + + return filepath.Join(resolve(filepath.Dir(path)), filepath.Base(path)) +} diff --git a/internal/cli/guard_files_test.go b/internal/cli/guard_files_test.go new file mode 100644 index 0000000..5ed2019 --- /dev/null +++ b/internal/cli/guard_files_test.go @@ -0,0 +1,170 @@ +package cli_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codedogapp/baton/internal/cli" + "github.com/codedogapp/baton/internal/flow" +) + +// TestGuardHookKeepsTheFileToolsOutOfBatonsFolder denies a role's create and edit of a file in +// baton's folder, which only baton writes (decision 48), however the path names it: through a +// symlink, with .., from the copilot's folder, in a new folder, or, on a disk that ignores case, +// in other case. It lets them write anywhere else, even beside baton's folder. +func TestGuardHookKeepsTheFileToolsOutOfBatonsFolder(t *testing.T) { + mainDir, linked := repos(t) + store := started(t, mainDir, fresh()) + + link := filepath.Join(linked, "notes") + if err := os.Symlink(filepath.Join(store.Dir, "features"), link); err != nil { + t.Fatal(err) + } + + // On a disk that ignores case, the path names baton's folder; on another, a folder that isn't + // there. + upper, inOtherCase := filepath.Join(mainDir, ".git", "BATON"), "" + if _, err := os.Stat(upper); err == nil { + inOtherCase = upper + "/x.md" + } + + feature := filepath.Join(store.Dir, "features", slug, "feature.json") + 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. + }{ + {"baton's folder", store.Dir, store.Dir}, + {"a file in it", feature, feature}, + {"a new note", feature + ".md", feature + ".md"}, + {"a file in a new folder", store.Dir + "/a/b/c.md", store.Dir + "/a/b/c.md"}, + {"a path with ..", linked + "/../main/.git/baton/x.md", store.Dir + "/x.md"}, + {"a relative path", "../main/.git/baton/x.md", store.Dir + "/x.md"}, + {"a symlink", link + "/demo-1/feature.json", link + "/demo-1/feature.json"}, + {"a new file behind a symlink", link + "/x.md", link + "/x.md"}, + {"a path in other case", upper + "/x.md", inOtherCase}, + {"the worktree", linked + "/main.go", ""}, + {"git's folder", mainDir + "/.git/config", ""}, + {"a folder beside baton's", mainDir + "/.git/baton-old/x.md", ""}, + {"a session's folder", t.TempDir() + "/files/note.md", ""}, + } + + for _, test := range cases { + for _, tool := range []string{"create", "edit"} { + t.Run(tool+" "+test.name, func(t *testing.T) { + args := map[string]any{"path": test.path, "old_str": "a", "new_str": "b"} + + want := "" + if test.want != "" { + want = guardStopped + test.want + " is in baton's folder " + store.Dir + + ", which only baton writes: write your notes in your session's files/ folder." + } + + input := guardInput(t, linked, tool, args) + wantGuard(t, guardHook(t, linked, paneOf(flow.RoleReviewer), input), 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) { + mainDir, linked := repos(t) + store := started(t, mainDir, fresh()) + + input := guardInput(t, "", "create", map[string]any{"path": "../main/.git/baton/x.md"}) + wantGuard( + t, + guardHook(t, linked, paneOf(flow.RoleLead), input), + guardStopped+store.Dir+"/x.md is in baton's folder "+ + store.Dir+", which only baton writes: write your notes in your session's files/ folder.", + ) +} + +// TestGuardHookLetsTheFileToolsWriteWhereBatonHasNoFolder lets a role's create and edit write in a +// repo without baton's folder, even where the folder would be, since baton keeps nothing there. +func TestGuardHookLetsTheFileToolsWriteWhereBatonHasNoFolder(t *testing.T) { + other := repoNamed(t, "other") + input := guardInput(t, other, "create", map[string]any{"path": other + "/.git/baton/x.md"}) + wantGuard(t, guardHook(t, other, paneOf(flow.RoleLead), input), "") +} + +// TestGuardHookFailsAndDenies fails, with the reason on stderr and nothing on stdout, when it +// can't tell whether to deny the tool; copilot then denies it. +func TestGuardHookFailsAndDenies(t *testing.T) { + mainDir, linked := repos(t) + started(t, mainDir, fresh()) + + outside := t.TempDir() + t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(outside)) + + inLead := paneOf(flow.RoleLead) + push := guardInput(t, linked, "bash", map[string]any{"command": "git push"}) + cases := []struct { + name string + args []string + env map[string]string + input string + want string // The start of the reason, after "baton hook guard: ". + }{ + {"with an argument", []string{"now"}, inLead, push, `unexpected argument "now"`}, + {"without input", nil, inLead, "", "read the hook's input: EOF"}, + {"with input that isn't JSON", nil, inLead, "bash", "read the hook's input: jsontext: "}, + {"with input that isn't an object", nil, inLead, "[]", "read the hook's input: json: "}, + { + "outside a pane", nil, nil, push, + "want BATON_ROLE: run it in a pane that baton started, which names its role there\n", + }, + { + "with bash's arguments a string", nil, inLead, + guardInput(t, linked, "bash", "git push"), "read the arguments of bash: json: ", + }, + { + "without bash's arguments", nil, inLead, + `{"cwd":"` + linked + `","toolName":"bash"}`, + "read the arguments of bash: jsontext: unexpected EOF", + }, + { + "with a path that isn't a string", nil, inLead, + guardInput(t, linked, "edit", map[string]any{"path": 1}), + "read the arguments of edit: json: ", + }, + { + "outside a git repo", nil, inLead, + guardInput(t, outside, "create", map[string]any{"path": outside + "/x.md"}), + "find the git repo: fatal: not a git repository", + }, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + hook := guardEnv(linked, test.env, test.input) + hook.Args = append(hook.Args, test.args...) + got := runEnv(t.Context(), hook) + + want := "baton hook guard: " + test.want + if got.code != 1 || got.stdout != "" || !strings.HasPrefix(got.stderr, want) { + t.Errorf("got %+v; want exit code 1 and %q", got, want) + } + }) + } +} + +// TestGuardHookReportsAnOutputItCantWrite fails when it can't write its answer, which copilot +// then doesn't get, so that copilot denies the tool. +func TestGuardHookReportsAnOutputItCantWrite(t *testing.T) { + var stderr strings.Builder + + hook := guardEnv(t.TempDir(), paneOf(flow.RoleQA), + guardInput(t, "", "bash", map[string]any{"command": "git push"})) + hook.Stdout, hook.Stderr = failing{}, &stderr + + code := cli.Run(t.Context(), hook) + if got := stderr.String(); code != 1 || + got != "baton hook guard: jsontext: write error: output closed\n" { + t.Errorf("exit code %d and %q; want 1 and the output's error", code, got) + } +} diff --git a/internal/cli/guard_test.go b/internal/cli/guard_test.go new file mode 100644 index 0000000..fb93ce3 --- /dev/null +++ b/internal/cli/guard_test.go @@ -0,0 +1,181 @@ +package cli_test + +import ( + "encoding/json/v2" + "strings" + "testing" + + "github.com/codedogapp/baton/internal/cli" + "github.com/codedogapp/baton/internal/flow" +) + +// The reasons that baton hook guard denies a role's tool with (decision 52). +const ( + guardStopped = "baton's guard stopped this call, and none of it ran. " + toOutward = "can act outside the Mac, which only the orchestrator does, with the user's " + + "approval. Keep your work local, and say in your handover note what should go out." + toControl = "drives the user's tmux panes or the Mac's apps, which a role never does." + toAgent = "starts another agent, which a role never does: do the work yourself." + toBaton = "isn't a role's command: a role runs `baton wait`, `baton handover`, " + + "`baton ask` and `baton status`." + toUnknown = "runs a program that only the run decides, which the guard can't check: name " + + "the program in the script itself." +) + +// denied returns the reason that denies a role's script for the command, which the guard names so. +func denied(name, why string) string { return guardStopped + "`" + name + "` " + why } + +// TestGuardHookDeniesARoleWhatTheGuardWatches denies a role's bash script for its first command +// that the guard watches, but baton's commands that a role runs, and a script that the guard can't +// read; it lets the others run. +func TestGuardHookDeniesARoleWhatTheGuardWatches(t *testing.T) { + cases := map[string]string{ + "go test ./... && git commit -am 'Add the demo'": "", + "curl -fsS https://example.com/docs": "", + "": "", + "git push origin HEAD": denied("git push", toOutward), + "gh pr create --fill": denied("gh pr create", toOutward), + "curl -d x https://h/": denied("curl", toOutward), + "tmux send-keys -t 0 y": denied("tmux", toControl), + "osascript -e 'beep'": denied("osascript", toControl), + "claude -p 'Review it'": denied("claude", toAgent), + `"$GIT" push`: denied("… push", toUnknown), + "baton answer demo-1 y": denied("baton answer", toBaton), + "baton feature start x": denied("baton feature", toBaton), + "baton hook stop": denied("baton hook", toBaton), + "baton pane": denied("baton pane", toBaton), + "baton": denied("baton", toBaton), + `baton "$verb"`: denied("baton …", toBaton), + "baton wait": "", + "baton wait -h": "", + "baton handover --to qa": "", + "baton ask 'Which one?'": "", + "baton status": "", + "baton help": "", + "baton -h": "", + "baton -help": "", + "baton --help": "", + "git status && baton wait && git push && tmux ls": denied("git push", toOutward), + "echo 'open": guardStopped + "The guard can't read the script (1:6: reached EOF without " + + "closing quote `'`): write it in plain bash.", + } + + dir := t.TempDir() + + for script, want := range cases { + t.Run(script, func(t *testing.T) { + args := map[string]any{"command": script, "description": "Run it", "mode": "sync"} + input := guardInput(t, dir, "bash", args) + wantGuard(t, guardHook(t, dir, paneOf(flow.RoleEngineer), input), want) + }) + } +} + +// TestGuardHookLetsARolesOtherToolsRun lets a role's tools other than bash, create and edit run, +// whatever their arguments, since they read, or look after the role's shells: even a view of +// baton's folder. +func TestGuardHookLetsARolesOtherToolsRun(t *testing.T) { + mainDir, linked := repos(t) + store := started(t, mainDir, fresh()) + + cases := map[string]any{ + "view": map[string]any{"path": store.Dir}, + "grep": map[string]any{"pattern": "push", "path": store.Dir}, + "glob": map[string]any{"pattern": "**/*.md", "path": store.Dir}, + "web_fetch": map[string]any{"url": "https://example.com/?q=push"}, + "read_bash": map[string]any{"shellId": "7", "delay": 5}, + "stop_bash": map[string]any{"shellId": "7"}, + "list_bash": map[string]any{}, + "task": "git push", + } + + for tool, args := range cases { + t.Run(tool, func(t *testing.T) { + input := guardInput(t, linked, tool, args) + wantGuard(t, guardHook(t, linked, paneOf(flow.RoleQA), input), "") + }) + } +} + +// TestGuardHookSaysNothingInTheOrchestratorsPane lets every tool of the orchestrator's pane run +// on copilot's own terms, since the orchestrator's guard comes later (decision 52). +func TestGuardHookSaysNothingInTheOrchestratorsPane(t *testing.T) { + mainDir, _ := repos(t) + store := started(t, mainDir, fresh()) + + cases := map[string]map[string]any{ + "bash": {"command": "git push && tmux ls"}, + "create": {"path": store.Dir + "/brief.md", "file_text": "Build it.\n"}, + } + + for tool, args := range cases { + t.Run(tool, func(t *testing.T) { + input := guardInput(t, mainDir, tool, args) + wantGuard(t, guardHook(t, mainDir, orchestratorPane(), input), "") + }) + } +} + +// guardHook runs baton hook guard in the pane, from the folder, with the hook's input. +func guardHook(t *testing.T, dir string, env map[string]string, input string) result { + t.Helper() + + return runEnv(t.Context(), guardEnv(dir, env, input)) +} + +// guardEnv returns the environment of baton hook guard in the pane, from the folder, with the +// hook's input. +func guardEnv(dir string, env map[string]string, input string) cli.Env { + hook := hookEnv(dir, env, input) + hook.Args = []string{"hook", "guard"} + + return hook +} + +// guardInput returns the input that copilot gives its preToolUse hook when a pane's copilot calls +// the tool with the arguments in the folder, if any. +func guardInput(t *testing.T, dir, tool string, args any) string { + t.Helper() + + input := map[string]any{ + "sessionId": "0b5d2c4e", "timestamp": 1767225600000, "toolName": tool, "toolArgs": args, + } + if dir != "" { + input["cwd"] = dir + } + + data, err := json.Marshal(input) + if err != nil { + t.Fatal(err) + } + + return string(data) +} + +// wantGuard checks that baton hook guard let the tool run, without a word, if reason is "", and +// otherwise denied it with the reason: a JSON object, on a line of its own. +func wantGuard(t *testing.T, got result, reason string) { + t.Helper() + + if reason == "" { + if got != (result{}) { + t.Errorf("got %+v; want exit code 0 and no output", got) + } + + return + } + + var answer struct { + Decision string `json:"permissionDecision"` + Reason string `json:"permissionDecisionReason"` + } + + if got.code != 0 || got.stderr != "" || !strings.HasSuffix(got.stdout, "}\n") || + json.Unmarshal([]byte(got.stdout), &answer, json.RejectUnknownMembers(true)) != nil { + t.Fatalf("got %+v; want exit code 0 and a JSON object", got) + } + + if answer.Decision != "deny" || answer.Reason != reason { + t.Errorf("got %+v; want the decision deny, with the reason %q", answer, reason) + } +} diff --git a/internal/cli/hook.go b/internal/cli/hook.go index f7b80e9..0460ee4 100644 --- a/internal/cli/hook.go +++ b/internal/cli/hook.go @@ -54,8 +54,8 @@ func hookStop(ctx context.Context, env Env, args []string) error { } var input stopInput - if err := json.UnmarshalDecode(jsontext.NewDecoder(env.Stdin), &input); err != nil { - return fmt.Errorf("read the hook's input: %w", err) + if err := readInput(env, &input); err != nil { + return err } if input.Active { @@ -82,6 +82,16 @@ func hookStop(ctx context.Context, env Env, args []string) error { return json.MarshalEncode(jsontext.NewEncoder(env.Stdout), output) } +// readInput reads the hook's input, a JSON object, into the value, without waiting for the end of +// the input, which copilot might keep open. +func readInput(env Env, input any) error { + if err := json.UnmarshalDecode(jsontext.NewDecoder(env.Stdin), input); err != nil { + return fmt.Errorf("read the hook's input: %w", err) + } + + return nil +} + // stopReason returns the reason that sends the role's copilot back to work, or "" to let it stop. // A role that waits may stop. So may the orchestrator when no work can come to it. A role that // doesn't wait gets the reason to wait if work can come to it, to ask again if its question waits diff --git a/internal/cli/pane.go b/internal/cli/pane.go index f7dfc14..bbb2174 100644 --- a/internal/cli/pane.go +++ b/internal/cli/pane.go @@ -25,7 +25,7 @@ const cacheFolder = "baton" const allowAll = "COPILOT_ALLOW_ALL" // envBin is the variable that names baton's own program in every pane's copilot, for the -// plugin's agentStop hook, which runs baton hook stop (decision 47). +// plugin's hooks, which run baton hook guard and baton hook stop (decisions 47 and 52). const envBin = "BATON_BIN" // pane becomes the pane's copilot, with the role's settings from the config file (decision 49). diff --git a/internal/cli/run.go b/internal/cli/run.go index 57ed003..3be4972 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -21,7 +21,7 @@ type Env struct { Getenv func(string) string // Reads the process's environment. Dir string // The working folder; empty for the process's own. // Stdin is the pane's terminal, which baton pane reads while it waits, or the hook's input, - // which baton hook stop reads. + // which baton's hooks read. Stdin io.Reader Stdout io.Writer Stderr io.Writer @@ -59,6 +59,7 @@ Commands: ask ask the orchestrator a question, in a role's pane, then wait for the answer feature start start a feature from a brief: its branch, its worktree and its tmux session handover hand a feature over to the next role, with a note + hook guard copilot's tool hook: deny a role what the guard watches, before it runs hook stop copilot's stop hook: send a pane's copilot that stops too soon back to work pane become the pane's copilot: the orchestrator's, or the role's once it has work status list the repo's features, with their holder, round and status @@ -84,7 +85,7 @@ var commands = map[string]commandFunc{ // groups are baton's commands of two words, by their first word, then their second. var groups = map[string]map[string]commandFunc{ "feature": {"start": startFeature}, - "hook": {"stop": hookStop}, + "hook": {"guard": hookGuard, "stop": hookStop}, } // usageError is a command line that doesn't parse. The flag package has reported it. diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 768d21c..534ece7 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -11,6 +11,7 @@ Commands: ask ask the orchestrator a question, in a role's pane, then wait for the answer feature start start a feature from a brief: its branch, its worktree and its tmux session handover hand a feature over to the next role, with a note + hook guard copilot's tool hook: deny a role what the guard watches, before it runs hook stop copilot's stop hook: send a pane's copilot that stops too soon back to work pane become the pane's copilot: the orchestrator's, or the role's once it has work status list the repo's features, with their holder, round and status @@ -37,7 +38,7 @@ const handoverUsage = `Usage of baton handover: func TestRunReportsTheUsage(t *testing.T) { wantStart := "baton feature: want start\n\n" + usage - wantStop := "baton hook: want stop\n\n" + usage + wantHook := "baton hook: want guard or stop\n\n" + usage cases := []struct { name string args []string @@ -65,11 +66,16 @@ func TestRunReportsTheUsage(t *testing.T) { result{code: 2, stderr: wantStart}, }, {"with feature start's -h", []string{"feature", "start", "-h"}, result{stderr: startUsage}}, - {"with hook alone", []string{"hook"}, result{code: 2, stderr: wantStop}}, + {"with hook alone", []string{"hook"}, result{code: 2, stderr: wantHook}}, { "with hook's unknown command", []string{"hook", "start"}, - result{code: 2, stderr: wantStop}, + result{code: 2, stderr: wantHook}, + }, + { + "with hook guard's -h", + []string{"hook", "guard", "-h"}, + result{stderr: "Usage of baton hook guard:\n"}, }, { "with hook stop's -h", diff --git a/internal/plugin/files/com.github.copilot/hooks/hooks.json b/internal/plugin/files/com.github.copilot/hooks/hooks.json index 11f1bbc..f6ba464 100644 --- a/internal/plugin/files/com.github.copilot/hooks/hooks.json +++ b/internal/plugin/files/com.github.copilot/hooks/hooks.json @@ -1,6 +1,12 @@ { "version": 1, "hooks": { + "preToolUse": [ + { + "type": "command", + "bash": "\"$BATON_BIN\" hook guard" + } + ], "agentStop": [ { "type": "command", diff --git a/internal/plugin/hooks_test.go b/internal/plugin/hooks_test.go index 6032dea..2da48c6 100644 --- a/internal/plugin/hooks_test.go +++ b/internal/plugin/hooks_test.go @@ -8,9 +8,10 @@ import ( "testing" ) -// TestTheStopHookRunsBaton checks the plugin's hooks: the agentStop hook alone, which runs baton -// hook stop with the program that baton pane names in the pane's copilot (decision 47). -func TestTheStopHookRunsBaton(t *testing.T) { +// TestTheHooksRunBaton checks the plugin's hooks: the preToolUse hook, which runs baton hook guard, +// and the agentStop hook, which runs baton hook stop, each with the program that baton pane names +// in the pane's copilot (decisions 47 and 52). +func TestTheHooksRunBaton(t *testing.T) { data, err := fs.ReadFile(written(t), "com.github.copilot/hooks/hooks.json") if err != nil { t.Fatal(err) @@ -30,7 +31,10 @@ func TestTheStopHookRunsBaton(t *testing.T) { t.Fatalf("hooks.json: %v", err) } - want := map[string][]hook{"agentStop": {{Type: "command", Bash: `"$BATON_BIN" hook stop`}}} + want := map[string][]hook{ + "preToolUse": {{Type: "command", Bash: `"$BATON_BIN" hook guard`}}, + "agentStop": {{Type: "command", Bash: `"$BATON_BIN" hook stop`}}, + } if config.Version != 1 || !maps.EqualFunc(config.Hooks, want, slices.Equal) { t.Errorf("hooks.json is %+v, want the version 1 and the hooks %+v", config, want) } diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index 04db8c4..1a48dd4 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -1,6 +1,7 @@ // 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, and the agentStop hook, -// which runs baton hook stop (decisions 11, 36, 37 and 47). +// 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). package plugin import (