diff --git a/CHANGELOG.md b/CHANGELOG.md index e208e72..15aea29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,33 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.25.0] - 2026-08-28 + +The work a turn does ends as something somebody can review. io-harness has +offered seven git built-ins — `git_status`, `git_diff`, `git_log`, `git_add`, +`git_commit`, `git_branch`, `git_worktree` — on every workspace run since long +before this interface existed, with no feature gate and a fixed argv that can +reach no other subcommand. io-cli had never surfaced one of them. + +The branch the working tree is on is on the status line, read from `.git/HEAD` +rather than from a subprocess, and it follows the agent when it switches. A +commit the agent makes is committed to the scrollback with the message it wrote +and the branch it landed on. `/commit` hands the turn's work to the agent to +describe and stage, under the identity `[run.commit_identity]` names — the +identity 0.14.0 made reach a turn and which nothing had yet read. + +**And it repairs what made all of that unreachable.** Under `ask-writes` — the +posture the wizard recommends — io-harness's git spawn treats an asking `exec` +policy as a hard refusal rather than raising an approval, so every one of the +seven tools was refused before it ran and the operator was never asked. io-cli +now names that refusal, before a turn is spent and whenever one arrives, and +offers the single rule that lifts it: `exec` allowed for `git`, one binary, +for the session. + +A sub-agent can also work in a checkout of its own. `worktree = true` on an +`[[agent]]` entry roots that child under `.worktrees/` on its own branch before +its first step, and the fleet says which children have one. + ## [0.24.0] - 2026-08-28 A turn proves its work instead of asserting it. io-harness has carried a diff --git a/Cargo.lock b/Cargo.lock index d2affcc..12e7027 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.24.0" +version = "0.25.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index 14573d0..b407a14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.24.0" +version = "0.25.0" edition = "2021" # io-harness declares 1.95, set by `libsqlite3-sys` calling the std `cfg_select!` # macro from its build script. Nothing here can be lower than what the harness diff --git a/README.md b/README.md index d2c4611..717f808 100644 --- a/README.md +++ b/README.md @@ -468,6 +468,7 @@ above a row that ranked there for reasons having nothing to do with it. | `/image` | draw an attached image again: /image 1 | | `/copy` | put the last answer on the system clipboard | | `/copy diff` | put the whole run's patch on the system clipboard | +| `/commit` | ask the agent to describe this turn's work and commit it; allow to permit git | **inspect** @@ -496,6 +497,13 @@ above a row that ranked there for reasons having nothing to do with it. | `/gates` | the check a turn must pass before it is done: a command, a file, or a rubric | | `/import` | bring instructions, MCP servers, skills and a model across from another agent | +`/commit` is new in 0.25.0 and sits under **this turn** beside `/copy` and `/copy +diff` because it is the third thing you do with work that has just finished: one +puts the answer somewhere, one puts the patch somewhere, and this one makes the +patch permanent. Its description says *ask the agent* because that is literally +what the word does — it sends a prompt, and the agent reviews, stages and writes +the message. See [Git](#git). + `/plugin` is new in 0.20.0 and sits beside those two because it is the third surface of the same kind: something a configuration file declares by name, whose effect on the session is otherwise invisible. See [Capability @@ -1209,6 +1217,112 @@ back with. `io exec` gains exit `6` for this — the agent finished and the work does not hold up. See [Exit status](#exit-status). +## Git + +A gate says the work holds up. This says what becomes of it. + +The agent has had seven git built-ins on every workspace run since long before +this interface existed: `git_status`, `git_diff`, `git_log`, `git_add`, +`git_commit`, `git_branch` and `git_worktree`. They are io-harness's, and each is +a fixed argv that can reach no other subcommand — there is no `push`, no `remote` +and no `reset` among them. What was missing until 0.25.0 was any of it reaching +you. + +**The branch the working tree is on is on the status line.** It is read out of +`.git/HEAD` as text, because git writes it there in a format that has not changed +in the lifetime of the tool and this program starts no process to ask — so it +costs a file read, and it follows the agent when the agent switches branch. A +detached head is drawn as a short object id rather than as nothing, and a +directory that is not a repository draws no field at all: `io` runs in plenty of +them and must not get worse there. + +**A commit the agent makes is committed into the scrollback** — the branch it +landed on and the message the model wrote. The +diff is not drawn a second time. It is already on screen immediately above, from +the step that wrote it, and drawing it twice would cost you the reason the block +is there at all. + +**`/commit` hands this turn's work to the agent, and the agent writes the +message.** io-cli runs no git and composes no subject line: the command sends a +prompt asking it to review what changed with those tools, stage what belongs to +this turn, and say what the change does and why. That is a billed turn against a +real model, and it is why the row says *ask the agent* rather than promising a +deterministic act. + +**`[run.commit_identity]` decides who the commit is authored as**, and io-cli +reads that value rather than picking one. io-harness hands the name and email +from that section to git on the commit invocation itself, and the section always +resolves to something, so a repository with no identity of its own is told which +default io-harness will use. You are told before the turn is spent, because the +author of a commit is the one thing about it that cannot be corrected afterwards +without rewriting history. + +### The refusal this repairs + +**Before 0.25.0 all seven tools were refused before they ran, for most operators, +and nobody was ever asked.** io-harness's git spawn checks the `exec` policy +itself and accepts only an outright allow. Every other gated act turns an *ask* +into a question on your screen and waits for it; this one returns a refusal +instead, so `ask` behaves exactly as `deny` does. `ask before writes` — the +posture the wizard recommends, and the one this README recommends with it — sets +`exec` to ask. That is the whole of the defect, and it is filed upstream as +io-harness#214. + +What io-cli does about it is name the refusal and offer one rule: `exec` allowed +for `git`, one binary, for this session. `/commit` asks that question *before* it +spends the turn, because a commit the policy was always going to refuse still +costs a real completion to discover. The rule goes through the same remembered +layer as anything else you allow for a session, so it is exactly as strong as +those and no stronger: it widens an asking default and cannot take back a deny +from a layer beneath it. One binary name is also the narrowest grant that works — +an `exec` pattern has no notion of a subcommand, so `git` says *this program may +be spawned* and nothing about any other. + +**Under a posture that denies rather than asks, no rule is offered.** A rule is +matched before a default, so the same allowance would work under `read only` too, +and a keystroke that quietly defeats the one posture whose name is a promise is +not a convenience. What you are told there is to change posture — a decision, and +not a shortcut. + +### A checkout of its own + +Every agent in a tree shares one working directory, so two children editing the +same file are one overwriting the other. `worktree = true` on an `[[agent]]` entry +gives that child its own checkout instead: + +```toml +[[agent]] +name = "reviewer" +worktree = true +``` + +io-harness roots it under `.worktrees/` in your repository, on a new branch +created before the child's first step. **The branch is not named after the +entry** — it is the entry's name, the parent run, the step and a digest of the +child's goal, so `reviewer` becomes something like `reviewer-12-3-a1b2c3d4`. +That is what makes two children of one entry, spawned in the same step, land in +two checkouts rather than one; and since nothing here removes either, it is also +the shape to look for when you go and find the work afterwards. If the worktree +cannot be +made — no git, not a repository, the boundary refusing that path — the spawn +fails with the reason and **no child starts**, rather than quietly sharing the +parent's tree and reintroducing the collision the switch exists to remove. +`/fleet` marks the rows whose roster entry asked for one. See [The +fleet](#the-fleet). + +That mark is a property of the roster entry and not a directory. io-harness +records a child's actual worktree path and hands it back to nobody, so a path +drawn on that row could only be reconstructed — and a reconstruction is an +address that is wrong the moment either side changes, which matters here because +you would `cd` into it. + +**What none of this does.** Nothing removes a worktree and nothing deletes a +branch: that is yours to do, because removing one throws away the work the child +was spawned to produce. io-cli does not open a pull request, and the seven tools +reach no remote at all. The work ends as commits on a branch in your own +checkout, and what happens to it after that is a decision this program does not +make for you. + ## Pictures **Drag a picture onto the prompt, or copy it and paste.** That is the whole of @@ -1771,7 +1885,10 @@ from 0.24.0**, which was the last of these to be named here and was named with i reason: it needed a surface of its own, and `/gates` is that surface. There is still no `[verify]` section, because there is none in io-harness's schema to apply — what a session carries comes from `[app.io-cli.gates]`, which is io-cli's -own. See [Verification gates](#verification-gates). +own. See [Verification gates](#verification-gates). **`[run.commit_identity]` is +*read* from 0.25.0**: it has reached a turn's contract since 0.14.0 like every +other section, and until there was a commit to author it was a value nothing in +this interface had cause to look at. See [Git](#git). That leaves one key still unapplied, and it has a reason too: `run.templates` is the thirteenth `[run]` key, reachable only through its own accessor. It is not a @@ -1790,6 +1907,15 @@ allowlist it found and writes no rule at all. Setting the boundary here is can see what you are granting. See [Bringing your setup across](#bringing-your-setup-across). +**Git stops at your own checkout.** The seven built-ins the agent has are fixed +argvs with no remote among them, so nothing here pushes, fetches or opens a pull +request, and 0.25.0 adds no surface that does. Nothing removes a worktree or +deletes a branch either — both throw away work, and both are yours to decide. And +io-cli starts no git process of its own: the branch on the status line is a read +of `.git/HEAD`, and `/commit` is a prompt. What lifts the tools when your posture +refuses them is a permission rule, not a code path that runs git behind the +policy. See [Git](#git). + **Sixel is still absent**, because encoding it means palette quantisation and another dependency, for terminals that either speak one of the two protocols already here or draw half blocks correctly. The Kitty path covers PNG rather than diff --git a/docs/config.example.toml b/docs/config.example.toml index 86ad83b..1c3d61b 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -95,7 +95,8 @@ net = "deny" # [instructions] AGENTS.md and whatever else you point it at # [[mcp]] MCP servers. Merged with [[app.io-cli.mcp]] below. # [[lsp]] language servers. Merged with [[app.io-cli.lsp]]. -# [[agent]] the roster a fan-out draws its children from +# [[agent]] the roster a fan-out draws its children from. 0.25.0 +# adds `worktree`; see the block below # [web] provider-executed search and fetch # [memory] how much durable memory a run may keep # [[plugin]] capability bundles, declared by path. 0.20.0; see the @@ -207,6 +208,48 @@ net = "deny" # transport = "http" # url = "https://mcp.example.com/v1" +# --- The agent roster (0.25.0) ---------------------------------------------- +# +# io-harness's own section: the roster a contained turn's fan-out draws its +# children from. `name` is what the parent asks for, and every other key narrows +# that child against its parent — a `role`, a `model`, a `max_steps`, an +# `effort`, and the two refusals `deny_write` and `deny_net`. An entry with +# nothing but a name is exactly the child a bare spawn produced before the +# roster existed. +# +# `worktree = true` GIVES ONE CHILD ITS OWN CHECKOUT. Every agent in a tree +# shares one working directory otherwise, so two children editing the same file +# are one overwriting the other. Set this and io-harness roots that child under +# `.worktrees/` in the repository, on a new branch created before its first +# step. THE BRANCH IS NOT NAMED AFTER THIS ENTRY: it is the entry's name, the +# parent run, the step and a digest of the child's goal — `reviewer` becomes +# something like `reviewer-12-3-a1b2c3d4`. That is what makes the line below +# true, and it is the shape to look for when you go and find the work. +# `/fleet` marks the rows whose definition asked for one. +# +# WHAT IT COSTS. Another checkout of the repository on disk, and a branch per +# CHILD rather than per definition — two children of the same entry spawned in +# the same step get one each, which is the collision this key exists to remove. +# It also gives the spawn a new way to fail: no git, not a repository, or the +# boundary above refusing that path, and then NO CHILD STARTS. Quietly sharing +# the parent's tree would be the exact overwrite the key was set to prevent. +# +# AND NOTHING REMOVES ONE. Not io-harness, not io-cli, not the end of the run: +# the worktree and its branch are still there when the tree finishes, because +# removing either throws away the work the child was spawned to produce. So +# `.worktrees/` accumulates until you clear it out yourself. +# +# [[agent]] +# name = "reviewer" +# role = "You read a diff and say what is wrong with it." +# model = "anthropic/claude-haiku-4.5" +# max_steps = 20 +# deny_write = true +# +# [[agent]] +# name = "author" +# worktree = true + # --- Capability bundles (0.20.0) -------------------------------------------- # # A bundle is a directory with a `plugin.toml` in it, and it is loaded because diff --git a/src/app.rs b/src/app.rs index b4831df..ada46be 100644 --- a/src/app.rs +++ b/src/app.rs @@ -318,6 +318,21 @@ pub struct App { /// says `custom` rather than naming one it is not, and the first press of the /// key moves to a posture the operator did choose. posture: Option, + /// Whether this turn has already been told why its git tools did nothing. + /// + /// **One paragraph per turn, not one per refused call.** A model that reaches + /// for git under a posture that refuses it does not stop at the first refusal + /// — it reads the observation, tries `git_status` instead of `git_diff`, and + /// is refused again, five times in a step. The explanation is the same + /// sentence every time, and five copies of it push the transcript that + /// explains them off the screen. The per-call fact is already committed by + /// [`crate::events`], one refusal line each; this is the *reason*, and a + /// reason said twice is noise. + /// + /// Reset by [`App::started`], so the next turn is explained again — the + /// operator may have changed posture between them, and a flag that survived + /// the turn would silence the first refusal of a session that never heard it. + git_explained: bool, } impl App { @@ -352,6 +367,7 @@ impl App { root: std::path::PathBuf::new(), diff_style: crate::settings::DiffStyle::default(), posture: None, + git_explained: false, } } @@ -696,6 +712,55 @@ impl App { &self.remembered } + /// Allow `git` for the rest of this session. + /// + /// **The only door to this rule, and it exists because the ordinary one is + /// shut.** Every other entry in `remembered` arrives through + /// [`App::answer_approval`]: the policy said `Ask`, a question reached the + /// operator, and they answered it with *this session*. The harness's git + /// tools never take that path. `Git::run` refuses anything short of + /// `Effect::Allow` before an approver is consulted at all — see + /// [`crate::approval::refuses_git`] — so under the posture the wizard + /// recommends the seven git tools are refused without anyone ever being + /// asked, and there is no question for an answer to attach to. This method is + /// the answer with no question in front of it. + /// + /// Idempotent, and by value rather than by count: the rule is a fact about + /// the session and not a tally of how many times it was asked for. Pressing + /// the same key twice, or `/commit` reaching here after the refusal already + /// did, must leave one rule — a duplicate would be a second layer entry + /// saying exactly what the first says, which is a policy that is harder to + /// read and no more permissive. + pub fn allow_git(&mut self) { + let rule = crate::approval::git_allowance(); + if !self.remembered.contains(&rule) { + self.remembered.push(rule); + } + } + + /// Say which branch the tree is on. + /// + /// **Backed by [`Status`] rather than by a field of its own, and the reason is + /// worth stating because this release nearly shipped both.** The branch is + /// drawn on the status line and on the `/status` page, so `Status` has to hold + /// it; a second copy here would be a second answer to one question, and the + /// two would part company the first time either was set without the other — + /// which is the shape of the defect 0.17.0 found between `/context` and + /// `ctx N%`, where neither number was wrong and the pair was. + /// + /// So this is an accessor pair over one value. [`crate::repo::branch`] reads + /// `.git/HEAD` for the branch a session starts on; a `git_branch` call names + /// the branch a turn made, which no file read at the end can distinguish from + /// what was already true. Both arrive here. + pub fn set_branch(&mut self, branch: Option) { + self.status.branch = branch; + } + + /// The branch the tree is on, as this session last heard it. + pub fn branch(&self) -> Option<&str> { + self.status.branch.as_deref() + } + pub fn mode(&self) -> Mode { self.mode } @@ -728,6 +793,9 @@ impl App { // just finished can still open the pane and see what happened; it is the // *next* prompt that means those rows are no longer about anything. self.fleet.forget(); + // A new turn is a new posture's worth of chances to be refused, and a new + // operator who has not read the last turn's paragraph. See `git_explained`. + self.git_explained = false; self.quits = 0; self.announce(); } @@ -1082,6 +1150,78 @@ impl App { // there is one counter and not two that can disagree. self.status.unknown = self.events.unknown(); self.pending.extend(lines); + // Last, so the explanation lands *under* the refusal line `Events` just + // committed rather than above it. The order is the argument: the fact, + // then the reason for it. + self.note_git(event); + } + + /// The git surface's share of an event. + /// + /// **Two facts, and neither of them arrives on the `/commit` path.** That is + /// the whole reason this is here and not in the command handler. + /// + /// A `git_branch` call names the branch it is making, and that name is the + /// only place the new branch appears — the file on disk says it afterwards, + /// but by then nothing distinguishes a branch the agent made from the one the + /// session opened on. + /// + /// A refusal of `exec git` reaches here from *any* of the seven git tools, + /// whoever reached for them. `/commit` refuses before it spends a turn, so a + /// commit the operator asked for never gets this far; what does get here is + /// the agent reaching for `git_status` or `git_branch` on its own initiative + /// mid-turn, being refused with no question raised — see [`App::allow_git`] — + /// and the operator watching a turn quietly achieve nothing. Explaining only + /// what `/commit` initiated would leave exactly that case silent, which is the + /// defect this release exists to repair. + fn note_git(&mut self, event: &io_harness::RunEvent) { + // **Delegated rather than reimplemented, and the whole event goes across + // rather than its kind.** [`Status::note_branch`] already decides what a + // `git_branch` announcement means — the target of such a call *is* the + // branch name, and a call that named nothing falls back to the tool's own + // name and must be skipped rather than recorded as a branch called + // `git_branch`. That rule belongs beside the field it fills, and having + // it in one place is what keeps the status line, the `/status` page and + // the commit block naming the same branch. + self.status.note_branch(event); + match &event.kind { + // `"exec"` is io-harness's own word on the wire, and not + // [`crate::approval::act_word`]'s — that one reads `run`, for the + // operator. Matching the operator's vocabulary here would match + // nothing, forever, in silence. + io_harness::EventKind::Refused { act, target, .. } + if act == "exec" && target == crate::approval::GIT && !self.git_explained => + { + self.git_explained = true; + let posture = match self.posture { + Some(posture) => format!("the `{}` posture", posture.short()), + None => "the policy in force".to_string(), + }; + // **The sentence says what to type and does not promise it will + // work, and both halves are corrections.** It named an action — + // allowing `git` — with no way to perform it anywhere in the + // product, so an operator reading it had been told about a door + // and not where it was. And it asserted the allowance *lifts* the + // refusal, which `crate::commit::asked` had already learnt is + // false whenever a rule rather than a tier default decided: a deny + // wins over any later allow, so under a `deny_exec` in the + // operator's own file that promise can never be kept. This arm + // sees an `EventKind::Refused` whose `rule` and `layer` it does + // not receive, so it cannot tell the two apart — and a sentence + // that cannot tell them apart must not claim either. + self.record( + Tone::Refused, + format!( + "{posture} does not let the agent run `git`, and the harness's git \ + tools are refused outright rather than asked about — so nothing \ + stopped to ask you. `/commit allow` permits `git` for this session \ + where the posture is what refused it, and says so when something \ + else did." + ), + ); + } + _ => {} + } } /// Take an event from a run this session is **watching** rather than driving. @@ -1125,6 +1265,33 @@ impl App { } } + /// A commit, committed where it happened. + /// + /// Beside [`App::edits`] and shaped like it: the lines are already text — see + /// [`crate::commit::block`], which builds them without a theme so they can be + /// asserted without one — and the blank line above is the same one a diff + /// gets, because a commit is a block and reads as one under the tool cell that + /// produced it. + /// + /// Through `pending` rather than straight to the terminal, which is what keeps + /// the rewind arithmetic honest: [`App::take_pending`] is the one place rows + /// are counted into `turn_rows`, so a block that went round it would be rows + /// on screen that an undo could not erase. + /// The first line carries the tone and the rest do not, which is what stops + /// `ok:` from being stamped down the left of a commit body. One header and its + /// content is the shape every block in this transcript already has. + pub fn committed(&mut self, lines: Vec) { + let mut lines = lines.into_iter(); + let Some(header) = lines.next() else { + return; + }; + self.pending.push(Line::from("")); + self.pending.push(self.theme.notice(Tone::Success, header)); + for line in lines { + self.pending.push(self.theme.notice(Tone::Normal, line)); + } + } + /// A picture, committed where it happened. /// /// Beside [`App::edits`] and for the same reason: the read that produced it @@ -2285,3 +2452,165 @@ pub fn gate_retry( ), } } + +#[cfg(test)] +mod tests { + use super::*; + use io_harness::EventKind; + + /// A refusal exactly as `Git::run` raises one: act `exec`, target the program + /// name, and no rule — the posture's own default decided. + fn git_refused() -> RunEvent { + RunEvent::new( + 1, + 1, + EventKind::Refused { + act: "exec".to_string(), + target: crate::approval::GIT.to_string(), + rule: None, + layer: None, + }, + ) + } + + fn app() -> App { + let mut app = App::new(crate::theme::DARK, "m"); + app.started(); + app + } + + fn text(app: &mut App) -> String { + app.take_pending() + .iter() + .flat_map(|line| line.spans.iter()) + .map(|span| span.content.to_string()) + .collect::>() + .join("") + } + + /// **F2's second half, and the arm its sabotage breaks.** The refusal arrives + /// as an event and nothing about it says `/commit`: this is the agent reaching + /// for a git tool on its own. Gating the explanation on the `/commit` path + /// leaves this silent, which is exactly the defect the release repairs. + #[test] + fn f2_an_unprompted_git_refusal_is_explained() { + let mut app = app(); + app.event(&git_refused(), Duration::ZERO); + let said = text(&mut app); + assert!( + said.contains("run `git`") && said.contains("/commit allow"), + "an unprompted git refusal said: {said:?}" + ); + } + + /// The posture is named, because "something refused this" is the sentence an + /// operator cannot act on. + #[test] + fn f2_the_explanation_names_the_posture_that_decided() { + let mut app = app(); + app.set_posture(Some(Posture::AskWrites)); + app.event(&git_refused(), Duration::ZERO); + assert!(text(&mut app).contains("`ask-writes` posture")); + } + + /// A model refused once retries, and five refusals are five facts but one + /// reason. The per-call lines are `crate::events`'s and stay; the paragraph is + /// said once. + #[test] + fn f2_a_turn_that_retries_git_is_explained_once() { + let mut app = app(); + for _ in 0..5 { + app.event(&git_refused(), Duration::ZERO); + } + assert_eq!(text(&mut app).matches("/commit allow").count(), 1); + } + + /// And the next turn is explained again: the flag is about a turn, not about + /// the session. + #[test] + fn f2_the_next_turn_is_explained_again() { + let mut app = app(); + app.event(&git_refused(), Duration::ZERO); + let _ = text(&mut app); + app.started(); + app.event(&git_refused(), Duration::ZERO); + assert!(text(&mut app).contains("/commit allow")); + } + + /// A refusal of something that is not git explains nothing about git. + #[test] + fn f2_another_act_is_not_a_git_refusal() { + let mut app = app(); + app.event( + &RunEvent::new( + 1, + 1, + EventKind::Refused { + act: "write".to_string(), + target: "/etc/hosts".to_string(), + rule: None, + layer: None, + }, + ), + Duration::ZERO, + ); + assert!(!text(&mut app).contains("/commit allow")); + } + + #[test] + fn f2_the_allowance_is_reachable_and_pushing_it_twice_leaves_one() { + let mut app = app(); + app.allow_git(); + app.allow_git(); + assert_eq!(app.remembered(), [crate::approval::git_allowance()]); + } + + #[test] + fn f2_a_branch_call_names_the_branch_it_made() { + let mut app = app(); + app.event( + &RunEvent::new( + 1, + 1, + EventKind::ToolCall { + name: io_harness::tools::GIT_BRANCH_TOOL.to_string(), + target: "feat/0.25.0".to_string(), + }, + ), + Duration::ZERO, + ); + assert_eq!(app.branch(), Some("feat/0.25.0")); + } + + /// A `git_branch` announced with no `name` argument falls back to the tool's + /// own name. That is a malformed call, not a branch called `git_branch`. + #[test] + fn f2_a_nameless_branch_call_names_nothing() { + let mut app = app(); + app.event( + &RunEvent::new( + 1, + 1, + EventKind::ToolCall { + name: io_harness::tools::GIT_BRANCH_TOOL.to_string(), + target: io_harness::tools::GIT_BRANCH_TOOL.to_string(), + }, + ), + Duration::ZERO, + ); + assert_eq!(app.branch(), None); + } + + /// The block goes through `pending`, so `take_pending` counts its rows into + /// `turn_rows` and a rewind can erase what it drew. + #[test] + fn f2_a_commit_block_is_counted_by_the_rewind_arithmetic() { + let mut app = app(); + app.committed(vec![ + "committed on main".to_string(), + " subject".to_string(), + ]); + assert_eq!(app.take_pending().len(), 3); + assert_eq!(app.turn_rows(), 3); + } +} diff --git a/src/approval.rs b/src/approval.rs index 11d99db..0dfae8f 100644 --- a/src/approval.rs +++ b/src/approval.rs @@ -733,3 +733,51 @@ pub fn session_policy( } effective_policy(&policy, remembered) } + +/// The program `io_harness::tools::git` spawns, spelled the way the harness +/// spells it when it asks the policy about it. +/// +/// A bare name and not a path: the harness passes this exact string to +/// `Policy::check(Act::Exec, ..)`, and an `Act::Exec` pattern is compared against +/// the target and against its basename — never against a command line — so the +/// name is the whole vocabulary a rule about git has. +pub const GIT: &str = "git"; + +/// Will this policy refuse a git spawn? +/// +/// `Ask` counts as a refusal here, and that is the entire point. Every other +/// gated act in the harness turns `Effect::Ask` into a question for the +/// [`Approver`]; git's spawn does not. `Git::run`, at `src/tools/git.rs:549-561` +/// of io-harness 0.69.0, checks `self.policy.check(Act::Exec, &self.program)` and +/// returns `Error::Refused` on **anything that is not `Allow`**, before any +/// approver exists to be consulted. So the question a caller needs answered is +/// not *does this ask* but *does this fall short of allow*, which is what this +/// answers. +pub fn refuses_git(policy: &io_harness::Policy) -> bool { + policy.check(Act::Exec, GIT).effect != Effect::Allow +} + +/// The rule that lets the git tools run at all. +/// +/// [`crate::settings::Posture::AskWrites`] sets `exec: Effect::Ask`, and it is the +/// posture the wizard recommends — so on the reading in [`refuses_git`] the seven +/// git tools are refused, for most operators, without anyone ever being asked. The +/// repair is a rule, not a special case in the spawn path: io-cli offers this +/// through the same `remembered` layer [`effective_policy`] already builds, so a +/// git allowance is exactly as strong as any other thing the operator allowed for +/// the session — it widens an *asking* default and still cannot re-allow a deny +/// from a layer beneath it, because layers add capability and never take a denial +/// back. +/// +/// The pattern is one binary name. `Act::Exec` matching (see [`GIT`]) has no +/// notion of a subcommand, so `"git"` is the narrowest expressible grant that +/// makes the tools work: it says *this program may be spawned* and nothing about +/// any other. A pattern like `"*"` would answer the same criterion by handing over +/// every binary on the machine in order to hand over one. +pub fn git_allowance() -> Rule { + Rule { + act: Act::Exec, + effect: Effect::Allow, + pattern: GIT.into(), + } +} diff --git a/src/commands.rs b/src/commands.rs index a26644f..febb302 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -163,6 +163,24 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/copy diff", "put the whole run's patch on the system clipboard", ), + // Beside `/copy` and `/copy diff` because it is the third thing an operator + // does with the work a turn has just finished: one puts the answer somewhere, + // one puts the patch somewhere, and this one makes the patch permanent. Same + // subject, same moment, one row apart — which is the argument `GROUPS` already + // writes out for those two, and the reason this is filed under `Turn` rather + // than under the group that merely shows things. + // + // **The description says "ask the agent" because that is literally what + // happens.** io-cli runs no git: the word sends a prompt, and it is the agent + // that reviews what changed with io-harness's git tools, stages what belongs + // and writes the message — see [`crate::commit::prompt`]. A row reading + // "commit this turn's work" would promise a deterministic act and deliver a + // billed turn whose result the operator can still be surprised by, which is + // the one thing a command that writes to a repository must not do. + ( + "/commit", + "ask the agent to describe this turn's work and commit it; allow to permit git", + ), ( "/config", "every setting, the value in force and the file that decided it", @@ -268,16 +286,21 @@ pub const COMMANDS: &[(&str, &str)] = &[ /// /// **Grouped by the operator's intent rather than by which part of the harness /// answers**, because the second is an implementation detail and the first is -/// the only thing somebody scanning a list of thirty-one is holding in their +/// the only thing somebody scanning a list of thirty-two is holding in their /// head. /// /// Four groups and none longer than ten, which is the bound `tests/commands.rs` -/// asserts. A flat list of thirty-one is a list nobody reads; 0.16.0 is the +/// asserts. A flat list of thirty-two is a list nobody reads; 0.16.0 is the /// release that grouped them, at twenty, and every release since has added to a /// group rather than to a list. **The count in this paragraph is the one number /// here that goes stale on its own** — it said twenty through 0.17.0, which had /// twenty-three, and twenty-six through 0.23.0, which had thirty — so it is /// written out rather than left as "a few". +/// +/// **Two of the four groups are now at nine or better and `Turn` is at the bound +/// itself**, which is 0.25.0's doing; what happens when the next command has +/// nowhere to go is answered in the `/gates` note at the foot of [`GROUPS`] and +/// is not re-decided here. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum Group { /// The conversation itself: start one, leave one, come back to one. @@ -364,6 +387,24 @@ pub const GROUPS: &[(Group, &[&str])] = &[ "/image", "/copy", "/copy diff", + // **`/commit` takes `Turn` to ten of ten, which is the bound.** It is + // here on the argument the note above this group already makes, and + // not on there being room: + // it acts on the work the turn just finished — the same subject + // `/image`, `/copy` and `/copy diff` have — and the thing a command + // acts *on* is what decides its group. It asks the store nothing, + // which is what `Inspect` means, and it writes no configuration file, + // which is what `Configure` means. + // + // So this group is full, and the answer for the next command that + // would fill one is already written down rather than owed: the + // `/gates` note under `Configure` below — `src/commands.rs:447-449` — + // pre-committed it when that group reached nine, and this is the + // release that has to keep it. Re-file what is in the wrong group; do not + // widen the bound. Two groups have now stood at the edge of it and + // neither release moved it, which is the whole value of having said so + // in advance. + "/commit", ], ), ( @@ -939,6 +980,40 @@ pub enum Action { Context, /// Put something on the system clipboard over OSC 52. Copy(Copied), + /// Ask the agent to describe the work this turn did, and to commit it. + /// + /// **A prompt and not a git invocation, and that difference is the whole of + /// what the word means.** This crate runs no git of its own — io-harness keeps + /// the engine private, and the one module here allowed to reach a subprocess + /// at all is [`crate::shell`] — so the action sends the sentence + /// [`crate::commit::prompt`] builds and the *agent* does the work: it reviews + /// what changed with io-harness's git tools, stages what belongs to this turn, + /// and writes the message. What comes back is a `git_commit` call, which + /// [`crate::commit::made_in`] reads because it is the only durable record that + /// a commit happened — no `EventKind` carries a message, a branch or an object + /// id, and no `Store` method returns one. + /// + /// **The message is never an argument, and that is a decision rather than an + /// omission.** A commit message is a subject, a blank line and a body; none of + /// that is a word that fits after a slash, and offering half of one would put + /// io-cli back in the business of writing the message — which is precisely + /// what the command exists not to do. + /// + /// **The one word it does take is `allow`, and it exists because the refusal + /// would otherwise be an offer nobody can accept.** io-harness's git spawn + /// answers an *asking* `exec` posture with a refusal instead of an approval, + /// so no `ApprovalRequested` is ever raised and the operator never reaches the + /// remember path every other act has. `/commit` says so and names the one rule + /// that lifts it; `/commit allow` is how they take it. The rule covers the + /// single binary `git`, lasts for the session, and is the same + /// [`crate::approval::git_allowance`] the refusal named — a keystroke that + /// grants exactly what was described and nothing else. + /// + /// `true` means the operator asked for the allowance as well as the commit. + /// The word is spelled the way `/contain on` is: an argument on the existing + /// row rather than a second command, because it is one answer to one question + /// and a `COMMANDS` row is for a surface rather than for an answer. + Commit(bool), /// Put the whole conversation back into the scrollback. Transcript, /// Clear the screen and start a new conversation. @@ -1792,6 +1867,24 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { Some("diff") | Some("patch") => Action::Copy(Copied::Diff), _ => Action::Copy(Copied::Answer), }, + // **One spelling, and everything after it ignored.** `/ci` and `/save` are + // both words a hand might reach for and neither has been typed at this + // prompt yet, which is the rule `/status` and `/compact` already follow: a + // second name is a name to keep working forever in exchange for nothing. + // + // And no argument is read, not even the rest of the line. `/remember` takes + // the rest because the sentence IS the thing being stored; here the message + // is the agent's to write, and a half-typed subject picked up off the prompt + // would be io-cli writing it after all — see [`Action::Commit`]. + // `allow` and nothing else, because there is exactly one thing to say + // here beyond the word itself. An unrecognised second word commits + // without the allowance rather than refusing the whole line: the operator + // asked for a commit, and the worst case is the refusal they were already + // going to get, said again with the rule named. + "commit" => Action::Commit(matches!( + input.split_whitespace().nth(1), + Some("allow") | Some("allow-git") + )), unknown => { let mut lines = vec![theme.notice( Tone::Warning, diff --git a/src/commit.rs b/src/commit.rs new file mode 100644 index 0000000..be82431 --- /dev/null +++ b/src/commit.rs @@ -0,0 +1,279 @@ +//! What the agent committed, and the prompt that asks it to. +//! +//! io-harness offers `git_commit` on every workspace run and has since long +//! before this interface existed. What it does **not** offer is any record of the +//! commit as a fact: no `EventKind` variant carries a message, a branch or an +//! object id, and no `Store` method returns one. The commit message survives in +//! exactly one place — the tool call the model made — and this module reads it +//! from there. +//! +//! # Read the typed call, never the display string +//! +//! `Store::step_turns` returns `AssistantTurn`s whose `calls` are typed +//! `ToolCall`s: a name and a `serde_json::Value` of arguments. That is the shape +//! this module reads. +//! +//! `StepRecord::tool_call` also contains the message, as +//! `git_commit:{"message":"…"}` joined by `" | "` — and it is **not** used here. +//! That joining is an internal display convention of the dependency rather than a +//! documented format, and splitting it on `:` fails on the first commit message +//! containing a colon, which is most of them. +//! +//! # A call is a decision, not an outcome +//! +//! io-harness stages an `AssistantTurn` from the provider's response, with the +//! calls exactly as the model produced them, *before* any of them is dispatched. +//! So a `git_commit` the policy refused, or one git rejected, is in `calls` beside +//! one that landed and reads identically there. Nothing this module can see +//! separates them, and no `EventKind` carries a tool's result either — which is +//! why [`made_in`] answers "what the model asked for" and the block is committed +//! by the caller, which watched the refusal and the failure go past. That +//! boundary is stated here rather than guessed at, because the wrong half of it +//! is a scrollback claiming a commit that does not exist. +//! +//! # The block does not re-draw the diff +//! +//! `crate::events` states the rule this module is an exception to: one cell per +//! call, because uncollapsed tool output is what makes a transcript unreadable. +//! The exception is narrow — a commit is the one tool call whose *whole purpose* +//! is to be read later — and it stays narrow by naming what git reported rather +//! than re-drawing hunks. The step's diffs are already on screen immediately +//! above, committed by the driver's edit path, and drawing them twice would cost +//! the reader the thing the exception was made for. + +use io_harness::Identity; + +/// The argument `git_commit` carries the message in. +/// +/// Named rather than spelled inline because it is the one string in this module +/// that comes from the dependency's tool schema, and a typo in it produces a +/// commit block that is silently always empty. +pub const MESSAGE: &str = "message"; + +/// The tool whose calls this module reads. +pub const TOOL: &str = io_harness::tools::GIT_COMMIT_TOOL; + +/// A commit the agent made during a turn. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Made { + /// The step whose call made it. + pub step: u32, + /// The message the agent wrote, exactly as it was passed to `git_commit`. + pub message: String, +} + +impl Made { + /// The message's first written line, which is what a one-line summary shows. + /// + /// A commit message is a subject and an optional body separated by a blank + /// line; showing the whole thing on a status row would push the body through + /// a surface that has no room for it. + /// + /// **First *written* line, not first line.** A model that opens its message + /// with a newline has still written a subject, and taking `lines().next()` + /// there answers `""` — a status row that has gone blank for a commit that + /// has a perfectly good subject one line below. The scan stops at the first + /// line with something on it, so nothing further down can be promoted past a + /// real subject. + pub fn subject(&self) -> &str { + self.message + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or_default() + } +} + +/// Every commit made in the turns given, oldest first. +/// +/// Pure over the store's own rows so it can be asserted without a store: the +/// caller passes what `Store::step_turns` returned. A call carrying no `message` +/// argument, one whose `message` is not a string, and one whose message is only +/// whitespace are all skipped rather than rendered as an empty commit — each of +/// those means the model called the tool wrongly and the harness refused it, not +/// that somebody committed nothing. +/// +/// Order is the order the model worked in: turns arrive from `step_turns` +/// oldest step first, and a turn's `calls` are in the order it made them, so +/// walking both in sequence is already chronological. Nothing sorts afterwards, +/// because a sort on `step` alone would shuffle two commits made in one step +/// into whichever order the sort happened to be stable about. +/// +/// A turn holding one `git_commit` among a batch of reads and writes yields that +/// one call and nothing else; the name check is per call, not per turn. +pub fn made_in(turns: &[io_harness::AssistantTurn]) -> Vec { + let mut out = Vec::new(); + for turn in turns { + for call in &turn.calls { + if call.name != TOOL { + continue; + } + let Some(message) = call.arguments.get(MESSAGE).and_then(|m| m.as_str()) else { + continue; + }; + if message.trim().is_empty() { + continue; + } + out.push(Made { + step: turn.step, + message: message.to_string(), + }); + } + } + out +} + +/// The lines a commit commits into the scrollback. +/// +/// Plain strings rather than styled lines, for the reason every other page in +/// this crate returns text: the tone is the caller's, and a module that builds +/// styled lines cannot be asserted without a theme. +/// +/// `branch` is where it landed, optional because it can genuinely be unknown — a +/// contained child commits in a checkout this process cannot name — and a line +/// saying "unknown" is worse than a line that is not there. A branch that is +/// present but blank is treated as absent for the same reason: it is an unknown +/// wearing a value's clothes. +/// +/// **There is deliberately no "what git printed" parameter, and one was removed +/// to make that true.** It had a single production call site, passing `None` +/// forever, because no `EventKind` carries a tool's result — the module header +/// says so. Only the tests ever supplied it, which made it dead flexibility, and +/// the README carried a sentence describing a line no operator could ever see. +/// +/// Only a commit that landed reaches here. See the module note on why a refused +/// or failed `git_commit` is the caller's to filter and not this function's to +/// detect. +pub fn block(made: &Made, branch: Option<&str>) -> Vec { + let mut lines = Vec::new(); + lines.push(match branch.map(str::trim).filter(|b| !b.is_empty()) { + Some(branch) => format!("committed on {branch}"), + None => "committed".to_string(), + }); + for line in made.message.lines() { + // A blank line in the body stays blank rather than becoming two spaces. + // Indenting nothing is invisible on screen and load-bearing everywhere + // else — it is what a copied block pastes back, and what a test that + // compares committed lines has to spell. + lines.push(if line.trim().is_empty() { + String::new() + } else { + format!(" {line}") + }); + } + lines +} + +/// The prompt `/commit` hands to the agent. +/// +/// io-cli does not write the message. It asks the agent to describe the work it +/// just did and to stage it, which is the whole of what the command means — and +/// it is why this is a prompt rather than a git invocation. This crate cannot run +/// git at all: the engine is `pub(crate)` in the dependency, and +/// `tests/dependencies.rs` permits a process spawn in `src/shell.rs` alone, +/// identified by path. That gate matches raw text, comments included, so the +/// forbidden name is not spelled out here — not even to say this module does not +/// use it, which is a sentence that reddens the gate it is describing. +pub fn prompt() -> String { + "Commit the work from this turn. Review what changed with the git tools, stage \ + what belongs in this commit, and write a message describing what the change \ + does and why. Do not commit files unrelated to this turn's work." + .to_string() +} + +/// What `/commit` may do, given the policy in force. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Asked { + /// Nothing refuses git. Buy the turn. + Ready, + /// An **asking default** refuses it, and [`crate::approval::git_allowance`] + /// would lift it. The sentence names the rule; `/commit allow` takes it. + Offer(String), + /// Something refuses it that the allowance cannot lift, or must not. + Refuse(String), +} + +/// Whether `/commit` may spend a turn, and what to say when it may not. +/// +/// **The check happens before the turn, not after it.** A commit the policy was +/// always going to refuse still costs a real completion against a real model to +/// discover, so the one question worth asking first is whether git can run at all. +/// +/// **The three answers turn on the whole verdict and not on the tier default, and +/// this release shipped the shallower version first.** `Verdict` carries `rule`, +/// which is `None` exactly when the tier default decided, so: +/// +/// - `Allow` — [`Asked::Ready`]. +/// - `Ask` **with no rule** — an asking *default*. The operator chose a posture +/// that promises to ask and the harness's git spawn does not ask, so naming the +/// one rule that lifts it tells them what their own posture meant to do. This +/// is the only case the allowance is offered in, because it is the only case +/// where the allowance both helps and is honest. +/// - **Anything else** — [`Asked::Refuse`], and the allowance is neither offered +/// nor applied. Two distinct situations arrive here and both are traps: +/// +/// A **denying default** is `read only`. A rule is matched *before* a default, +/// so the allowance would in fact work there — which is exactly why it must not +/// be offered. A keystroke that defeats the one posture whose name is a promise +/// is not a convenience, and worse, the `.git` write gate would refuse the +/// commit anyway, so the turn would be bought for nothing. +/// +/// A **rule** — a `deny_exec` in the operator's own configuration, or a deny +/// with an allowlist — cannot be lifted by a later allow at all, because deny +/// wins across layers. Offering the allowance there would print advice that can +/// never be taken, on every attempt, forever. +pub fn asked(policy: &io_harness::Policy) -> Asked { + let verdict = policy.check(io_harness::Act::Exec, crate::approval::GIT); + match (verdict.effect, verdict.rule.as_deref()) { + (io_harness::Effect::Allow, _) => Asked::Ready, + (io_harness::Effect::Ask, None) => Asked::Offer( + "this posture asks before running a command, and the harness's git tools are refused \ + rather than asked — so a commit would be refused after the turn was paid for. Run \ + `/commit allow` to permit `git` for this session and commit." + .to_string(), + ), + // A rule decided, whatever it said. Name the layer, because a refusal an + // operator cannot locate in their own files is one they cannot act on. + (_, Some(rule)) => Asked::Refuse(format!( + "a rule in the policy refuses git ({rule}{}), so the agent cannot commit. Allowing \ + `git` for the session cannot lift it — a deny wins over any later allow — so this is \ + a change to make in the file the rule came from.", + verdict + .layer + .as_deref() + .map(|layer| format!(" in {layer}")) + .unwrap_or_default(), + )), + // A denying default: `read only`, and it is a posture rather than a rule. + (_, None) => Asked::Refuse( + "this posture does not let the agent run a command, so it cannot commit. Change the \ + posture if you want it to — `/commit allow` deliberately will not, because a rule \ + beats a default and this one would quietly undo the posture you chose." + .to_string(), + ), + } +} + +/// The sentence naming who a commit will be authored as. +/// +/// Shown before the turn is spent, because the identity is the one thing about a +/// commit that cannot be corrected afterwards without rewriting history. +/// +/// **This function has no default of its own, and that is the whole of it.** +/// `[run.commit_identity]` reaches `TaskContract::commit_identity` through the +/// harness's own `Config::apply_to`, and io-harness passes that value to git as +/// `-c user.name=… -c user.email=…` on the commit invocation itself. The field is +/// an `Identity` rather than an `Option`, already carrying +/// `Identity::default()` when the operator configured no section — so a +/// repository with no identity of its own is told which default *io-harness* will +/// use, by reading the same value git will be handed. +/// +/// A name chosen here instead would be a string this crate invented, written into +/// the operator's history by a tool that was only asked to describe it. There is +/// nowhere else to correct that afterwards, which is why the branch that would +/// pick one does not exist rather than being merely unused: an identity is read +/// and printed, never substituted, not even for a value this crate recognises as +/// the default. +pub fn authored_as(identity: &Identity) -> String { + format!("authored as {} <{}>", identity.name, identity.email) +} diff --git a/src/fleet.rs b/src/fleet.rs index 1159409..57d7158 100644 --- a/src/fleet.rs +++ b/src/fleet.rs @@ -51,7 +51,7 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use ratatui::Frame; -use io_harness::{AgentMessage, Agents, EventKind, RunEvent}; +use io_harness::{AgentDef, AgentMessage, Agents, EventKind, RunEvent}; use crate::glyphs::Glyphs; use crate::picker::fit; @@ -116,6 +116,24 @@ pub struct Child { /// see [`Fleet::name`] for the two shapes that resolve and the one that /// cannot. pub role: Option, + /// Whether the definition it was spawned from asked for its own worktree. + /// + /// **A property of the roster entry, not a directory.** io-harness writes a + /// contained child's actual worktree path into its `runs` row and no query + /// ever selects it back out, and the functions that derive one are private to + /// the harness — so a path drawn here could only be *reconstructed*, and a + /// reconstruction is an address that disagrees with the truth the moment + /// either side changes. That is the same rule `DERIVED_MARK` states for the + /// role — named without a doc link because it is private to this module and a + /// public item may not link one: copy a constant only where being wrong is + /// harmless. Being wrong + /// about a directory is not harmless — an operator would `cd` into it. + /// + /// So this is the one bit that *is* knowable: `contract.agents` says whether + /// the definition carries `worktree = true`, and that is what the row says. + /// `false` where the roster cannot name this child at all, which is the same + /// honest silence [`Child::role`] keeps. + pub worktree: bool, /// Its nesting level: one below the event that announced it. pub depth: u32, /// What it was asked to do. @@ -308,14 +326,22 @@ impl Fleet { /// `n(n+1)/2`. Nothing here re-walks history. pub fn name(&mut self, addresses: &[(String, i64)], roster: &Agents) { for (address, run_id) in addresses { - let role = role_of(address, roster); + let def = def_of(address, roster); + let role = def.map(|def| def.name.clone()); + // Read off the same definition the role came from rather than looked + // up again: two lookups of one address are two chances to disagree. + let worktree = def.is_some_and(|def| def.worktree); if let Some(child) = self.child_mut(*run_id) { child.address = Some(address.clone()); - // Assigned rather than merged: `role_of` is a pure function of the + // Assigned rather than merged: `def_of` is a pure function of the // address and the roster, so a `None` here means the roster cannot // name this one, and keeping a stale role after an `/agents` // reload would be showing a definition that no longer exists. child.role = role; + // The same rule, and it matters more: a stale `true` would keep + // saying a child is contained after the definition that contained + // it was edited away. + child.worktree = worktree; } } } @@ -380,6 +406,10 @@ impl Fleet { // reads one — see `Fleet::name`. address: None, role: None, + // Not knowable from the event either: `Spawned` carries a run + // id and a goal, and the roster entry behind them is what says + // whether this one is contained. + worktree: false, depth: event.depth + 1, goal: goal.clone(), state: State::Working, @@ -498,8 +528,20 @@ impl Fleet { .map(|child| { let indent = " ".repeat(child.depth.saturating_sub(1) as usize); let drawn = crate::status::format_tokens(child.drawn); + // A word, not a symbol, and so it needs no entry in [`Glyphs`]: + // `worktree` is the same eight letters in both sets, which is the + // test that set applies — a mark goes in it when the two sets + // would otherwise spell one thing differently, as `arrow` does. + // It is also the word the roster spells, so an operator reading + // the row and an operator reading `contract.agents` are reading + // the same term. + let contained = if child.worktree { + format!("worktree {sep} ") + } else { + String::new() + }; let head = format!( - "{indent}{} {sep} {} {sep} {drawn} drawn {sep} ", + "{indent}{} {sep} {} {sep} {drawn} drawn {sep} {contained}", child.label(), child.state.word(), ); @@ -602,6 +644,10 @@ impl Fleet { /// The `[[agent]]` definition an address was spawned from, where the address can /// still say. /// +/// The definition itself rather than its name, because two things on the row come +/// off it — the role label and whether the child is contained — and one lookup +/// cannot disagree with itself. +/// /// Two shapes resolve and one cannot, and the one that cannot is why this returns /// an `Option` rather than a guess: /// @@ -621,9 +667,9 @@ impl Fleet { /// /// Pure and bounded: one split and one `BTreeMap` lookup, no allocation on the /// path that fails. -fn role_of(address: &str, roster: &Agents) -> Option { +fn def_of<'a>(address: &str, roster: &'a Agents) -> Option<&'a AgentDef> { let stem = address.split(DERIVED_MARK).next().unwrap_or(address); - roster.get(stem).map(|def| def.name.clone()) + roster.get(stem) } /// The mark between a sender and a recipient, in whichever set is in play. diff --git a/src/lib.rs b/src/lib.rs index ef54c3a..055dc12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,20 @@ //! mid-flight exactly as an uncontained one can, and containment decides fan-out //! and nothing else. See [`queue`] for the surface an operator says it through. //! +//! Since 0.25.0 the work a turn does can end as something somebody reviews. The +//! harness has offered seven git built-ins on every workspace run since long +//! before this interface existed, and this crate had never surfaced one of them: +//! see [`repo`] for the branch the working tree is on, read from `.git/HEAD` +//! rather than from a subprocess, and [`commit`] for what the agent committed and +//! for the prompt `/commit` hands it. +//! +//! **The reason that capability was unreachable is a policy fact, not a rendering +//! one.** The harness's git spawn treats an *asking* `exec` posture as a refusal +//! rather than raising an approval, so under the posture the wizard recommends +//! every one of the seven tools is refused before it runs and the operator is +//! never asked. [`approval::git_allowance`] is the single rule that lifts it, and +//! it names one binary. +//! //! [io-harness]: https://docs.rs/io-harness pub mod app; @@ -35,6 +49,7 @@ pub mod bridge; pub mod cli; pub mod clipboard; pub mod commands; +pub mod commit; pub mod compact; pub mod complete; pub mod composer; @@ -71,6 +86,7 @@ pub mod providers; pub mod queue; pub mod recall; pub mod reload; +pub mod repo; pub mod resume; pub mod reviewer; pub mod rewind; diff --git a/src/main.rs b/src/main.rs index 1bfc088..1a0d405 100644 --- a/src/main.rs +++ b/src/main.rs @@ -873,6 +873,14 @@ async fn loop_over Result>( let mut provider = make(&model)?; let mut app = App::new(theme, model); app.set_diff_style(diff_style); + // **Before the first keystroke, because the branch is true before the first + // turn and the field claiming otherwise would be a lie of omission.** Every + // other read of it is at a turn boundary, which meant an operator opening + // `io` in a repository saw no branch at all until they had spent a turn — + // while the README and the CHANGELOG both say the branch the working tree is + // on is on the status line. One file read at startup makes that sentence true + // from the first frame. + app.set_branch(io_cli::repo::branch(session.root())); // Before anything is drawn and before the first keystroke, so the keys are // never briefly the defaults, and so the notices are the first thing in the // scrollback rather than something that appears under an answer. @@ -2480,7 +2488,76 @@ async fn loop_over Result>( continue; } - match app.key(key) { + // **`/commit` is a submit, not a report, so it is rewritten here rather + // than answered in the `Command::Slash` arm below.** Every other command + // in that arm reports something or writes a file; this one hands the + // turn's work to the agent, which means it has to become the same + // `Command::Submit` a typed prompt becomes — otherwise the prompt would + // sit in the queue until the operator started a turn of their own, which + // is not what the word means. + // + // The decision itself is `commit::asked`, in the library, because nothing + // under `tests/` links this file and a refusal written here could be + // neither asserted nor sabotaged. This arm holds the wiring: build the + // policy actually in force, ask, and either say why not or submit. + let mut command = app.key(key); + if let Command::Slash(text) = &command { + if let Action::Commit(allow) = commands::parse(text, app.keys(), &app.theme) { + // **Asked first, allowed second, and the order is the fix for a + // defect this release found in its own wiring.** Applying the + // allowance before asking made the question moot: the rule is + // matched ahead of the tier default, so pushing it turned every + // posture — `read only` included — into one that permits `git`, + // and `asked` then answered `Ready` and bought the turn. Under + // `read only` the `.git` write gate refused the commit anyway, so + // the turn was spent on work that was never going to land, which + // is the single thing this check exists to prevent. Worse, the + // rule outlives the keystroke: `remembered` is threaded into every + // later turn, so one `/commit allow` left the agent able to run + // git for the rest of a session whose status line still read + // `policy:read-only`. + // + // So the answer decides. `Offer` is the one case the allowance is + // both effective and honest, and it is the only case it is applied + // in. + let effective = approval::session_policy(&policy, app.posture(), app.remembered()); + command = match io_cli::commit::asked(&effective) { + io_cli::commit::Asked::Offer(_) if allow => { + app.allow_git(); + app.say( + Tone::Muted, + io_cli::commit::authored_as(&opening.commit_identity), + ); + Command::Submit(io_cli::commit::prompt()) + } + io_cli::commit::Asked::Offer(sentence) => { + app.say(Tone::Refused, sentence); + Command::None + } + io_cli::commit::Asked::Refuse(sentence) => { + app.say(Tone::Refused, sentence); + Command::None + } + io_cli::commit::Asked::Ready => { + // **Said before the turn is bought, not after it lands.** + // Authorship is the one thing about a commit that cannot + // be corrected later without rewriting history, so the + // operator sees who it will be attributed to while the + // decision is still theirs. Read off `opening`, built at + // startup from the same builder every turn uses, rather + // than from a sixth `contract::session` call — + // `tests/contract.rs` counts those and fails at five. + app.say( + Tone::Muted, + io_cli::commit::authored_as(&opening.commit_identity), + ); + Command::Submit(io_cli::commit::prompt()) + } + }; + } + } + + match command { Command::None => {} Command::Exit => return Ok(()), // Nothing is running at an idle prompt, so there is nothing to stop. @@ -2551,6 +2628,16 @@ async fn loop_over Result>( Action::Print(lines) => { screen.commit(&lines).map_err(|error| error.to_string())?; } + // Rewritten into a `Command::Submit` above, so this is not the + // path `/commit` takes. It stays because the match is exhaustive + // and a wildcard here would swallow the next command somebody + // adds without an arm — which is the defect `tests/commands.rs` + // exists to catch, and it would be a shame to reintroduce it in + // the driver on the release that repaired the gate. + Action::Commit(_) => debug_assert!( + false, + "/commit is rewritten into a submit before the match and cannot arrive here" + ), Action::Quit => return Ok(()), Action::Setup => { app.say( @@ -4027,6 +4114,13 @@ async fn turn( // on the contract only where the contract itself reaches the run. let (gate, mut plans) = io_cli::plan::channel(); app.started(); + // **Re-read at the turn boundary, so a branch changed outside io-cli is stale + // for at most one turn.** A `git_branch` call updates the field live during a + // turn, but nothing tells this process about a `git switch` the operator ran + // in another terminal — and a status line naming a branch the tree left is + // worse than one naming none. One file read per turn is the whole cost, and + // it is the same read that answers a session's very first turn. + app.set_branch(io_cli::repo::branch(session.root())); paint(screen, app)?; // **The fifth seam, and the only one that speaks to a turn already in @@ -4217,6 +4311,7 @@ async fn turn( app.status.elapsed = at; app.event(&event, at); commit_edits(app, store, &event, screen.width()); + commit_commits(app, store, &event); // The live half of `ctx N%`. Anchored on a step rather than on // every event, for the reason `commit_edits` above it is: the // assembly is written once a step and reading it per event would @@ -4559,6 +4654,7 @@ async fn turn( // and the last step of a turn is exactly the one that loses that race, // so the edit a reader most wants to see is the one that vanishes. commit_edits(app, store, &event, width); + commit_commits(app, store, &event); // And on the drain, for the same race the two lines above it are here // for: the last step of a turn is exactly the one whose event the select // loop loses to the turn's own return. @@ -4913,6 +5009,93 @@ fn commit_edits(app: &mut App, store: &Store, event: &io_harness::RunEvent, widt } } +/// Put a commit the agent made into the scrollback, with the message it wrote. +/// +/// **Anchored on `Step` for the reason [`commit_edits`] above it is**: io-harness +/// documents `Step` as emitted once the step has been committed to the store, so +/// the assistant turn this reads is on disk by the time it is asked for. Reading +/// at the tool call instead would be a race that passes on a quiet laptop and +/// fails on a loaded machine, which `src/status.rs` already records for the +/// context read. +/// +/// **The step's `changed` flag is NOT the signal, and believing it was is the +/// worst defect this release nearly shipped.** `changed` is a step-level OR over +/// every dispatch in the step (`run/step.rs` folds `step_changed |= changed`), so +/// an ordinary `write_file` in the same step sets it — and a `git_commit` beside +/// that write which git rejected, or which the policy refused before it ran, +/// arrives under `changed: true` all the same. `crate::commit::made_in` reads the +/// model's *requested* calls and cannot tell a commit that landed from one that +/// did not, which its own rustdoc says; so the caller must, and the caller is +/// this. Under the wizard's own posture the two together printed the refusal +/// paragraph and a `committed on main` block in the same step. +/// +/// The honest signal is the step's own decision. io-harness writes +/// `"{tool} ok"` or `"{tool} exit {code}"` per dispatched call and joins them with +/// `"; "` (`run/dispatch.rs`), so `git_commit ok` appears once per commit that +/// actually succeeded. Counting them bounds what may be drawn: a step whose +/// commit was refused counts zero and draws nothing. +/// +/// **Depth zero only.** `App`'s branch is the session's, read from the root's +/// `.git/HEAD`; a contained child with `worktree = true` is on a different branch +/// in a different checkout, and io-harness exposes no reader for where — so a +/// child's commit drawn here would be attributed to a branch it was never on. +/// Better silent than wrong, and `src/fleet.rs` already draws that line. +/// +/// The diff is deliberately not redrawn: [`commit_edits`] has already put this +/// step's hunks on screen immediately above. +fn commit_commits(app: &mut App, store: &Store, event: &io_harness::RunEvent) { + let io_harness::EventKind::Step { .. } = &event.kind else { + return; + }; + if event.depth > 0 { + return; + } + let steps = match store.steps(event.run_id) { + Ok(steps) => steps, + // Said once and quietly. A commit that happened is still on the branch + // whether or not this crate could read it back, so the session is not + // worth interrupting over a store read. + Err(error) => { + app.say( + Tone::Muted, + format!("the commit for this step could not be read: {error}"), + ); + return; + } + }; + let landed = steps + .iter() + .find(|step| step.step == event.step) + .map(|step| step.decision.matches(COMMIT_LANDED).count()) + .unwrap_or_default(); + if landed == 0 { + return; + } + let Ok(turns) = store.step_turns(event.run_id) else { + return; + }; + let branch = app.branch().map(str::to_string); + // Bounded by what the step reported landing. Pairing a particular call to a + // particular success is not possible — the decisions are joined into one + // string — so a step that made two commits and landed one draws one. That is + // an under-claim in a case no model has produced here, and an under-claim is + // the side of this to be wrong on. + for made in io_cli::commit::made_in(&turns) + .into_iter() + .filter(|made| made.step == event.step) + .take(landed) + { + app.committed(io_cli::commit::block(&made, branch.as_deref())); + } +} + +/// What a step's decision says when a commit actually landed. +/// +/// io-harness builds it as `format!("{name} {}")` per dispatched call, so +/// this is that string for `git_commit` and nothing else matches it — `exit 1` +/// and `refused` both fail the test, which is the whole point. +const COMMIT_LANDED: &str = "git_commit ok"; + /// Put the whole conversation back into the terminal's own scrollback. /// /// Upward and never into a pane. The viewport is eight rows and cannot grow, and @@ -6039,6 +6222,13 @@ async fn resume_pending( // twice over. Saying a run has started fixes both — the prompt is queued and // the key becomes an interrupt — and it is what a live turn says here too. app.started(); + // **The same re-read the ordinary turn does, and leaving it out here was a + // defect rather than an omission of no consequence.** A resumed run drives + // work that commits, and without this the branch is whatever the last live + // turn left — or nothing at all in a fresh process that resumed immediately. + // A commit block naming the branch the tree was on an hour ago is exactly + // what `App::set_branch` calls worse than naming none. + app.set_branch(io_cli::repo::branch(session.root())); paint(screen, app)?; let driving = async { match decided { @@ -6132,6 +6322,7 @@ async fn resume_pending( app.status.elapsed = at; app.event(&event, at); commit_edits(app, store, &event, screen.width()); + commit_commits(app, store, &event); note_context(app, store, &event, seen, &continuing); paint(screen, app)?; } diff --git a/src/repo.rs b/src/repo.rs new file mode 100644 index 0000000..612f9b4 --- /dev/null +++ b/src/repo.rs @@ -0,0 +1,129 @@ +//! The branch the working tree is on. +//! +//! **This module is not called `git`, and the name is the point.** io-harness owns +//! the git engine — `Git`, `GitCmd` and `GitOutcome` are all `pub(crate)` there, so +//! nothing in this crate can build a governed argv or run a git command — and a +//! crate-root module named for it would send a reader looking here for something +//! that is not here. The same reasoning named [`crate::picture`] rather than +//! `image` in 0.9.0, and kept [`crate::verify`] for the wizard's credential check +//! in 0.24.0. +//! +//! **Nothing here spawns anything.** `tests/dependencies.rs` permits a process +//! spawn in `src/shell.rs` alone, identified by path, and that assertion is +//! unamended by this release. That gate matches raw text, comments included, so +//! the forbidden name is not spelled out here either — not even to say this module +//! does not use it, which is a sentence that reddens the gate it is describing. +//! A branch name does not need a subprocess anyway: git writes it into `.git/HEAD` +//! as text, in a format that has not changed in the lifetime of the tool, and the +//! standard library reads a file. +//! +//! The four shapes this has to survive, only one of which a developer's own +//! checkout usually has: +//! +//! - A **symbolic ref** — `ref: refs/heads/` — which is the ordinary case. +//! - A **detached head**, where `HEAD` holds a raw object id. Reported as a short +//! id, because reporting nothing is how a surface goes blank at the moment an +//! operator most needs to know where they are. +//! - A **linked worktree**, where `.git` is a *file* holding `gitdir: ` +//! rather than a directory. This is exactly what a `worktree = true` child gets, +//! so the release that adds that switch cannot be the release that fails to read +//! it. +//! - A directory that is **not a repository at all**, which answers `None` — io-cli +//! runs in plenty of them and must not become worse there. + +/// How long a detached head's object id is rendered. +/// +/// Seven is what `git log --oneline` and every code host use, so the id here is +/// the id an operator will recognise elsewhere. +pub const SHORT_ID: usize = 7; + +/// The branch the tree at `root` is on, or `None` when there is no answer. +/// +/// `None` means "this is not a repository, or its `HEAD` cannot be read" — never +/// "the branch is empty". A caller draws nothing at all for `None`, which is what +/// keeps io-cli silent in a directory that was never a checkout. +pub fn branch(root: &std::path::Path) -> Option { + let head = git_dir(root)?.join("HEAD"); + let text = std::fs::read_to_string(head).ok()?; + read_head(&text) +} + +/// The real git directory for `root`, following the one indirection git uses for a +/// linked worktree. +/// +/// In an ordinary checkout `.git` is a directory and this is it. In a linked +/// worktree — which is what `AgentDef::worktree` gives a child agent — `.git` is a +/// *file* whose contents are `gitdir: `, and the path may be relative to the +/// worktree. Both are resolved here so no caller has to know the difference. +pub fn git_dir(root: &std::path::Path) -> Option { + // **Walked upward, because a workspace root is not always a repository root.** + // `io -C src` is an ordinary thing to do, and git itself walks up from the + // working directory — so the agent's own seven tools find the repository + // while a check that looked only at `root/.git` found nothing and reported, + // permanently, that a perfectly readable checkout had no branch. The first + // ancestor holding a `.git` wins, which is git's own rule. + root.ancestors().find_map(git_dir_at) +} + +/// The git directory named by exactly this directory's `.git`, if it has one. +fn git_dir_at(root: &std::path::Path) -> Option { + let dot = root.join(".git"); + let meta = std::fs::metadata(&dot).ok()?; + if meta.is_dir() { + return Some(dot); + } + let text = std::fs::read_to_string(&dot).ok()?; + // First line only, for the same reason `read_head` takes one: the file holds a + // single `gitdir:` line, and anything after it is not part of the path. Taking + // the whole text would fold a stray second line into the directory name and + // turn a readable worktree into a `None`. + let named = text.lines().next()?.strip_prefix("gitdir:")?.trim(); + if named.is_empty() { + return None; + } + let path = std::path::Path::new(named); + Some(if path.is_absolute() { + path.to_path_buf() + } else { + root.join(path) + }) +} + +/// What a `HEAD` file says, as a branch name or a short object id. +/// +/// Split from [`branch`] because this is the half that has to be right: the file +/// read above it either works or answers `None`, while every wrong answer this +/// module could give is a misparse in here. `tests/repo.rs` still drives it +/// through a real `.git` on disk rather than calling it directly — the format is +/// the thing under test, and a caller that hands it a string it invented is +/// asserting its own invention. +pub fn read_head(text: &str) -> Option { + let line = text.lines().next()?.trim(); + if line.is_empty() { + return None; + } + match line.strip_prefix("ref:") { + // A symbolic ref. Only the `refs/heads/` prefix comes off — everything + // after it is the name, so `refs/heads/feature/x` reads as `feature/x` + // and not as `x`. Taking the last path component instead would rename + // every nested branch in the product to its own tail, which is precisely + // the case a developer's own checkout usually does not have. + // + // A ref that is not under `refs/heads/` — a tag, a remote — is not a + // branch, and answering `None` for it says so rather than inventing a + // name for a head that has none. + Some(reference) => { + let name = reference.trim().strip_prefix("refs/heads/")?; + (!name.is_empty()).then(|| name.to_string()) + } + // A detached head. Reported short, and only when the WHOLE line is an + // object id — anything else is a `HEAD` this function does not + // understand, and truncating it to seven characters anyway would put + // arbitrary file contents on the status line dressed as a commit. Hence + // the check before the slice: a line too short to be an id, or carrying a + // single non-hex character, is no answer rather than a plausible-looking + // wrong one. + None => (line.len() >= SHORT_ID && line.chars().all(|c| c.is_ascii_hexdigit())) + .then(|| line.chars().take(SHORT_ID).collect()), + } +} diff --git a/src/status.rs b/src/status.rs index b1ce377..d52e362 100644 --- a/src/status.rs +++ b/src/status.rs @@ -313,6 +313,33 @@ pub struct Status { /// showing the mode alone is reading an intention — `workspace-write` reaching /// a portable floor means resource caps and nothing else. pub containment: Option, + /// The branch the working tree is on, or `None` where there is no answer. + /// + /// **A fact about the checkout, so neither [`Status::forget_run`] nor + /// [`Status::start_run`] clears it** — which puts it beside [`Status::policy`] + /// and [`Status::budgets`] rather than beside [`Status::containment`], the + /// field it otherwise most resembles. The containment word describes how *one + /// run's* commands were held and dies with that run; a branch describes the + /// directory the operator is standing in, and that directory does not change + /// because a turn ended, because `/clear` started a new conversation, or + /// because `/resume` put another one on screen. A line that blanked it there + /// would be erasing a true fact about the operator's own checkout at the exact + /// moment they opened a fresh conversation to work in it. + /// + /// Set two ways, and they answer different halves of one question. + /// [`Status::note_branch`] takes it from a `git_branch` call the moment the + /// agent makes one, so a branch io-cli's own agent created is on screen before + /// the turn it created it in has finished. The driver re-reads + /// [`crate::repo::branch`] at the turn boundary, which is what keeps a branch + /// changed by anything *else* — a shell in another pane, a colleague's script + /// — from standing here forever. The event arrives before the tool's result is + /// known and a name that already exists is refused, so the second source is + /// also the one that corrects the first. + /// + /// `None` draws **nothing at all** — not `none`, not an empty label. io-cli is + /// run in plenty of directories that were never a checkout, and the rule this + /// line holds everywhere is that an absent fact is absent rather than zero. + pub branch: Option, /// Whether later turns propose a plan before they work. /// /// **Not run-scoped, and the only field on this line that is a standing @@ -540,6 +567,7 @@ impl Status { cost: None, context: None, containment: None, + branch: None, spend: None, plan: None, planning: false, @@ -585,11 +613,14 @@ impl Status { /// read, and absent is the honest answer until the agent writes a list in the /// run that is now on screen. /// - /// The model, the posture, plain mode, the session's age and the depth of the - /// prompt queue are not run facts and are left alone — the session is the - /// same session either way, and the queue in particular is not even reachable - /// from here: see [`Status::queued_prompts`] for why blanking the count - /// would leave the line contradicting prompts that are still going to run. + /// The model, the posture, plain mode, the session's age, the branch the tree + /// is on and the depth of the prompt queue are not run facts and are left + /// alone — the session is the same session either way, and the queue in + /// particular is not even reachable from here: see [`Status::queued_prompts`] + /// for why blanking the count would leave the line contradicting prompts that + /// are still going to run. [`Status::branch`] is the sharpest of them: + /// changing which conversation is on screen does not check out another + /// branch, so clearing it here would blank a fact that is still true. pub fn forget_run(&mut self) { self.tokens = None; // **The money goes with the tokens it was derived from.** `start_run` @@ -671,6 +702,78 @@ impl Status { self.gate_attempt = None; } + /// Take the branch from a `git_branch` call, if that is what this event is. + /// + /// **The whole event rather than its parts, and the filter is here rather than + /// at the call site**, which is the shape [`Status::note_context_from`] already + /// has: the driver hands over every event it routes and cannot pick the wrong + /// one, and the name being matched lives next to the field it sets. The name is + /// io-harness's own constant and is never spelled here as a literal, so a + /// rename on that side is a compile error rather than a field that silently + /// stops updating. + /// + /// `target` is what io-harness made the subject of the call, which for this + /// tool is its `name` argument — the branch being created and moved onto. An + /// empty one sets nothing: the field's absence means "no answer", and an empty + /// string dressed as a branch name is the one thing it must never hold. + /// + /// **Optimistic, on purpose, and corrected at the turn boundary.** io-harness + /// documents `ToolCall` as emitted before the result is known, and this tool + /// refuses a name that already exists — so this is the branch the agent asked + /// for and not yet the branch git granted. The driver's re-read of + /// [`crate::repo::branch`] when the turn ends is what settles it, and until + /// then being one refused call ahead is worth being right for the whole of the + /// turn in the ordinary case. + pub fn note_branch(&mut self, event: &io_harness::RunEvent) { + // **Depth zero only, and 0.25.0 is the release that makes this matter.** + // A `worktree = true` child works in its own checkout on its own branch; + // its `git_branch` call is a fact about *that* tree, and this field is a + // fact about the session's. Taking a child's branch here would put a name + // on the status line, the `/status` page and every later commit block + // that no reader could act on, because the checkout it belongs to is one + // io-harness exposes no way to name. + if event.depth > 0 { + return; + } + let io_harness::EventKind::ToolCall { name, target } = &event.kind else { + return; + }; + // **`target == name` is a call that named nothing, not a branch called + // `git_branch`.** io-harness picks a call's subject from the first + // conventional argument it carries and falls back to the tool's own name + // when it carries none, so an announcement with no `name` argument + // arrives here reading `git_branch` — and recording that would put a + // branch nobody has on the status line, the `/status` page and every + // commit block, from a call that failed. Trimmed as well as compared, + // because whitespace is not a branch either. + if name != io_harness::tools::GIT_BRANCH_TOOL || target == name || target.trim().is_empty() + { + return; + } + self.branch = Some(target.clone()); + } + + /// The branch, drawn, or nothing at all. + /// + /// **One method, reached by both renderers**, which is the shape + /// [`Status::budgets_left`], [`Status::queued_left`], [`Status::cost_field`] + /// and [`Status::gate_field`] already have and for the reason written where + /// they are: this file has shipped a field into one renderer twice — 0.8.0's + /// spend and 0.12.0's planning phase — and both times it was green in a unit + /// test and nowhere on screen, because the binary draws the footer on every + /// terminal seven rows or taller. + /// + /// `git:main`, spelled the way [`Status::policy`] and [`Status::provider`] are + /// spelled on [`Status::fields`]. A bare `main` would be a word with no owner + /// sitting beside a containment word and a posture, and a reader who does not + /// already know the branch cannot tell which of the three it is. Four cells + /// buys that, and four cells is what the shortest honest prefix costs — the + /// same reason a detached head is seven characters of object id in + /// [`crate::repo`] rather than forty. + pub fn branch_field(&self) -> Option { + self.branch.as_ref().map(|branch| format!("git:{branch}")) + } + /// Set [`Status::cost`] from what this run has actually called, priced by /// `table`. /// @@ -1185,6 +1288,28 @@ impl Status { if let Some(containment) = &self.containment { fields.push(Field::new(containment.clone(), Tone::Muted)); } + // **Immediately right of the containment word, which is its nearest kin on + // this line, and left of the plan claim.** Both are standing facts about + // the circumstances the agent is working in rather than counters of what a + // turn spent — one says how its commands are held, the other says which + // checkout its writes land in — so they read together and narrow together. + // + // Left of `plan` rather than right of it, and that is the drop order rather + // than a grouping: `fits` gives up fields from the right, and what the + // agent *claims* about its own list is the field this line already names as + // the first to go. Which branch the operator is standing on outlives the + // claim, the turn and the conversation, so it survives one column further. + // The cost of putting it here is that the plan claim and the `unknown` + // diagnostic beyond it are pushed right by the field's own width plus a + // separator, which is what they give up on a crowded line. + // + // `Muted` like the containment word beside it: it is a fact about where the + // work is happening, not a bound on what may happen, which is the tone + // `policy` and `planning` wear. + fields.extend( + self.branch_field() + .map(|text| Field::new(text, Tone::Muted)), + ); // Rightmost, and so the first field to go when the terminal narrows. It is // the only field on this line that is not an observation — everything to // its left is something the harness reported happening, and this is what @@ -1276,7 +1401,17 @@ impl Status { // terminal does not have. let mut left = Vec::new(); if !self.working { - left.push(Span::styled("• ready", muted)); + // **`theme.glyphs.bullet` and not a literal `•`.** This line carried + // a hardcoded U+2022 from the release that wrote it, so the footer + // drew a character the terminal had just been told it could not + // render. It went unnoticed because every glyph sweep in + // `tests/glyphs.rs` ran over `Status::line`, which only a terminal + // under seven rows ever draws, while `Status::render` takes this + // footer on every real one — a gate aimed at the surface nobody sees. + left.push(Span::styled( + format!("{} ready", theme.glyphs.bullet), + muted, + )); left.push(Span::styled(separator, muted)); } left.push(Span::styled( @@ -1409,6 +1544,33 @@ impl Status { } allowed.push(Span::styled("planning", muted)); } + // **`counts` and NOT the right-hand group, and this release measured why.** + // The branch first went in beside the posture, on the argument that it + // describes the circumstances the agent works in rather than counting + // anything — which reads well and is wrong, because of how the two groups + // yield. `row` fits its right-hand group all or nothing, so the group + // survives only while it fits what narrowing leaves; at eighty columns, + // with `planning` on, the counters are already gone and there is nothing + // left to pay with. The measurement, taken rather than reasoned about: + // + // [None] "/ for commands read-only · workspace-write/… · planning" + // [Some("main")] "/ for commands" + // + // **Every real branch does that** — the break-even is a name one character + // long — so the operator finishes a turn under `/plan on` with nothing on + // screen saying the next write will stop and ask. That is verbatim the + // failure `f4_a_full_counts_row_drops_a_counter_and_not_the_planning_phase` + // exists to prevent, and 0.22.0 shipped its ancestor. + // + // So the branch is a counter's neighbour rather than a posture's: it is + // the one fact on this row an operator can afford to lose, because it is + // still readable one keystroke away on the `/status` page while a posture + // that has silently vanished is not. Pushed last, so `counts.pop()` takes + // it before it takes any number — a narrow row keeps what it measured and + // gives up where it measured it. + if let Some(text) = self.branch_field() { + counts.push(text); + } // **When the two groups cannot both fit, the counters yield — not the // group.** `row` fits its right-hand group all or nothing, so a counts // row one character too wide took the posture, the containment word AND @@ -1705,6 +1867,24 @@ pub fn committed( // is this" and it is io-harness's — the same rule `App::set_root` follows. facts.push(("workspace".into(), session.root().display().to_string())); + // Directly under the workspace, because it is the second half of the same + // fact: the path says which directory, and this says which of its branches is + // checked out. Read off `Status` rather than off disk, so opening this page + // reads nothing and reports exactly what the footer is already showing — the + // rule the budgets on this page are held to, arrived at from the other side. + // + // An absence gets a row of its own here, unlike on the status line where it + // draws nothing at all. This surface is one fact per row with no width to + // compete for, and every other unknown on it — the provider, the sandbox, the + // containment — says why it is unknown rather than going missing. + facts.push(( + "branch".into(), + match &status.branch { + Some(branch) => branch.clone(), + None => format!("not known {dash} this workspace has no readable git head"), + }, + )); + // **Where io-cli keeps what it keeps, and who decided.** The directory in // force rather than [`crate::home::path`]: under `$IO_CONFIG` the file is // somewhere io-cli did not choose, and reporting the home this crate *would* diff --git a/tests/commands.rs b/tests/commands.rs index 3a5c6af..278ef2d 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -53,6 +53,14 @@ fn the_commands_are_the_commands() { "/compact", "/copy", "/copy diff", + // 0.25.0 — the third thing an operator does with the work a turn has + // just finished, and it sits beside the two that copy it: one takes + // the answer, one takes the patch, this one makes the patch permanent. + // It goes under **this turn** and not **inspect** for that reason, and + // it takes that group to ten, which is the bound — so the next command + // that would fill a group re-files one that is in the wrong group + // rather than widening it. + "/commit", "/config", // 0.18.0 — the other two surfaces that write a file the operator // keeps, and they ask the same scope question `/config` does: three @@ -118,6 +126,21 @@ fn the_commands_are_the_commands() { /// stops covering the next row the moment somebody forgets to extend it, which is /// exactly the failure this test exists for. Exactly ONE listed command may /// resolve to the help listing, and it is `/help`. +/// +/// **0.25.0 rewrote how "the help listing" is recognised, because the old +/// spelling had stopped recognising it at all.** The check was one comparison — +/// `parse(name) == parse("help")` — and that was true of the `unknown` arm only +/// while the arm literally returned `help(…)`. It has not for some time: it now +/// builds a *warning notice* followed by `commands(theme)` alone, with no key +/// table, so an armless command's `Action::Print` can never equal `/help`'s. The +/// gate was passing on the one command it was allowed to pass on and blind to +/// every command it existed to catch. `/commit`'s sabotage — the row added and +/// the `parse` arm withheld — went green under it, which is how it was found. +/// +/// So the property is asserted against **both** shapes an inert command can take: +/// the help listing itself, and the sentence the `unknown` arm writes. The second +/// is matched on `there is no /`, which is the arm's own text and the one +/// thing a fallen-through command is guaranteed to carry. #[test] fn no_listed_command_falls_through_to_the_help_listing() { let listing = commands::parse("help", &defaults(), &DARK); @@ -126,16 +149,27 @@ fn no_listed_command_falls_through_to_the_help_listing() { .map(|(name, _)| *name) .filter(|name| { let typed = name.strip_prefix('/').unwrap_or(name); - commands::parse(typed, &defaults(), &DARK) == listing + let got = commands::parse(typed, &defaults(), &DARK); + if got == listing { + return true; + } + // The `unknown` arm names the word it did not recognise, and `parse` + // decides on the FIRST word — which is what `/copy diff` is spelled + // out of, and what the sentence would carry if it ever fell through. + let word = typed.split_whitespace().next().unwrap_or(typed); + match got { + Action::Print(lines) => text(&lines).contains(&format!("there is no /{word}")), + _ => false, + } }) .collect(); assert_eq!( inert, vec!["/help"], - "these commands are listed but resolve to the help listing, which is what \ - an unknown command resolves to — they are advertised and inert. Give each \ - one an arm in `commands::parse` and an arm in the driver, or take the row \ - out of `COMMANDS`." + "these commands are listed but resolve to what an UNKNOWN command resolves \ + to — the help listing, or the notice that says the command does not exist. \ + They are advertised and inert. Give each one an arm in `commands::parse` \ + and an arm in the driver, or take the row out of `COMMANDS`." ); } @@ -269,14 +303,14 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio ); // No command was added for any of this — `/resume` was extended — so the // inventory is the size the other gates in this file assert. The number is - // 30 plus 0.24.0's one addition, and the two halves are named separately on - // purpose: a total that merely went up by one would be satisfied by 0.23.0 - // growing a command and 0.24.0 losing one. + // 31 plus 0.25.0's one addition, and the two halves are named separately on + // purpose: a total that merely went up by one would be satisfied by this + // release growing a command and losing another. assert_eq!( COMMANDS.len(), - 31, - "0.23.0 adds no command and 0.24.0 adds exactly one, `/gates`; a \ - thirty-second here means one arrived unrecorded", + 32, + "0.25.0 adds exactly one command, `/commit`, on top of the thirty-one \ + 0.24.0 shipped; a thirty-third here means one arrived unrecorded", ); } diff --git a/tests/commit.rs b/tests/commit.rs new file mode 100644 index 0000000..01848e1 --- /dev/null +++ b/tests/commit.rs @@ -0,0 +1,453 @@ +//! F5 — a step that committed says so, once, from the typed call. +//! F7 — the identity a commit will carry is read, never chosen. +//! +//! Both criteria are asserted over values built here rather than over a `Store`. +//! `AssistantTurn` and `ToolCall` are public and constructible, so a run's rows +//! can be spelled directly — which keeps the assertion about the shape io-cli +//! reads instead of about SQLite, and keeps the one property F5 actually turns on +//! visible in the test source: **the message is a `serde_json::Value` field, and +//! never a substring of a display string.** +//! +//! That is why every message below that carries meaning carries a colon. The +//! naive reading of a commit — split `StepRecord::tool_call`, whose +//! `name:json` joining is an internal display convention — survives a corpus of +//! messages that happen to have no `:` in them, and dies on `fix: …`, which is +//! the first line of most commits ever written. A test suite whose messages are +//! all colonless would pass under that reading and prove nothing. + +use io_cli::commit::{self, Made}; +use io_harness::{AssistantTurn, Defaults, Effect, Identity, Policy, ToolCall}; + +/// A `git_commit` call carrying `message`. +fn commits(message: &str) -> ToolCall { + ToolCall { + name: commit::TOOL.to_string(), + arguments: serde_json::json!({ "message": message }), + } +} + +/// A turn at `step` that made exactly the calls given, and wrote nothing. +fn turn(step: u32, calls: Vec) -> AssistantTurn { + AssistantTurn::new(step, None::, calls) +} + +/// The message every colon-sensitive assertion uses. +/// +/// A conventional-commit subject, because that is the shape the sabotage breaks +/// on and the shape this product's own history is written in. +const COLON: &str = "fix: handle the colon case"; + +#[test] +fn f5_the_message_comes_back_whole_including_its_colon() { + let made = commit::made_in(&[turn(1, vec![commits(COLON)])]); + + assert_eq!( + made, + vec![Made { + step: 1, + message: COLON.to_string(), + }], + "the message is the `message` argument of the typed call, verbatim. \ + Anything less than the whole string means it was reconstructed from a \ + display form rather than read.", + ); +} + +#[test] +fn f5_a_colon_message_survives_the_block_and_the_subject() { + // The same string again, through the two surfaces that render it. A reading + // that truncated at the colon would leave `made_in` correct and still put + // `handle the colon case` on screen, so the property is asserted where the + // operator actually sees it and not only at the boundary. + let made = Made { + step: 4, + message: COLON.to_string(), + }; + + assert_eq!(made.subject(), COLON); + assert!( + commit::block(&made, Some("feat/0.25.0")) + .iter() + .any(|line| line.contains(COLON)), + "the committed block carries the message as written", + ); +} + +#[test] +fn f5_only_git_commit_calls_in_a_turn_are_read() { + // A real committing step reads, writes and then commits, all in one turn. + // The name is checked per call, so the neighbours contribute nothing — and a + // `message` argument on a *different* tool is not a commit message, which is + // the mistake a per-turn check would make. + let made = commit::made_in(&[turn( + 7, + vec![ + ToolCall { + name: "read_file".to_string(), + arguments: serde_json::json!({ "path": "src/commit.rs" }), + }, + commits(COLON), + ToolCall { + name: "write_file".to_string(), + arguments: serde_json::json!({ "message": "not a commit: a note" }), + }, + ], + )]); + + assert_eq!( + made, + vec![Made { + step: 7, + message: COLON.to_string(), + }], + ); +} + +#[test] +fn f5_commits_come_back_oldest_first() { + // Two turns, and two calls inside the second, because both orderings have to + // hold: `step_turns` returns turns oldest first and a turn's `calls` are in + // the order the model made them. Distinct messages rather than distinct + // steps, so a sort on `step` alone cannot pass this. + let made = commit::made_in(&[ + turn(2, vec![commits("feat: the first thing")]), + turn( + 5, + vec![commits("fix: the second thing"), commits("docs: the third")], + ), + ]); + + assert_eq!( + made.iter().map(Made::subject).collect::>(), + vec![ + "feat: the first thing", + "fix: the second thing", + "docs: the third", + ], + ); + assert_eq!( + made.iter().map(|m| m.step).collect::>(), + vec![2, 5, 5] + ); +} + +#[test] +fn f5_a_call_with_no_usable_message_commits_nothing() { + // Each of these is a call the model made wrongly and the harness refused. + // None of them is somebody committing an empty message, and rendering a + // block for one would be a scrollback claiming a commit that does not exist. + let unusable = [ + serde_json::json!({}), + serde_json::json!({ "msg": "fix: wrong argument name" }), + serde_json::json!({ "message": 42 }), + serde_json::json!({ "message": null }), + serde_json::json!({ "message": ["fix: a list is not a message"] }), + serde_json::json!({ "message": "" }), + serde_json::json!({ "message": " \n\t " }), + // Not an object at all — a model that produced a bare string where the + // schema asked for arguments. `get` on it answers `None`, and the point + // of asserting it is that it must not panic. + serde_json::json!("fix: arguments that are not an object"), + ]; + + for arguments in unusable { + let made = commit::made_in(&[turn( + 1, + vec![ToolCall { + name: commit::TOOL.to_string(), + arguments: arguments.clone(), + }], + )]); + assert!( + made.is_empty(), + "arguments {arguments} yielded a commit; a call the harness refused \ + must commit no block", + ); + } +} + +#[test] +fn f5_no_turns_and_no_commits_are_the_same_silence() { + assert!(commit::made_in(&[]).is_empty()); + assert!(commit::made_in(&[turn(1, Vec::new())]).is_empty()); +} + +#[test] +fn f5_the_block_carries_the_message_and_the_branch() { + let made = Made { + step: 3, + message: COLON.to_string(), + }; + + assert_eq!( + commit::block(&made, Some("feat/0.25.0")), + vec!["committed on feat/0.25.0".to_string(), format!(" {COLON}"),], + "one block, carrying both things the criterion names — and the colon is \ + the point of the fixture, not decoration", + ); +} + +#[test] +fn f5_an_unknown_branch_or_report_leaves_out_a_line_rather_than_inventing_one() { + let made = Made { + step: 3, + message: COLON.to_string(), + }; + + assert_eq!( + commit::block(&made, None), + vec!["committed".to_string(), format!(" {COLON}")], + ); + // Present but blank is an unknown wearing a value's clothes, and is treated + // as the unknown it is rather than drawn as `committed on `. + assert_eq!(commit::block(&made, Some(" ")), commit::block(&made, None),); +} + +#[test] +fn f5_a_body_is_kept_whole_and_the_subject_is_only_its_first_line() { + let made = Made { + step: 9, + message: format!("{COLON}\n\nThe body: it explains why.\nAnd a second line."), + }; + + assert_eq!( + made.subject(), + COLON, + "a status row shows the subject alone" + ); + assert_eq!( + commit::block(&made, Some("develop")), + vec![ + "committed on develop".to_string(), + format!(" {COLON}"), + // The blank separator stays blank. Indenting nothing would put two + // trailing spaces into everything that copies this block back out. + String::new(), + " The body: it explains why.".to_string(), + " And a second line.".to_string(), + ], + ); +} + +#[test] +fn f5_a_subject_below_a_leading_blank_line_is_still_the_subject() { + // A model that opens with a newline has still written a subject. Taking + // `lines().next()` here answers `""` — a status row gone blank for a commit + // that has one. + let made = Made { + step: 1, + message: format!("\n{COLON}\n\nbody"), + }; + assert_eq!(made.subject(), COLON); +} + +#[test] +fn f5_the_subject_of_a_message_with_nothing_in_it_is_empty() { + // `made_in` never builds one of these — every such call is skipped above — + // so this pins the direct-construction path: empty, not a panic, and not the + // body of some later line promoted into a subject it never was. + for message in ["", "\n", " ", "\n\n\n"] { + assert_eq!( + Made { + step: 1, + message: message.to_string(), + } + .subject(), + "", + "message {message:?}", + ); + } +} + +#[test] +fn f5_the_prompt_asks_the_agent_to_write_the_message() { + let prompt = commit::prompt(); + assert!( + prompt.contains("Commit the work from this turn"), + "the command is a prompt to the agent, not a git invocation io-cli makes", + ); + assert!( + !prompt.is_empty() && !prompt.contains(" "), + "a prompt continued across source lines keeps single spaces: {prompt:?}", + ); +} + +#[test] +fn f7_the_identity_shown_is_the_one_the_contract_carries() { + let configured = Identity { + name: "release bot".to_string(), + email: "bot@example.com".to_string(), + }; + assert_eq!( + commit::authored_as(&configured), + "authored as release bot ", + "`[run.commit_identity]` is read and printed, both halves, unchanged", + ); +} + +#[test] +fn f7_an_unconfigured_repository_is_told_the_harness_default() { + // `TaskContract::commit_identity` is an `Identity`, not an `Option`, + // and holds exactly this when no `[run.commit_identity]` was written. So the + // absent-section case is this value, and what io-cli must show for it is the + // value io-harness will hand git — read from `Identity::default()` itself + // rather than copied into a literal here, because a copy is a second place + // for the string to live and the wrong one would still pass. + let default = Identity::default(); + let shown = commit::authored_as(&default); + + assert_eq!( + shown, + format!("authored as {} <{}>", default.name, default.email), + ); + assert!( + shown.contains("io-harness"), + "the default belongs to the harness and says so: {shown:?}", + ); + assert!( + !shown.contains("io-cli"), + "a name this crate chose would be written into the operator's history by \ + a tool that was only asked to describe it, and there is nowhere to \ + correct it afterwards: {shown:?}", + ); +} + +#[test] +fn f7_no_identity_is_substituted_for_one_that_merely_looks_like_a_default() { + // The shape of the F7 sabotage: recognise the default and swap in something + // of io-cli's own. An identity is read and printed, so a name that reads like + // a placeholder is still printed as written. + for name in ["io-harness agent", "", "unknown"] { + let identity = Identity { + name: name.to_string(), + email: "somebody@example.com".to_string(), + }; + assert_eq!( + commit::authored_as(&identity), + format!("authored as {name} "), + ); + } +} + +// F2's first half — the check that happens BEFORE a turn is bought. +// +// `asked` is the whole of the pre-turn decision, and it lives here rather than in +// the driver for the reason every decision in this release does: nothing under +// `tests/` links `src/main.rs`, so a refusal written there could be neither +// asserted nor sabotaged. The driver holds the wiring — build the policy in +// force, ask, and either say why not or submit — and this is where the answer is +// pinned. + +/// A policy whose `exec` default is what a posture would set it to. +fn posture(exec: Effect) -> Policy { + let mut policy = Policy::permissive(); + policy.defaults = Defaults { + read: Effect::Allow, + write: Effect::Allow, + exec, + net: Effect::Deny, + }; + policy +} + +#[test] +fn f2_a_posture_that_allows_git_buys_the_turn() { + assert_eq!( + commit::asked(&posture(Effect::Allow)), + commit::Asked::Ready, + "nothing refuses git, so there is nothing to say and the turn is bought", + ); +} + +#[test] +fn f2_an_asking_default_is_offered_the_allowance() { + let commit::Asked::Offer(sentence) = commit::asked(&posture(Effect::Ask)) else { + panic!("an asking default is the one case the allowance both helps and is honest in"); + }; + assert!( + sentence.contains("refused rather than asked"), + "the sentence has to name what the posture did, because the posture's own \ + name says it will ask and the harness does not: {sentence:?}", + ); + assert!( + sentence.contains("/commit allow"), + "an offer the operator cannot take is not an offer: {sentence:?}", + ); +} + +#[test] +fn f2_a_denying_default_is_refused_and_never_offered_the_allowance() { + // **The defect this arm was rewritten for.** A rule is matched BEFORE a tier + // default, so the allowance would in fact work under `read only` — which is + // exactly why it must not be offered there. The first version of this test + // asserted only the wording of the refusal and would have passed while the + // driver applied the rule anyway. + let refused = commit::asked(&posture(Effect::Deny)); + assert!( + matches!(refused, commit::Asked::Refuse(_)), + "a denying posture must never be offered the allowance: {refused:?}", + ); + let commit::Asked::Refuse(sentence) = refused else { + unreachable!() + }; + assert!( + sentence.contains("posture"), + "it is told what to change instead: {sentence:?}", + ); +} + +#[test] +fn f2_a_rule_that_refuses_git_is_refused_rather_than_offered_advice_it_cannot_take() { + // A deny wins over any later allow, across layers. Offering the allowance + // here would print advice that can never be taken, on every attempt, forever + // — and the operator's own configuration is where the answer actually is, so + // the sentence names the rule. + for effect in [Effect::Ask, Effect::Deny] { + let mut policy = Policy::permissive(); + policy.defaults = Defaults { + read: Effect::Allow, + write: Effect::Allow, + // An ASKING default underneath, so only the rule can be what decides. + exec: Effect::Ask, + net: Effect::Deny, + }; + policy.layers.push(io_harness::Layer { + name: "the operator's own".into(), + rules: vec![io_harness::Rule { + act: io_harness::Act::Exec, + effect, + pattern: "git".into(), + }], + }); + let answer = commit::asked(&policy); + assert!( + matches!(answer, commit::Asked::Refuse(_)), + "a rule decided, so the allowance cannot lift it and must not be \ + offered ({effect:?}): {answer:?}", + ); + let commit::Asked::Refuse(sentence) = answer else { + unreachable!() + }; + assert!( + sentence.contains("git"), + "the refusal names the rule that decided: {sentence:?}", + ); + } +} + +#[test] +fn f2_the_three_answers_are_three_different_sentences() { + let offered = match commit::asked(&posture(Effect::Ask)) { + commit::Asked::Offer(sentence) => sentence, + other => panic!("expected an offer: {other:?}"), + }; + let denied = match commit::asked(&posture(Effect::Deny)) { + commit::Asked::Refuse(sentence) => sentence, + other => panic!("expected a refusal: {other:?}"), + }; + assert_ne!( + offered, denied, + "an operator who asked to be asked and one who asked for nothing to run \ + are in different situations, and only one of them has a one-keystroke fix", + ); +} diff --git a/tests/fleet.rs b/tests/fleet.rs index 841a0cb..f6560ca 100644 --- a/tests/fleet.rs +++ b/tests/fleet.rs @@ -1,6 +1,8 @@ //! F4 — the fleet counts tiers, and a queued child is a count and never a row. //! F5 — the view opens over the composer, updates live, and costs no row it does //! not have. +//! F9 — a child spawned from a roster entry that asks for its own worktree says +//! so, and no row names a directory. //! //! Everything here is driven by a synthetic `RunEvent` stream, which is the whole //! point: the model has to be right for a tree that queues, drains, detaches and @@ -13,8 +15,9 @@ use std::time::Duration; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use io_cli::app::App; use io_cli::fleet::{Fleet, State}; +use io_cli::glyphs::{ASCII, UNICODE}; use io_cli::theme::DARK; -use io_harness::{EventKind, RunEvent}; +use io_harness::{AgentDef, Agents, EventKind, RunEvent}; fn key(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::NONE) @@ -555,3 +558,121 @@ fn f5_shift_enter_still_writes_a_newline_through_the_open_view() { cannot be written while the view is open", ); } + +/// The roster io-cli already hands to `Fleet::name` for role labelling: one entry +/// that asks for its own worktree, one that does not. +fn roster() -> Agents { + Agents::new() + .with(AgentDef::new("builder").with_worktree()) + .with(AgentDef::new("scout")) +} + +/// F9 — the mark is derived from `contract.agents`, and only from there. +/// +/// Two children, same tree, same shape of address; the only thing that separates +/// them is the roster entry behind each. A view that marked both, or neither, +/// would be reporting something other than what the contract says. +#[test] +fn f9_a_child_of_a_worktree_entry_is_marked_and_an_ordinary_one_is_not() { + let mut fleet = Fleet::new(); + fleet.event(&spawned(1, 0, 7, "port the tokenizer")); + fleet.event(&spawned(1, 0, 8, "look around")); + fleet.name( + &[("builder#7".to_string(), 7), ("scout#8".to_string(), 8)], + &roster(), + ); + assert!(fleet.children()[0].worktree, "{:?}", fleet.children()[0]); + assert!(!fleet.children()[1].worktree, "{:?}", fleet.children()[1]); + + let rows = fleet.rows(80, &UNICODE); + assert!(rows[0].contains("worktree"), "{:?}", rows[0]); + assert!( + !rows[1].contains("worktree"), + "an entry that never asked for one is not marked: {:?}", + rows[1], + ); + // And the mark sits in front of the goal, so the goal is still the column + // that gives way when the row is narrow. + let head = rows[0].find("worktree").expect("the mark"); + let goal = rows[0].find("port the tokenizer").expect("the goal"); + assert!(head < goal, "{:?}", rows[0]); +} + +/// F9 — no row names a directory. +/// +/// The sabotage arm draws the derived `.worktrees/` path instead. It cannot +/// be right: io-harness writes a contained child's real path into its `runs` row, +/// no query selects it back out, and the deriver is private — so anything drawn +/// here is reconstructed from two copied private functions, which the note on +/// `DERIVED_MARK` in `src/fleet.rs` already forbids for exactly this reason. A +/// wrong role costs a label; a wrong path is somewhere an operator goes. +#[test] +fn f9_no_row_names_a_directory() { + let mut fleet = Fleet::new(); + fleet.event(&spawned(1, 0, 7, "port the tokenizer")); + fleet.event(&spawned(1, 0, 8, "look around")); + fleet.name( + &[("builder#7".to_string(), 7), ("scout#8".to_string(), 8)], + &roster(), + ); + for glyphs in [UNICODE, ASCII] { + for row in fleet.rows(80, &glyphs) { + assert!(!row.contains(".worktrees"), "{} {row:?}", glyphs.name); + // No separator either: the goals above carry none, so any that turns + // up was put there by the row. + assert!(!row.contains('/'), "{} {row:?}", glyphs.name); + assert!(!row.contains('\\'), "{} {row:?}", glyphs.name); + } + } +} + +/// F9 — the mark is a word, so it is the same in both sets and fits eighty +/// columns with a long address and a long goal in front of it. +#[test] +fn f9_the_mark_is_ascii_and_the_row_still_fits_eighty_columns() { + let mut fleet = Fleet::new(); + fleet.event(&spawned( + 1, + 0, + 7, + "port the tokenizer, the error paths, and everything that reads either of \ + them, one at a time, without changing behaviour", + )); + fleet.name(&[("builder#7".to_string(), 7)], &roster()); + for glyphs in [UNICODE, ASCII] { + let row = fleet.rows(80, &glyphs).remove(0); + assert!(row.chars().count() <= 80, "{} {row:?}", glyphs.name); + assert!(row.contains("worktree"), "{} {row:?}", glyphs.name); + assert!( + row.contains("builder#7"), + "the identity survives the cut: {} {row:?}", + glyphs.name, + ); + } + assert!( + fleet.rows(80, &ASCII).remove(0).is_ascii(), + "the mark degrades because it never needed a glyph", + ); +} + +/// F9 — an address the roster cannot name is not marked, and a reload that drops +/// the entry unmarks the child rather than remembering it. +#[test] +fn f9_an_unknown_address_is_not_marked_and_the_mark_is_not_kept_stale() { + let mut fleet = Fleet::new(); + fleet.event(&spawned(1, 0, 7, "one")); + fleet.name(&[("left-hand".to_string(), 7)], &roster()); + assert!(!fleet.children()[0].worktree, "nothing was guessed"); + + fleet.name(&[("builder#7".to_string(), 7)], &roster()); + assert!(fleet.children()[0].worktree); + // `/agents` reloaded, and the entry no longer asks for one. + fleet.name( + &[("builder#7".to_string(), 7)], + &Agents::new().with(AgentDef::new("builder")), + ); + assert!( + !fleet.children()[0].worktree, + "the row was still saying what a definition used to say", + ); +} diff --git a/tests/glyphs.rs b/tests/glyphs.rs index ab060c2..4eeeed1 100644 --- a/tests/glyphs.rs +++ b/tests/glyphs.rs @@ -1093,3 +1093,166 @@ async fn the_approval_overlay_draws_in_ascii() { let decision = deciding.await.expect("the approver did not panic"); assert!(matches!(decision, Decision::Deny { .. })); } + +// --------------------------------------------------------------------------- +// 0.25.0 — the worktree mark and the branch field +// --------------------------------------------------------------------------- + +/// **F5 — the fleet's worktree mark is a word, so the two sets spell it +/// identically and there is nothing to degrade.** +/// +/// The claim `src/fleet.rs` makes for the mark is stronger than "it has an ASCII +/// form": it needs no entry in [`Glyphs`] at all, because the eight letters are +/// the same in both sets. So the sweep is not the only assertion here — the two +/// rows are compared character for character with the set's own separator +/// normalised out, and the mark is required to sit at the same column in each. +/// A mark that had reached for a symbol would fail the sweep; one that had +/// reached for a *different* symbol per set would pass the sweep and fail this. +#[test] +fn the_worktree_mark_is_the_same_word_in_both_sets() { + use io_cli::fleet::Fleet; + use io_harness::{AgentDef, Agents}; + + let roster = Agents::new().with(AgentDef::new("builder").with_worktree()); + let mut fleet = Fleet::new(); + fleet.event(&RunEvent::at_depth( + 1, + 1, + 0, + EventKind::Spawned { + child_run_id: 7, + goal: "port the tokenizer".to_string(), + }, + )); + fleet.name(&[("builder#7".to_string(), 7)], &roster); + + let unicode = fleet.rows(80, &UNICODE).remove(0); + let plain = fleet.rows(80, &ASCII).remove(0); + + assert_ascii("the fleet row", &plain); + for row in [&unicode, &plain] { + assert!(row.contains("worktree"), "the mark went missing: {row:?}"); + } + // Columns, not byte offsets: the Unicode separator is two bytes and one cell, + // so a comparison of `str::find` results would report a difference the + // terminal never draws. + let column = |row: &str| row.find("worktree").map(|byte| row[..byte].chars().count()); + assert_eq!( + column(&unicode), + column(&plain), + "the mark moved between the sets\nunicode: {unicode:?}\nascii: {plain:?}", + ); + // The whole row, with each set's separator replaced by the other's, is the + // same string — so the *only* difference between the two is the separator. + assert_eq!( + unicode.replace(UNICODE.separator.trim(), ASCII.separator.trim()), + plain, + "the sets draw more than a separator apart\nunicode: {unicode:?}\nascii: {plain:?}", + ); +} + +/// **F5 — the branch field is a plain name, so it draws the same under the ASCII +/// set, under `--plain` and under `NO_COLOR`.** +/// +/// Three surfaces, because the branch reaches all three and they are drawn by +/// three different pieces of code: `Status::line` builds fields and joins them, +/// `Status::footer` hand-builds two groups, and `Status::branch_field` is what +/// both read. A field that had acquired a glyph in one of them would be a mark on +/// a terminal that cannot draw it, in the one place an operator looks to find out +/// which checkout their agent is writing into. +/// +/// `NO_COLOR` is asserted as text equality rather than as a second sweep: the +/// colour axis must change nothing about *what* is on the row, and `MONO` against +/// `DARK` on the same set is the sharpest way to say so. +#[test] +fn the_branch_field_draws_in_ascii_and_survives_no_colour() { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.branch = Some("feat/0.25.0".into()); + + // Set-independent by construction, and that is the claim rather than an + // observation: nothing in `branch_field` takes a `Glyphs` at all. + let field = status.branch_field().expect("a branch is set"); + assert_eq!(field, "git:feat/0.25.0"); + assert_ascii("the branch field", &field); + + for (background, theme) in [ + ("coloured", ascii()), + ("uncoloured", MONO.with_glyphs(ASCII)), + ] { + let line = row(&status.line(120, &theme)); + assert_ascii("the status line's branch", &line); + assert!( + line.contains(&field), + "{background}: the branch left the status line: {line:?}", + ); + + // The row the branch is actually drawn on — the counts row, which is the + // third and last the footer builds. Swept whole rather than searched, + // because a mark that arrived beside the field would be as unreachable on + // a plain terminal as one inside it. The row above it is the identity row + // and is the subject of its own test below. + let rows = status.footer(120, &theme); + let counted = row(rows.last().expect("the footer's three rows")); + assert_ascii("the footer's counts row", &counted); + assert!( + counted.contains(&field), + "{background}: the branch left the footer: {counted:?}", + ); + } + + // The colour axis changed nothing about the text on either renderer, and the + // glyph axis changed nothing about the field on either. + for set in [UNICODE, ASCII] { + let coloured = DARK.with_glyphs(set); + let uncoloured = MONO.with_glyphs(set); + assert_eq!( + row(&status.line(120, &coloured)), + row(&status.line(120, &uncoloured)), + "{}: NO_COLOR changed what the status line says", + set.name, + ); + assert_eq!( + text(&status.footer(120, &coloured)), + text(&status.footer(120, &uncoloured)), + "{}: NO_COLOR changed what the footer says", + set.name, + ); + assert!( + row(&status.line(120, &coloured)).contains(&field), + "{}: the branch is drawn from the set rather than from the name", + set.name, + ); + } +} + +/// **F5 — the footer, whole, draws in ASCII.** +/// +/// The sweep above covers the counts row because that is where the branch lands. +/// This one covers all three, and it is the surface that most needed one: every +/// existing sweep in this file runs over `Status::line`, which has exactly one +/// production caller — the fallback for a terminal under seven rows — while +/// `Status::render` takes the footer on every real terminal. A mark that only the +/// footer draws has never been swept here at all. +/// +/// Both states, because the identity row draws different things in each: `• +/// ready` when nothing is running, a spinner frame and no word while a turn is. +#[test] +fn the_footer_draws_in_ascii_in_both_states() { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.branch = Some("feat/0.25.0".into()); + status.steps = Some(6); + + let theme = ascii(); + for state in [false, true] { + status.working = state; + // Every frame of the indicator, so a set whose later frames are Unicode + // cannot hide behind the first — the same shape the status line's sweep + // uses. + for _ in 0..UNICODE.spinner.len().max(ASCII.spinner.len()) * 2 { + assert_ascii("the footer", &text(&status.footer(120, &theme))); + status.advance(); + } + } +} diff --git a/tests/live.rs b/tests/live.rs index 4762561..e7f6639 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -2908,3 +2908,295 @@ async fn live_f5_a_rubric_is_judged_by_a_second_model() { let standing = io_cli::gates::standing(&attempts).expect("the review answered"); assert_eq!(standing.phase, "review"); } + +// --------------------------------------------------------------------------- +// 0.25.0 — the git surface. +// --------------------------------------------------------------------------- + +/// Build a real repository at `root`, on `main`, with one committed file. +/// +/// A real `.git`, written by git itself, because [`io_cli::repo`] parses the +/// format git actually writes and a fixture that mimics it would only prove the +/// mimicry. This is the one place in this file that shells out, and it is a test +/// fixture rather than product code — the crate itself starts no process, which +/// `tests/dependencies.rs` asserts by path. +fn a_repository(root: &std::path::Path) { + let git = |args: &[&str]| { + let out = std::process::Command::new("git") + .args(args) + .current_dir(root) + .output() + .expect("git runs"); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr), + ); + }; + git(&["init", "--initial-branch=main"]); + git(&["config", "user.email", "fixture@io-cli.invalid"]); + git(&["config", "user.name", "fixture"]); + std::fs::write(root.join("notes.md"), "# notes\n\nold line\n").expect("the fixture file"); + git(&["add", "notes.md"]); + git(&["commit", "-m", "fixture: the file the agent will change"]); +} + +/// **F8 — a `worktree = true` child works in a checkout of its own.** +/// +/// The rooting happens inside io-harness, before the child's first step, off a +/// roster io-cli passes in — and there is no public reader for a child's root, so +/// this asserts on the filesystem the harness created rather than on a store row. +/// That is the honest assertion available: `.worktrees/` exists and holds a +/// checkout, and the parent is still on `main`. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f8_a_worktree_child_works_in_a_checkout_of_its_own() { + use io_cli::contract::Capabilities; + use io_harness::{AgentDef, Agents, Containment}; + + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + a_repository(root); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let provider = io_harness::OpenRouter::new(&key, model()); + let policy = workspace_policy(); + + let (answerer, _questions) = io_cli::intent::channel(); + let roster = Agents::new().with( + AgentDef::new("builder") + .with_role("Edit the file you are asked to edit.") + .with_max_steps(6) + .with_worktree(), + ); + let contract = io_cli::contract::session( + "Spawn the `builder` agent and ask it to replace the line `old line` in \ + notes.md with `new line`. Wait for it.", + root.to_path_buf(), + &no_configuration(), + &Capabilities::default(), + Arc::new(answerer), + None, + ) + .with_agents(roster) + .with_max_steps(12); + + let result = session + .turn_contained_bounded_observed( + &contract, + &provider, + &store, + &policy, + &io_harness::ApproveAll, + &Containment::new(4, 2, 1, 200_000), + &io_harness::Ignore, + ) + .await + .expect("the turn runs"); + + let worktrees = root.join(".worktrees"); + let made: Vec = std::fs::read_dir(&worktrees) + .map(|entries| { + entries + .filter_map(|entry| entry.ok()) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect() + }) + .unwrap_or_default(); + println!("live 0.25.0 F8: run {} worktrees {made:?}", result.run_id); + + assert!( + !made.is_empty(), + "a `worktree = true` roster entry must root its child under `.worktrees/`; \ + nothing was created, so the roster did not reach the spawn", + ); + + // The child's checkout is a real one, and the parent did not move onto it. + for name in &made { + let child = worktrees.join(name); + assert!( + io_cli::repo::branch(&child).is_some(), + "the child's checkout at {child:?} has no readable head, so it is a \ + directory rather than a worktree", + ); + } + assert_eq!( + io_cli::repo::branch(root).as_deref(), + Some("main"), + "the parent must stay where it was; a child taking the parent's branch \ + with it is the overwriting this switch exists to stop", + ); +} + +/// **F5 — a commit the agent made comes back with the message it wrote.** +/// +/// The whole chain in one turn: the agent calls `git_commit`, the message +/// survives only in the typed call, and `commit::made_in` reads it back off +/// `Store::step_turns`. Asserted against the repository as well as the store, so +/// a block that rendered without a commit behind it fails here. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f5_a_commit_the_agent_made_reads_back_with_its_message() { + use io_cli::contract::Capabilities; + + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + a_repository(root); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let provider = io_harness::OpenRouter::new(&key, model()); + let policy = workspace_policy(); + + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::session( + "Replace the line `old line` in notes.md with `new line`, then stage and \ + commit that change with a message describing it.", + root.to_path_buf(), + &no_configuration(), + &Capabilities::default(), + Arc::new(answerer), + None, + ) + .with_max_steps(12); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &policy, + &io_harness::ApproveAll, + &io_harness::Ignore, + ) + .await + .expect("the turn runs"); + + let turns = store + .step_turns(result.run_id) + .expect("the assistant turns"); + let made = io_cli::commit::made_in(&turns); + println!("live 0.25.0 F5: commits {made:?}"); + + assert!( + !made.is_empty(), + "the agent was asked to commit and no `git_commit` call came back off \ + `Store::step_turns`; the message survives nowhere else", + ); + + for commit in &made { + assert!( + !commit.subject().is_empty(), + "a commit block with an empty subject says nothing: {commit:?}", + ); + } + + // And the repository agrees. A block drawn from a call the policy refused + // would pass everything above and fail here, which is why this assertion is + // against git rather than against the store. + let log = std::process::Command::new("git") + .args(["log", "--oneline", "--no-decorate"]) + .current_dir(root) + .output() + .expect("git log runs"); + let log = String::from_utf8_lossy(&log.stdout); + println!("live 0.25.0 F5: log\n{log}"); + assert!( + log.lines().count() >= 2, + "the fixture commit is one; the agent's is the second, and the log has \ + only: {log}", + ); +} + +/// **F1/F2 — an asking posture refuses git before anything is spent, and the one +/// rule lifts it.** +/// +/// Asserted against a real run rather than against a `Policy` alone, because the +/// claim this release rests on is about what io-harness's *spawn* does with an +/// asking posture, and that is only observable by letting it try. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f1_f2_an_asking_posture_refuses_git_and_the_allowance_lifts_it() { + use io_cli::contract::Capabilities; + + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + a_repository(root); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let provider = io_harness::OpenRouter::new(&key, model()); + + // The posture io-cli's own wizard recommends, and the one most operators run. + let asking = Policy { + layers: Policy::default().layers, + defaults: Posture::AskWrites.defaults(), + }; + assert!( + io_cli::approval::refuses_git(&asking), + "the premise of this release: an asking posture refuses git", + ); + + let lifted = io_cli::approval::effective_policy(&asking, &[io_cli::approval::git_allowance()]); + assert!( + !io_cli::approval::refuses_git(&lifted), + "and the one rule lifts it", + ); + + let collected = Arc::new(Mutex::new(Vec::new())); + let observer = Collector { + events: Arc::clone(&collected), + }; + + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::session( + "Run `git status` to see what has changed in this repository, then say what it said.", + root.to_path_buf(), + &no_configuration(), + &Capabilities::default(), + Arc::new(answerer), + None, + ) + .with_max_steps(6); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &asking, + &io_harness::ApproveAll, + &observer, + ) + .await + .expect("the turn runs"); + + let events = collected.lock().expect("not poisoned").clone(); + let refusals: Vec<(String, String)> = events + .iter() + .filter_map(|event: &RunEvent| match &event.kind { + EventKind::Refused { act, target, .. } => Some((act.clone(), target.clone())), + _ => None, + }) + .collect(); + println!( + "live 0.25.0 F1/F2: run {} refusals {refusals:?}", + result.run_id + ); + + // **The refusal io-cli explains, arriving from the harness rather than from a + // fixture.** `ApproveAll` is handed in deliberately: it answers the `.git` + // write gate, so anything refused here was refused by the spawn's own exec + // check, which is the behaviour reported upstream as io-harness#214. + assert!( + refusals + .iter() + .any(|(act, target)| act == "exec" && target == io_cli::approval::GIT), + "an asking posture must produce an `exec`/`git` refusal — that is what \ + `App::note_git` explains and what `/commit allow` lifts; got {refusals:?}", + ); +} diff --git a/tests/narrow.rs b/tests/narrow.rs index 9f2d563..4b85eb3 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1544,6 +1544,317 @@ fn f5_the_fleet_view_fits_eighty_columns_in_both_glyph_sets() { } } +/// **0.25.0 F5 — the worktree mark fits eighty columns in both sets, and the goal +/// is what pays for it, to the column.** +/// +/// The mark is a word rather than a symbol, so the two sets spell it identically +/// and the arithmetic is the same on both — which is the claim, not an +/// assumption. It is drawn immediately in front of the goal, and the goal is the +/// only column on the row that can give way, so the cost is measurable: the same +/// child, the same address, the same goal, named once from a roster entry that +/// asks for a worktree and once from one that does not. The marked row keeps +/// **exactly eleven fewer characters** of its goal — `worktree` is eight, the +/// separator between it and the goal is three — and that is the whole bill. +/// +/// Both rows are fitted rather than clipped, which the trailing mark says: a row +/// that had overrun eighty would fail the width assertion, and one that had lost +/// the identity in front of the goal would fail the `builder#7` assertion. +/// +/// Sabotage: draw the mark *after* the goal. The width assertion still passes, +/// the row still contains the word — and the eleven-character claim fails, +/// because the goal is then fitted to the room the mark has not taken yet and the +/// mark is what runs off the end. +#[test] +fn f5_the_worktree_mark_fits_eighty_columns_and_the_goal_pays_for_it() { + use io_cli::fleet::Fleet; + use io_harness::{AgentDef, Agents, EventKind, RunEvent}; + + let goal = "port the tokenizer, the error paths, and everything that reads either \ + of them, one at a time, without changing behaviour"; + + // The same entry twice, differing in the one field the mark is derived from. + // `Fleet::name` assigns rather than merges, so naming the same child from the + // second roster is the whole change between the two rows below. + let plain = Agents::new().with(AgentDef::new("builder")); + let contained = Agents::new().with(AgentDef::new("builder").with_worktree()); + + // How much of `goal` survived on this row: the longest prefix of it the row + // still contains. `fit` truncates by character and appends the set's mark, so + // this is exactly the room the goal was given minus the mark. + let kept = |row: &str| -> usize { + (0..=goal.chars().count()) + .rev() + .find(|n| row.contains(&goal.chars().take(*n).collect::())) + .expect("the empty prefix is in every row") + }; + + for theme in themes() { + let set = theme.glyphs.name; + let mut fleet = Fleet::new(); + fleet.event(&RunEvent::at_depth( + 1, + 1, + 0, + EventKind::Spawned { + child_run_id: 7, + goal: goal.to_string(), + }, + )); + + fleet.name(&[("builder#7".to_string(), 7)], &plain); + let before = fleet.rows(WIDTH, &theme.glyphs).remove(0); + fleet.name(&[("builder#7".to_string(), 7)], &contained); + let after = fleet.rows(WIDTH, &theme.glyphs).remove(0); + + for row in [&before, &after] { + assert!( + row.chars().count() <= WIDTH as usize, + "{set}: {row:?} is wider than the terminal", + ); + assert!( + row.contains("builder#7") && row.contains("working"), + "{set}: the identity survives the cut: {row:?}", + ); + assert!( + row.ends_with(theme.glyphs.ellipsis), + "{set}: the goal is what gets cut, and says so with the set's own \ + mark: {row:?}", + ); + } + + assert!( + after.contains("worktree"), + "{set}: the mark went missing at eighty columns: {after:?}", + ); + assert!( + !before.contains("worktree"), + "{set}: an entry that never asked for one is not marked: {before:?}", + ); + // In front of the goal, so the goal is still the column that gives way. + assert!( + after.find("worktree") < after.find("port the tokenizer"), + "{set}: the mark is drawn after the goal: {after:?}", + ); + + // The bill, in columns, and it is the same in both sets because the mark + // is a word and the separator is three cells in each. + assert_eq!( + kept(&after) + 11, + kept(&before), + "{set}: the mark cost the goal something other than eleven columns\n\ + plain: {before:?}\nmarked: {after:?}", + ); + } +} + +/// **0.25.0 F11 — the branch is a plain name, and it costs what it says it +/// costs.** +/// +/// Three shapes, because they are the three a real checkout produces and their +/// widths are what every fitter on the footer is spending: `main` on an ordinary +/// branch, a detached head, and the branch this repository is actually on. The +/// widths are stated rather than derived, so a field that grew a decoration — +/// brackets, an arrow, a dirty marker — moves a number here rather than quietly +/// eating a counter off the footer. +/// +/// The separator is counted in, because the field never appears without one: it +/// is appended to a group that already has members on every row it can reach. +/// Three cells in both sets, so the bill is the same in each. +#[test] +fn f11_the_branch_field_is_a_plain_name_and_costs_a_stated_number_of_columns() { + use io_cli::status::Status; + + for theme in themes() { + let set = theme.glyphs.name; + let separator = theme.glyphs.separator.chars().count(); + let mut status = Status::new("io/model"); + + assert_eq!( + status.branch_field(), + None, + "{set}: a workspace with no readable head draws no field at all", + ); + + for (branch, cost) in [("main", 11), ("a1b2c3d", 14), ("feat/0.25.0", 18)] { + status.branch = Some(branch.to_string()); + let field = status + .branch_field() + .unwrap_or_else(|| panic!("{set}: a branch is set and no field was drawn")); + + assert_eq!( + field, + format!("git:{branch}"), + "{set}: the branch is a plain name and nothing else", + ); + assert_eq!( + field.chars().count() + separator, + cost, + "{set}: `{field}` no longer costs {cost} columns", + ); + assert!( + field.is_ascii(), + "{set}: the field needs a glyph it does not have: {field:?}", + ); + + // And on the one-row form it reaches the terminal whole rather than + // shortened, at a width that has room for it. + let line = text_of(&[status.line(WIDTH, &theme)]).remove(0); + assert!( + line.chars().count() <= WIDTH as usize, + "{set}: the status line overran eighty columns: {line:?}", + ); + assert!( + line.contains(&field), + "{set}: the branch was shortened rather than dropped whole: {line:?}", + ); + } + } +} + +/// **0.25.0 F11 — the branch reaches the footer, and yields before the standing +/// facts when the row cannot hold it.** +/// +/// **Two widths, and the first one is why.** The first version of this test +/// asserted only at eighty columns — where, with this fixture, the branch never +/// fits — so the footer with a branch and the footer without one were +/// character-identical and every assertion was equally true of both. Deleting +/// the push from `Status::footer` entirely left it green. It was a gate over a +/// field it could not observe, which is the shape 0.20.0's +/// `a_queue_behind_an_open_fleet_view_answers_no_key` had and the shape this +/// release already found once in `tests/commands.rs`. +/// +/// So the wide arm proves the field is drawn at all, and the narrow arm proves +/// what it gives up when it cannot be. Neither alone is a gate. +/// +/// The fixture is `tests/status.rs`'s own `full_row` without the planning phase: +/// the posture and containment word `src/status.rs` names in its comments as the +/// real thing, and the counters the footer had when the 0.21.0 live capture's +/// turn ended. The right-hand group is `read-only` (9) plus the separator (3) +/// plus `workspace-write/macos-sandbox-exec` (34) — forty-six columns. The key +/// hint is fourteen and its separator three. +/// +/// **The branch is in `counts`, pushed last, so `counts.pop()` takes it before it +/// takes any number.** That is the corrected design: a branch is the one fact on +/// this row an operator can afford to lose, because it is a keystroke away on the +/// `/status` page, while a posture that silently vanished is not. The rejected +/// design put it in the right-hand group, which `row` keeps or drops whole — and +/// at eighty columns with `planning` on, every real branch took the posture, the +/// containment mode and the planning phase off together. +/// +/// Sabotage: put the branch back in the right-hand group — under which the narrow +/// arm's standing-facts assertions fail. Or delete the push altogether — under +/// which the wide arm fails, which is the half that was missing. +#[test] +fn f11_a_branch_yields_before_the_standing_facts_at_eighty_columns() { + use io_cli::status::Status; + + let fixture = || { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.steps = Some(6); + status.tokens = Some(26_900); + status.context = Some(23); + status + }; + + for theme in themes() { + let set = theme.glyphs.name; + let footer = |status: &Status| -> String { + let rows = text_of(&status.footer(WIDTH, &theme)); + for row in &rows { + assert!( + row.chars().count() <= WIDTH as usize, + "{set}: a footer row overflowed eighty columns: {row:?}", + ); + } + rows.join("\n") + }; + // The same renderer at a width that can hold everything, for the arm that + // proves the field exists at all rather than what it displaces. + let footer_at = |status: &Status, width: u16| -> String { + text_of(&status.footer(width, &theme)).join("\n") + }; + + let bare = footer(&fixture()); + assert!( + bare.contains("6 steps"), + "{set}: the row did not hold a counter to begin with, so nothing below \ + measures anything: {bare:?}", + ); + assert!( + !bare.contains("26.9k tok") && !bare.contains("ctx 23%"), + "{set}: eighty columns already cost these two before a branch was set, \ + and the comparison below assumes it: {bare:?}", + ); + + let mut branched = fixture(); + branched.branch = Some("feat/0.25.0".into()); + let drawn = footer(&branched); + + // **The standing facts survive, and the branch is what yields.** This is + // the corrected behaviour and the assertion is written the way round it + // is because the first attempt at this field was the other way round: the + // branch went into the right-hand group, which `row` keeps or drops + // whole, and at eighty columns with `planning` on it took the posture, + // the containment mode and the planning phase off together — leaving a + // bare key hint. Every real branch did it; the break-even was a name one + // character long. + // + // So a branch is the one fact on this row an operator can afford to lose. + // It is a keystroke away on the `/status` page; a posture that silently + // vanished is not, and an operator finishing a turn under `/plan on` with + // nothing on screen saying the next write will stop and ask is exactly + // what `f4_a_full_counts_row_drops_a_counter_and_not_the_planning_phase` + // exists to prevent. + for standing in ["read-only", "workspace-write/macos-sandbox-exec"] { + assert!( + drawn.contains(standing), + "{set}: `{standing}` left the footer at eighty columns — the branch \ + must never be paid for out of the standing facts: {drawn:?}", + ); + } + // The counter survives too: the branch yields before any number does, + // because `counts.pop()` takes from the end and it is pushed last. + assert!( + drawn.contains("6 steps"), + "{set}: a counter was dropped to make room for a branch that then did \ + not fit either: {drawn:?}", + ); + assert!( + !drawn.contains("git:feat/0.25.0"), + "{set}: the branch fitted at eighty columns beside everything else, so \ + this fixture no longer measures the trade it was written for: {drawn:?}", + ); + // The hint is not a counter and is never the thing that pays. + assert!( + drawn.contains("/ for commands"), + "{set}: the key hint was dropped to make room for the branch: {drawn:?}", + ); + + // **The wide arm, and it is the half that was missing.** Everything above + // is equally true of a footer that never had a branch at all, because at + // eighty columns this fixture cannot fit one either way — so on its own it + // is a gate over a field it cannot observe. Given room, the branch must + // actually be drawn, and here it is asserted where nothing else can + // explain its presence. + let roomy = footer_at(&branched, 200); + assert!( + roomy.contains("git:feat/0.25.0"), + "{set}: the branch never reaches the footer at all — delete the push \ + from `Status::footer` and every narrow assertion above still passes, \ + which is what made the first version of this test vacuous: {roomy:?}", + ); + // And it is still the last thing in its group, which is what makes it the + // first to go when the room disappears. + let bare_wide = footer_at(&fixture(), 200); + assert!( + !bare_wide.contains("git:"), + "{set}: a branch appeared with none set: {bare_wide:?}", + ); + } +} + /// 0.11.0 F5 — the activity line drops fields from the right, and never wraps. /// /// The same rule the status line under it follows, and the same order the @@ -1683,6 +1994,10 @@ fn f11_status_folds_at_eighty_columns_rather_than_losing_the_end_of_a_row() { std::time::Duration::ZERO, ); app.status.budgets = io_cli::status::Budgets::in_force(&contract); + // The branch this page draws since 0.25.0, and a real one rather than `main`: + // the field is a plain name with no width of its own to hide behind, and a + // name short enough to fit anywhere would not be asked to survive the fold. + app.status.branch = Some("feat/0.25.0".into()); for theme in themes() { let set = theme.glyphs.name; @@ -1720,6 +2035,10 @@ fn f11_status_folds_at_eighty_columns_rather_than_losing_the_end_of_a_row() { session.root().display().to_string(), skills.display().to_string(), "organisation-baseline-secrets-and-egress".to_string(), + // The branch as a plain name — no `git:` prefix here, because this + // page is one fact per row and the label beside it already says what + // it is. + "feat/0.25.0".to_string(), "up to 12 agents, 4 at once per tier, 2 deep, 200000 tokens for the tree".to_string(), ] { let wanted: String = whole.chars().filter(|c| !c.is_whitespace()).collect(); @@ -1742,6 +2061,7 @@ fn f11_status_folds_at_eighty_columns_rather_than_losing_the_end_of_a_row() { let page = rows.join("\n"); for label in [ "workspace:", + "branch:", "session:", "model:", "provider:", diff --git a/tests/palette.rs b/tests/palette.rs index fbbde92..8c8ec0e 100644 --- a/tests/palette.rs +++ b/tests/palette.rs @@ -304,11 +304,11 @@ fn f1_an_exact_name_outranks_a_prefix_which_outranks_a_scattered_match() { #[test] fn f1_equal_scores_keep_the_first_row_still_between_keystrokes() { - // `c` matches NINE rows since 0.22.0: `copy`, `copy diff`, `contain`, - // `clear`, `config`, `context`, `compact` and now `cost` all begin with it, - // and `mcp` merely holds one — which is the point of the arm, since a + // `c` matches TEN rows since 0.25.0: `copy`, `copy diff`, `contain`, + // `clear`, `config`, `context`, `compact`, `cost` and now `commit` all begin + // with it, and `mcp` merely holds one — which is the point of the arm, since a // scattered match ranks below every prefix. `attach`, which also merely held - // one, stopped being a command in 0.13.1. `co` narrows to the seven that + // one, stopped being a command in 0.13.1. `co` narrows to the eight that // begin with `co`: `clear` has no `o` after its `c`, and neither does `mcp`. // (`stats`, the other command 0.22.0 added, holds no `c` at all.) // The three score the same and the tie-break is the order they were handed @@ -321,14 +321,14 @@ fn f1_equal_scores_keep_the_first_row_still_between_keystrokes() { // quietly stopped happening. let mut picker = palette(); type_at(&mut picker, "c"); - assert_eq!(picker.matching(), 9); + assert_eq!(picker.matching(), 10); // `clear` and not `copy` since the list became grouped: the tie-break is the // order the rows were handed in, and the session group now leads. The // PROPERTY is unchanged and is what this test is about — the marked row must // not move on a keystroke that did not change the result. assert_eq!(marked(&picker), "clear"); type_at(&mut picker, "o"); - assert_eq!(picker.matching(), 7); + assert_eq!(picker.matching(), 8); assert_eq!( marked(&picker), "contain", diff --git a/tests/policy.rs b/tests/policy.rs index a77e97f..0ab85d0 100644 --- a/tests/policy.rs +++ b/tests/policy.rs @@ -1,10 +1,16 @@ -//! F6 and F7 — the boundary the operator chooses, and the one the agent gets. +//! F1, F6 and F7 — the boundary the operator chooses, the one the agent gets, +//! and the one act that never reaches the operator at all. //! -//! Two criteria and they are deliberately not the same one. F6 is what the status -//! line says; F7 is what the next turn actually runs under. A mode indicator that -//! is not backed by the policy it names is invisible to every assertion on a +//! F6 and F7 are deliberately not the same criterion. F6 is what the status line +//! says; F7 is what the next turn actually runs under. A mode indicator that is +//! not backed by the policy it names is invisible to every assertion on a //! rendered line, which is why the second criterion asserts on an //! `io_harness::Policy` and never on a word. +//! +//! F1 is the same question asked of one act. `io_harness::tools::git` refuses a +//! spawn on any effect that is not `Allow` without consulting an approver, so +//! there is no rendered sentence to assert on even in principle — the only place +//! the repair is visible is the verdict. mod support; @@ -176,3 +182,73 @@ fn a_posture_is_recognised_from_the_defaults_it_is() { "a policy nobody offered is not silently reported as one that was", ); } + +/// **F1.** The posture the wizard recommends refuses git, and refuses it where no +/// approver is reachable: `Git::run` (io-harness 0.69.0, `src/tools/git.rs:549`) +/// treats every effect that is not `Allow` as a hard refusal. Asserted as +/// `!= Allow` rather than `== Ask`, because that inequality is the whole predicate +/// upstream applies. +#[test] +fn f1_an_asking_posture_refuses_git_before_anyone_is_asked() { + let base = Policy::default(); + let policy = approval::session_policy(&base, Some(Posture::AskWrites), &[]); + assert_ne!( + policy.check(Act::Exec, "git").effect, + Effect::Allow, + "the recommended posture leaves git short of allow, which upstream refuses", + ); + assert!(approval::refuses_git(&policy)); +} + +/// **F1.** The repair, offered through the mechanism that already exists for +/// everything else the operator allows for the session. +#[test] +fn f1_the_git_allowance_turns_that_refusal_into_an_allow() { + let base = approval::session_policy(&Policy::default(), Some(Posture::AskWrites), &[]); + let policy = approval::effective_policy(&base, &[approval::git_allowance()]); + assert_eq!( + policy.check(Act::Exec, "git").effect, + Effect::Allow, + "with the allowance in force the seven git tools may spawn", + ); + assert!(!approval::refuses_git(&policy)); +} + +/// **F1, and the half the sabotage attacks.** One rule, one program. An +/// `Act::Exec` pattern names a binary, so a grant written wider than the binary it +/// was for is a grant of the whole PATH. +#[test] +fn f1_the_git_allowance_changes_nothing_for_another_program() { + let base = approval::session_policy(&Policy::default(), Some(Posture::AskWrites), &[]); + let before = base.check(Act::Exec, "curl").effect; + let policy = approval::effective_policy(&base, &[approval::git_allowance()]); + assert_eq!( + policy.check(Act::Exec, "curl").effect, + before, + "allowing git must leave every other binary exactly where it was", + ); + assert_ne!( + policy.check(Act::Exec, "curl").effect, + Effect::Allow, + "an allowance for one program is not an allowance for the machine", + ); +} + +/// **F1.** A layer that denies exec still denies it afterwards. The allowance +/// rides the same `remembered` layer as everything else, and a later layer may add +/// capability but may never re-allow what an earlier one denied — so this is a +/// property of the mechanism, asserted rather than assumed. +#[test] +fn f1_a_denied_exec_is_still_denied_after_the_git_allowance() { + let base = Policy::default().layer("locked-down").deny_exec("git"); + for posture in Posture::ALL { + let policy = approval::session_policy(&base, Some(*posture), &[approval::git_allowance()]); + assert_eq!( + policy.check(Act::Exec, "git").effect, + Effect::Deny, + "{:?}: the allowance re-opened a spawn a layer had denied", + posture, + ); + assert!(approval::refuses_git(&policy)); + } +} diff --git a/tests/repo.rs b/tests/repo.rs new file mode 100644 index 0000000..e118971 --- /dev/null +++ b/tests/repo.rs @@ -0,0 +1,325 @@ +//! F3 — the branch is read from `.git/HEAD`, in every shape a real one takes. +//! +//! Every fixture here is a real `.git`, written with the same bytes git writes: +//! the trailing newline it always leaves on `HEAD`, the `gitdir:` file a linked +//! worktree gets instead of a directory, the per-worktree `commondir` beside it. +//! Nothing is mocked, because the only thing this criterion can get wrong is the +//! file format, and a fixture that invents the format asserts the invention. +//! +//! The shapes that matter are the ones a developer's own checkout does not have. +//! A symbolic ref works on the first try in anybody's clone; a detached head, a +//! linked worktree and a directory that was never a repository are the three that +//! reach an operator's screen without ever having reached the author's, so they +//! get more tests here than the ordinary case does. +//! +//! **`None` and `Some("")` are not the same answer**, and half of this file exists +//! to keep them apart. `None` draws no field at all; an empty string draws a label +//! with a hole where the branch should be, which reads as "you are nowhere" rather +//! than "this is not a repository". Every malformed input below is asserted absent +//! rather than merely falsy. + +use std::path::Path; + +use io_cli::repo; + +/// A real sha1 object id, and what seven characters of it look like. +/// +/// Written out rather than generated so the expected value below is a constant a +/// reader can check by eye against the fixture. +const ID: &str = "9fceb02d0ae598e95dc970b74767f19372d61af8"; +const SHORT: &str = "9fceb02"; + +fn write(path: &Path, contents: &str) { + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the directory"); + std::fs::write(path, contents).expect("the file"); +} + +/// Write what `git init` leaves on disk under `root`, with `head` as the literal +/// contents of `.git/HEAD`. +/// +/// The `config`, `objects/` and `refs/heads/` entries are not read by anything +/// under test. They are here so that every fixture is a repository git itself +/// would recognise — a test whose fixture is only the one file it reads cannot +/// notice the day the reading widens. +fn checkout(root: &Path, head: &str) { + let git = root.join(".git"); + write(&git.join("HEAD"), head); + write( + &git.join("config"), + "[core]\n\trepositoryformatversion = 0\n\tbare = false\n", + ); + std::fs::create_dir_all(git.join("objects")).expect("the object store"); + std::fs::create_dir_all(git.join("refs").join("heads")).expect("the ref store"); + + // A symbolic head names a branch, and a branch git wrote has a loose ref + // holding its tip. Writing it keeps the fixture honest for the nested case in + // particular, where the ref is a nested *directory* under `refs/heads/`. + // The empty name is skipped rather than written: `refs/heads/` with nothing + // after it is one of the malformed heads asserted below, and git has no file + // for it either. + match head.trim().strip_prefix("ref: refs/heads/") { + Some(name) if !name.is_empty() => { + write( + &git.join("refs").join("heads").join(name), + &format!("{ID}\n"), + ); + } + _ => {} + } +} + +/// Link a worktree at `tree` to the repository at `root`, the way +/// `git worktree add` does. +/// +/// The worktree's `.git` is a **file**, and its head lives in the repository's +/// `worktrees//` directory rather than beside it — which is the whole reason +/// following the indirection is a criterion. `named` is what that file says, so a +/// caller can hand it either the absolute path git usually writes or the relative +/// one it writes under `worktree.useRelativePaths`. +fn worktree(root: &Path, tree: &Path, name: &str, head: &str, named: &str) { + let inner = root.join(".git").join("worktrees").join(name); + write(&inner.join("HEAD"), head); + write(&inner.join("commondir"), "../..\n"); + write( + &inner.join("gitdir"), + &format!("{}\n", tree.join(".git").display()), + ); + write(&tree.join(".git"), &format!("gitdir: {named}\n")); +} + +#[test] +fn f3_a_symbolic_ref_is_the_branch_name() { + let dir = tempfile::tempdir().expect("a temporary directory"); + checkout(dir.path(), "ref: refs/heads/main\n"); + + assert_eq!(repo::branch(dir.path()).as_deref(), Some("main")); +} + +#[test] +fn f3_a_nested_branch_keeps_every_component_after_refs_heads() { + let dir = tempfile::tempdir().expect("a temporary directory"); + checkout(dir.path(), "ref: refs/heads/feature/nested-name\n"); + + // The tempting implementation takes the last path component, and it is wrong + // in the exact place this product lives: every release branch here is + // `feat/`, which that implementation would report as the version + // alone, on every screen, for the whole release. + assert_eq!( + repo::branch(dir.path()).as_deref(), + Some("feature/nested-name"), + ); +} + +#[test] +fn f3_a_detached_head_is_a_short_object_id() { + let dir = tempfile::tempdir().expect("a temporary directory"); + checkout(dir.path(), &format!("{ID}\n")); + + let head = repo::branch(dir.path()).expect("a detached head still says where it is"); + assert_eq!(head, SHORT); + // Seven is not an arbitrary number: it is what `git log --oneline` and every + // code host abbreviate to, so the id here is the id the operator will match + // against somewhere else. + assert_eq!(head.len(), repo::SHORT_ID); +} + +#[test] +fn f3_a_head_without_a_trailing_newline_reads_the_same() { + let with = tempfile::tempdir().expect("a temporary directory"); + let without = tempfile::tempdir().expect("a temporary directory"); + checkout(with.path(), "ref: refs/heads/main\n"); + // git always leaves the newline. An editor, a script, or a filesystem that + // truncated it does not make the branch unknown, and neither does a CRLF from + // a file written on Windows. + checkout(without.path(), "ref: refs/heads/main"); + + assert_eq!(repo::branch(with.path()), repo::branch(without.path())); + assert_eq!(repo::branch(without.path()).as_deref(), Some("main")); + + let crlf = tempfile::tempdir().expect("a temporary directory"); + checkout(crlf.path(), "ref: refs/heads/main\r\n"); + assert_eq!(repo::branch(crlf.path()).as_deref(), Some("main")); +} + +#[test] +fn f3_a_linked_worktree_is_followed_to_its_own_head() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let root = dir.path().join("main"); + let tree = dir.path().join("tree"); + checkout(&root, "ref: refs/heads/main\n"); + std::fs::create_dir_all(&tree).expect("the worktree"); + worktree( + &root, + &tree, + "tree", + "ref: refs/heads/feat/0.25.0\n", + &root + .join(".git") + .join("worktrees") + .join("tree") + .display() + .to_string(), + ); + + // Reading the worktree's own head rather than the repository's is the point: + // a `worktree = true` child agent is on a different branch to its parent, and + // reporting the parent's would be a confident wrong answer rather than none. + assert_eq!(repo::branch(&tree).as_deref(), Some("feat/0.25.0")); + assert_eq!(repo::branch(&root).as_deref(), Some("main")); +} + +#[test] +fn f3_a_relative_gitdir_is_resolved_against_the_worktree() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let root = dir.path().join("main"); + let tree = dir.path().join("tree"); + checkout(&root, "ref: refs/heads/main\n"); + std::fs::create_dir_all(&tree).expect("the worktree"); + // What git writes under `worktree.useRelativePaths`, and what a checkout that + // has been moved as a whole carries. Relative to the directory holding the + // `.git` file, which is the worktree root — not to the process's current + // directory, which is where this would go wrong silently. + worktree( + &root, + &tree, + "tree", + &format!("{ID}\n"), + "../main/.git/worktrees/tree", + ); + + assert_eq!(repo::branch(&tree).as_deref(), Some(SHORT)); +} + +#[test] +fn f3_a_directory_that_is_not_a_repository_has_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + + // io-cli runs in plenty of these and must not become worse in them. No `.git` + // at all is the ordinary case, not an error. + assert_eq!(repo::branch(dir.path()), None); +} + +#[test] +fn f3_a_missing_head_is_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + std::fs::create_dir_all(dir.path().join(".git").join("objects")).expect("a git directory"); + + assert_eq!(repo::branch(dir.path()), None); +} + +#[test] +fn f3_a_head_that_is_not_an_object_id_is_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + // The failure mode this guards is not a crash. It is seven characters of + // whatever the file happened to hold arriving on the status line looking + // exactly like a commit. + for garbage in [ + "not a reference at all\n", + "gitdir: /somewhere/else\n", + "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz\n", + // Thirty-nine hex characters and one that is not. The check has to cover + // the whole line, not the seven characters that get reported. + "9fceb02d0ae598e95dc970b74767f19372d61afg\n", + ] { + checkout(dir.path(), garbage); + assert_eq!( + repo::branch(dir.path()), + None, + "a HEAD reading {garbage:?} is not an object id", + ); + } +} + +#[test] +fn f3_a_head_too_short_to_be_an_object_id_is_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + // Six hex characters. Every character is legal and the whole thing is still + // not an id, so the length is checked before the slice rather than after — + // taking seven characters from six is where a truncating implementation + // panics or, worse, quietly reports the six. + checkout(dir.path(), "9fceb0\n"); + + assert_eq!(repo::branch(dir.path()), None); +} + +#[test] +fn f3_a_head_that_names_something_other_than_a_branch_is_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + for reference in [ + "ref: refs/tags/v0.25.0\n", + "ref: refs/remotes/origin/main\n", + ] { + checkout(dir.path(), reference); + assert_eq!( + repo::branch(dir.path()), + None, + "{reference:?} does not put the tree on a branch", + ); + } +} + +#[test] +fn f3_a_gitdir_file_naming_nothing_is_no_field() { + let dir = tempfile::tempdir().expect("a temporary directory"); + for broken in ["gitdir:\n", "gitdir: \n", "", "gitdir: /nowhere/at/all\n"] { + write(&dir.path().join(".git"), broken); + assert_eq!( + repo::branch(dir.path()), + None, + "a .git file reading {broken:?} points at no repository", + ); + } +} + +#[test] +fn f3_no_head_ever_yields_an_empty_name() { + let dir = tempfile::tempdir().expect("a temporary directory"); + // The sabotage this file is scored against reports a detached head as absent. + // This is the mirror of it: the field going blank rather than going away. An + // operator reading a label with nothing after it concludes something about + // where they are; an operator reading no label at all concludes nothing, which + // is the truth here. + for empty in ["", "\n", " \n", "ref:\n", "ref: \n", "ref: refs/heads/\n"] { + checkout(dir.path(), empty); + let answer = repo::branch(dir.path()); + assert_eq!( + answer, None, + "a HEAD reading {empty:?} named no branch, and an empty field is not \ + how that is said", + ); + } +} + +/// **F3 — a workspace below the repository root still knows its branch.** +/// +/// `io -C src` is an ordinary invocation, and git itself walks up from the +/// working directory — so the agent's seven built-ins find the repository while a +/// check that looked only at `/.git` found nothing and said, permanently, +/// that a perfectly readable checkout had no branch. Found by the adversarial +/// review; none of the arms above reached it, because every one of them builds +/// its fixture at the root it then asks about. +#[test] +fn f3_a_directory_below_the_repository_root_still_reads_the_branch() { + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + checkout(root, "ref: refs/heads/feat/0.25.0\n"); + + let nested = root.join("src").join("deep"); + std::fs::create_dir_all(&nested).expect("the nested directory"); + + assert_eq!( + repo::branch(&nested).as_deref(), + Some("feat/0.25.0"), + "a directory inside the checkout must read the checkout's branch; git \ + walks up and so must this", + ); + + // And a directory that is genuinely outside any repository still answers + // nothing, rather than walking up far enough to find someone else's. + let elsewhere = tempfile::tempdir().expect("a directory that is not a checkout"); + assert_eq!( + repo::branch(elsewhere.path()), + None, + "walking up must stop at the filesystem, not borrow an unrelated repository", + ); +} diff --git a/tests/status.rs b/tests/status.rs index e88a87f..79e4e8a 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -103,6 +103,123 @@ fn f4_the_planning_phase_is_named_and_survives_the_run() { assert!(!shown(&status).contains("planning")); } +/// **F4 — the branch is on both surfaces, and it describes the checkout rather +/// than the conversation.** +/// +/// It arrives from `EventKind::ToolCall`'s `target`, which io-harness fills with +/// the `name` argument of the `git_branch` call — the branch the agent created +/// and moved onto. Neither `Status::forget_run` nor `Status::start_run` clears +/// it, and that is the property: `/clear`, `/resume`, a rewind and the operator +/// simply typing again all change which conversation is on screen, and none of +/// them checks out another branch. A field cleared by any of them would blank a +/// fact about the operator's own working tree that is still true. +/// +/// Both renderers, through `both_renderers`, because `Status::line` is the +/// one-row fallback and `Status::footer` is what the binary draws on every +/// terminal seven rows or taller. 0.12.0 added a field to `line` alone, asserted +/// `line` alone, went green, and shipped a mode that was nowhere on screen. +/// +/// Sabotage: clear the field in `forget_run` beside `containment` — under which +/// only this test fails, and it fails by blanking a true fact about the +/// operator's checkout every time they start a new conversation. +#[test] +fn f4_the_branch_is_drawn_by_both_renderers_and_survives_the_conversation() { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + // A run-scoped neighbour, so the test states the difference rather than only + // the half it is about: the containment word dies with its run and this does + // not. + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + + status.note_branch(&event(EventKind::ToolCall { + name: "git_branch".into(), + target: "agent/fix-the-flake".into(), + })); + assert_eq!(status.branch.as_deref(), Some("agent/fix-the-flake")); + + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains("git:agent/fix-the-flake"), + "{renderer} does not say which branch the tree is on: {text:?}", + ); + } + + // The conversation under the line changes: `/clear`, `/resume`, `/fork`, a + // rewind. The checkout does not. + status.forget_run(); + assert_eq!( + status.containment, None, + "the containment word belongs to the run that reported it", + ); + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains("git:agent/fix-the-flake"), + "{renderer} forgot the branch when the conversation changed: {text:?}", + ); + } + + // And the ordinary path `forget_run` never reaches: the operator types again. + status.start_run(); + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains("git:agent/fix-the-flake"), + "{renderer} forgot the branch when the next turn started: {text:?}", + ); + } +} + +/// **F4 — a directory that is not a checkout draws no branch field at all.** +/// +/// The absence half, and the rule this whole line is held to: an absent fact is +/// absent, not zero and not a word standing in for one. io-cli runs in plenty of +/// directories that were never a repository and must not become worse there — so +/// there is no empty label, no `none`, and no bare `git:` with nothing after it. +/// +/// The empty `target` arm is the same rule one step earlier. `git_branch` names +/// the branch in its `target`, and an event carrying none is an event with no +/// answer in it; storing the empty string would put a prefix on screen owning +/// nothing. +/// +/// Sabotage: clear the field in `forget_run` beside `containment` — under which +/// only the surviving-the-conversation arm above fails, which is what makes the +/// pair state the field's whole contract. +#[test] +fn f4_a_workspace_with_no_branch_draws_no_branch_field_anywhere() { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + assert_eq!(status.branch, None); + assert_eq!(status.branch_field(), None); + + // An event that names the tool but carries no branch leaves it exactly as it + // was, rather than storing an empty name. + status.note_branch(&event(EventKind::ToolCall { + name: "git_branch".into(), + target: String::new(), + })); + // And so does every other tool call, whatever it points at. + status.note_branch(&event(EventKind::ToolCall { + name: "apply_patch".into(), + target: "src/lib.rs".into(), + })); + assert_eq!( + status.branch, None, + "a call with no branch in it named one anyway", + ); + + for (renderer, text) in both_renderers(&status) { + assert!( + !text.contains("git:"), + "{renderer} drew a branch field where there is no branch: {text:?}", + ); + assert!( + !text.contains("branch"), + "{renderer} named the absent field rather than leaving it out: {text:?}", + ); + assert!( + !text.contains("none"), + "{renderer} spelled the absence as a word: {text:?}", + ); + } +} + /// **F4 — it is a word, not only a tone.** /// /// The rule the whole product is held to, and the one a mode field cannot be