diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ea7d7..06b99d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,52 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.3.0] - 2026-08-17 + +The operator can read what the agent did to their files without leaving the +terminal and without losing the thread. An edit stops being a line saying a file +changed and becomes the change itself. + +### Added + +- **Edits render as diffs.** Not diffs io-cli computed — io-harness already + renders a unified diff for every edit its tools make and keeps it in the run's + durable trace, so what you see carries the file's own `@@` line numbers and is + the same text `Store::patch` would hand `patch`. Additions and removals are + coloured *and* marked, so the meaning survives `NO_COLOR`. +- **Word-level emphasis inside a changed line.** A run of removals is paired with + the run of additions after it only when the two are the same length; anything + else takes the whole wash. A `write_file` that rewrote two distant regions of a + file arrives as one hunk spanning both, and a greedier rule would emphasise the + difference between lines that have nothing to do with each other. +- **Syntax highlighting**, drawn in io-cli's own theme tokens rather than in a + second palette. The three new tokens — keyword, string, literal — are the + theme's, so a highlighted diff and the rest of the interface stay one look, and + `NO_COLOR` is still decided in one place. Green still means added: the parts of + a line both sides share are syntax coloured and the words that actually changed + keep the diff's colour, so the add/remove colour now points at the exact words + instead of washing the line. +- **A defined form below a hundred columns**, where word-level emphasis gives way + to the line — a bolded fragment in the middle of a line that now takes three + rows is harder to find than a whole row that is red. Nothing is truncated. +- **`diff = "minimal"` in `[app.io-cli]`** for reviewing by file rather than by + hunk: the changed lines and the `@@` header, without the context. Its absence + means `unified`, so no existing configuration file needs touching. + +### Changed + +- **An approval shows a write as a diff** against the file on disk, which is the + clause of the approval surface 0.2.0 shipped unmet — the harness hands an + approver the whole resulting file rather than a patch, so the old side is + io-cli's to supply. A file that does not exist yet reads as all addition. At + the tightest size the one row available is spent on `+3 -1` rather than on the + first line of the change, because the size of a write is the decision. + +### Fixed + +- The answers row in the approval overlay no longer carries a double space after + each separator. + ## [0.2.0] - 2026-08-17 The operator can see the boundary the agent is working under, change it, and diff --git a/Cargo.lock b/Cargo.lock index b18ea17..76f7515 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,12 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "adler2" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" + [[package]] name = "aho-corasick" version = "1.1.5" @@ -123,6 +129,30 @@ version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "2.13.1" @@ -307,6 +337,15 @@ dependencies = [ "libc", ] +[[package]] +name = "crc32fast" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9481c1c90cbf2ac953f07c8d4a58aa3945c425b7185c9154d67a65e4230da511" +dependencies = [ + "cfg-if", +] + [[package]] name = "crossterm" version = "0.28.1" @@ -417,6 +456,17 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" +[[package]] +name = "fancy-regex" +version = "0.16.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "998b056554fbe42e03ae0e152895cd1a7e1002aec800fdc6635d20270260c46f" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fastrand" version = "2.5.0" @@ -429,6 +479,22 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +[[package]] +name = "flate2" +version = "1.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" +dependencies = [ + "crc32fast", + "miniz_oxide", +] + +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "foldhash" version = "0.1.5" @@ -880,13 +946,14 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.2.0" +version = "0.3.0" dependencies = [ "clap", "crossterm", "io-harness", "ratatui", "serde", + "syntect", "tempfile", "tokio", "toml", @@ -1083,6 +1150,16 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "miniz_oxide" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fa76a2c86f704bdb222d66965fb3d63269ce38518b83cb0575fca855ebb6316" +dependencies = [ + "adler2", + "simd-adler32", +] + [[package]] name = "mio" version = "1.2.2" @@ -1743,6 +1820,12 @@ dependencies = [ "libc", ] +[[package]] +name = "simd-adler32" +version = "0.3.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a219298ac11a56ea9a6d2120044824d6f01aeb034955e7af7bc16858527deea" + [[package]] name = "simd_cesu8" version = "1.2.0" @@ -1894,6 +1977,24 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "syntect" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "656b45c05d95a5704399aeef6bd0ddec7b2b3531b7c9e900abbf7c4d2190c925" +dependencies = [ + "bincode", + "fancy-regex", + "flate2", + "fnv", + "once_cell", + "regex-syntax", + "serde", + "serde_derive", + "thiserror", + "walkdir", +] + [[package]] name = "tempfile" version = "3.27.0" diff --git a/Cargo.toml b/Cargo.toml index 27fb403..a33ede4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.2.0" +version = "0.3.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 @@ -50,6 +50,27 @@ clap = { version = "4", features = ["derive"] } tokio = { version = "1", features = ["rt-multi-thread", "macros", "sync", "time"] } serde = { version = "1", features = ["derive"] } toml = "1" +# Syntax highlighting inside a diff, and the feature set is the load-bearing part +# rather than the version. +# +# `default-features = false` excludes `regex-onig`, which is oniguruma — a C +# library. Two of this product's four artifacts are cross-compiled, to +# `x86_64-unknown-linux-musl` and `x86_64-pc-windows-msvc`, and a native build +# step in that path fails inside the release workflow rather than in a test. +# `regex-fancy` is the pure-Rust engine that replaces it. +# +# `default-themes` is off on purpose: the colours come from io-cli's own token +# set, so a highlighted diff and the rest of the interface are one aesthetic +# instead of two, and `NO_COLOR` keeps working because there is only one place +# that decides colour. `html`, `plist-load` and `yaml-load` are for loading +# grammars and emitting markup at runtime, which this product never does. +# +# `tests/dependencies.rs` asserts this exact feature list. A later +# `default-features = true` would pull the C engine back in silently. +syntect = { version = "5", default-features = false, features = [ + "default-syntaxes", + "regex-fancy", +] } [dev-dependencies] tempfile = "3" diff --git a/README.md b/README.md index e53f460..07ab083 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,7 @@ writes no key to disk at all. | `Ctrl+D` | exit, on an empty prompt | | `Shift+Tab` | cycle the permission posture, from the next turn | | `Ctrl+L` | clear the viewport, never the scrollback | +| `Ctrl+T` | put the whole conversation back into the scrollback | | `y / a / n` | answer an approval: allow once, allow this session, deny | | `Esc` | close a picker without choosing | @@ -110,9 +111,26 @@ writes no key to disk at all. | `/setup` | run the first-run wizard again | | `/theme` | change the theme for this session | | `/model` | change the model for this session | +| `/expand` | commit the last step's full detail into the scrollback | +| `/copy` | put the last answer on the system clipboard | +| `/copy diff` | put the whole run's patch on the system clipboard | +**Everything that shows you more of something writes it into the terminal's own +scrollback.** `Ctrl+T` and `/expand` do not open a pane: the viewport is a few +rows and this product has no alternate screen, so the place to read something +long is the buffer where the terminal's search, selection and tmux copy-mode +already work. `/expand` reads the step's full output back out of the run's +durable trace, which is where it went in the first place — the screen is not the +archive. + +`/copy` uses OSC 52, so it reaches the clipboard of the machine you are *sitting +at* rather than the one you are ssh'd into. Nothing acknowledges an OSC 52 write: +the line it prints says what was sent and how large it was, never that it +succeeded. Inside tmux it needs `set -g set-clipboard on`, and some terminals +refuse a large payload without saying so. + `/theme` and `/model` change this session only and say so. Making a choice permanent is `io setup`. @@ -122,6 +140,12 @@ io-cli has no configuration parser. io-harness owns discovery and layering, and io-cli's own settings live in the `[app.io-cli]` section that io-harness deliberately does not validate. See [`docs/config.example.toml`](docs/config.example.toml). +Two keys live there: `theme`, and `diff`, which is `unified` (the default, and +what an absent key means) or `minimal` — the changed lines and the `@@` header +without the context, for reviewing by file rather than by hunk. Because the +section is unvalidated by design, an unrecognised value reads as the default +rather than stopping a session from starting. + The file is found in this order: `$IO_CONFIG`, else `$IO_CONFIG_HOME/io.toml`, else `$XDG_CONFIG_HOME/io/io.toml` or `~/.config/io/io.toml`, and `%APPDATA%\io\io.toml` on Windows. A project's own `io.toml` and a gitignored @@ -151,21 +175,29 @@ refusal, error and warning also carries a word. ## What this release is not -0.2.0 is the permission boundary made visible and answerable: the approval -overlay, the posture key, the refusal that names its rule and its layer, and the -status fields around them. Diffs, syntax highlighting and collapsible tool output -are 0.3.0; resume, fork and rewind are 0.4.0; the headless subcommand and NDJSON -are 0.5.0; the screen-reader mode is 0.6.0; the slash palette and type-to-filter -pickers are 0.7.0. +0.3.0 is what the agent did to your files, made readable: diffs from the +harness's own stored hunks, word-level emphasis, syntax highlighting, collapsed +tool output, the transcript on one key and the clipboard over SSH. Resume, fork +and rewind are 0.4.0; the headless subcommand and NDJSON are 0.5.0; the +screen-reader mode is 0.6.0; the slash palette and type-to-filter pickers are +0.7.0. -Two things are absent for reasons worth stating rather than hiding. **Spend +Three things are absent for reasons worth stating rather than hiding. **Spend against the tree ceiling is not in the status line**: io-harness emits `SpendDraw` only from a contained turn, and its contained entry point takes no steer inbox, so rendering spend today would cost `Ctrl+C`. It arrives at 0.8.0 -with the fleet, which is contained anyway. And **the overlay shows a write's -proposed content plainly rather than as a diff** — the harness hands an approver -the resulting file, not a patch, so the diff is io-cli's to compute and 0.3.0's -to design. +with the fleet, which is contained anyway. **A diff cannot be expanded beyond +the context the harness stored** — three lines either side, which is what +`diff -u` has always carried; more than that is not in the trace, and reading it +off disk would be reading a version of the file that no longer exists. And +**there is no split view**: this renderer commits into the terminal's own +scrollback at its real width, a two-column comparison doubles the horizontal +budget for every line, and word-level emphasis inside a unified diff already +answers the question split view answers. + +One ceiling worth knowing about: a hunk is a fragment of a file, and each of its +lines is highlighted from a clean parse. A block comment or a multi-line string +that was opened *above* the hunk is not known here, so those lines read as code. ## Contributing diff --git a/docs/config.example.toml b/docs/config.example.toml index d6f77ad..021822a 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -91,3 +91,11 @@ net = "deny" [app.io-cli] # "dark" or "light". Absent means detect from the terminal background. theme = "dark" +# How much of a change a diff shows. +# +# "unified" the hunk as the harness stored it, context and all. The default, +# and what an absent key means — so a file written before 0.3.0 +# needs no change. +# "minimal" the changed lines and the @@ header, without the context. For +# reviewing by file rather than by hunk. +diff = "unified" diff --git a/src/app.rs b/src/app.rs index 5b1de26..76f7de1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -52,6 +52,11 @@ pub enum Command { Exit, /// Repaint the viewport from scratch, never the scrollback. ClearViewport, + /// Put the whole conversation back into the terminal's own scrollback. + /// + /// A command rather than something this type does, because the transcript + /// lives in the harness's store and the store belongs to the driver. + Transcript, } pub struct App { @@ -77,6 +82,15 @@ pub struct App { /// without this a *this session* answer would ask again on the next prompt. /// F5 asserts it on the policy handed to the next turn. remembered: Vec, + /// The workspace this session is held over. + /// + /// Kept so an approval can resolve a write's target: the harness sends it + /// relative to the workspace, and the process's working directory is not the + /// same thing under `io -C `. + root: std::path::PathBuf, + /// How much of a change a diff shows. From `[app.io-cli]`, defaulting to + /// unified, which is what every file written before 0.3.0 means. + diff_style: crate::settings::DiffStyle, /// The permission posture this session is running under. /// /// `None` means the configuration file holds a policy that is none of the @@ -98,17 +112,30 @@ impl App { pending: Vec::new(), approval: None, remembered: Vec::new(), + root: std::path::PathBuf::new(), + diff_style: crate::settings::DiffStyle::default(), posture: None, } } + /// Say which workspace this session is held over. + pub fn set_root(&mut self, root: impl Into) { + self.root = root.into(); + } + + /// Say how much of a change a diff should show. Read from the harness's own + /// configuration by the driver; never parsed here. + pub fn set_diff_style(&mut self, style: crate::settings::DiffStyle) { + self.diff_style = style; + } + /// A run stopped to ask. The overlay opens and takes the keyboard. /// /// Nothing is committed here. A question in the scrollback is one that can be /// scrolled away from a run which is blocked on it, which is what F1 asserts /// and the reason this surface is an overlay at all. pub fn open_approval(&mut self, ask: Ask) { - self.approval = Some(Approval::new(ask)); + self.approval = Some(Approval::new(ask, &self.root)); } /// The posture in force. @@ -227,12 +254,31 @@ impl App { } /// Take an event from the harness. - pub fn event(&mut self, event: &RunEvent) { + /// + /// `at` is the session's age, handed in by the driver rather than read here. + /// It is what lets a tool cell report how long its call took without any + /// module but `src/main.rs` touching a clock — the same shape [`App::tick`] + /// established, and the reason `tests/timing.rs` can still assert that no + /// test anywhere reads one. + pub fn event(&mut self, event: &RunEvent, at: Duration) { self.status_from(event); - let lines = self.events.event(event); + let lines = self.events.event(event, at); self.pending.extend(lines); } + /// What a step changed, as diffs. + /// + /// Handed in by the driver rather than read here. The store belongs to the + /// driver — it is the only thing that holds one — and keeping this a function + /// of values is what lets a test state a hunk by hand instead of standing up + /// a database to hold one. + pub fn edits(&mut self, edits: &[io_harness::Edit], width: u16) { + for edit in edits { + self.pending + .extend(crate::diff::cell(edit, &self.theme, width)); + } + } + /// The status line's share of an event. /// /// Only the events that carry a fact set a field, and nothing sets one to a @@ -316,6 +362,11 @@ impl App { // destroy the transcript, which on this renderer is the terminal's own // buffer rather than something this process can redraw. (KeyCode::Char('l'), true) => Command::ClearViewport, + // Upward, never into a pane. The conversation is already in the + // terminal's scrollback; this puts back the part that has scrolled + // out of reach, branched-away turns included, where the terminal's + // own search and copy-mode already work on it. + (KeyCode::Char('t'), true) => Command::Transcript, _ => { self.quits = 0; match self.composer.key(key) { diff --git a/src/approval.rs b/src/approval.rs index 2b451d9..69b3dd6 100644 --- a/src/approval.rs +++ b/src/approval.rs @@ -25,7 +25,7 @@ use crossterm::event::{KeyCode, KeyEvent}; use io_harness::approve::DecisionFuture; -use io_harness::{Act, ApprovalContext, Approver, Decision, Effect, Request, Rule}; +use io_harness::{Act, ApprovalContext, Approver, Decision, Edit, Effect, Request, Rule}; use ratatui::layout::Rect; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; @@ -71,9 +71,12 @@ impl Ask { &self.request.target } - /// What a write would leave behind, whole. The harness hands an approver the - /// resulting file rather than a patch, so anything diff-shaped is this - /// product's to compute — and is 0.3.0's, not this release's. + /// What a write would leave behind, whole. + /// + /// The harness hands an approver the resulting file rather than a patch, so + /// the old side has to come from somewhere else — 0.3.0 reads it off disk and + /// hands both to `Edit::with_hunk`, which is the harness's own renderer. See + /// [`diff_of`]. pub fn content(&self) -> Option<&str> { self.request.content.as_deref() } @@ -219,14 +222,39 @@ pub const REFUSED_BY_OPERATOR: &str = "the operator denied it"; pub struct Approval { ask: Ask, chosen: usize, + /// The write, as a change rather than as a wall of text. + /// + /// Computed once, when the overlay opens, because `render` runs per frame and + /// reading a file per frame is a file read per keystroke. `None` means there + /// is nothing to diff — the act is not a write, or the target could not be + /// read for a reason that is not "it does not exist yet" — and the overlay + /// falls back to showing the proposed content plainly, which is what 0.2.0 + /// did for every write. + proposed: Option, } impl Approval { - pub fn new(ask: Ask) -> Self { + /// Open an overlay for a question, resolving a write's target against + /// `root`. + /// + /// The root is required rather than inferred, and a live run is what settled + /// that. `Request.target` arrives **relative to the workspace** — `notes.txt`, + /// not `/tmp/x/notes.txt` — so an implementation that only read absolute + /// paths never found the file and quietly fell back to showing the proposed + /// content, which is a feature that ships looking like it works. Resolving + /// against this process's working directory instead would be worse: `io -C + /// ` sets the workspace without changing it, so a relative name could + /// match a different file that happens to exist here. + pub fn new(ask: Ask, root: &std::path::Path) -> Self { + let proposed = diff_of(&ask, root); // Opens on `Once`: the least committal answer, and never on a remembered // allow. A surface that opens on the widest answer is one where `Enter` // by reflex gives away the most. - Self { ask, chosen: 0 } + Self { + ask, + chosen: 0, + proposed, + } } /// Which answer is highlighted. @@ -366,6 +394,9 @@ impl Approval { /// It says what it cut. A reader who thinks they have seen a whole file and /// has seen a third of it is worse off than one who was told. fn preview(&self, room: usize, width: usize, theme: &Theme) -> Vec> { + if let Some(edit) = &self.proposed { + return self.as_diff(edit, room, width, theme); + } let Some(content) = self.ask.content() else { return Vec::new(); }; @@ -395,6 +426,68 @@ impl Approval { .collect() } + /// The write as a diff, in the rows there are. + /// + /// **The counts come first and the path does not come at all.** The + /// transcript's version of this cell leads with the path, because there a diff + /// arrives with nothing above it. Here the question row directly above already + /// names the target, and repeating it cost a whole assertion: at eighty + /// columns with a long path, ratatui clipped the row — silently, as it always + /// does — and what went was `+2 -0`, the one fact worth having when there is + /// only one row. That is 0.2.0's lesson arriving a second time in the same + /// overlay, and the answer is the same: put the load-bearing fact where it + /// cannot be the part that goes. + /// + /// At the tightest size that one row is `+3 -1`, which is the difference + /// between a write that touches three lines and one that rewrites four + /// hundred — a different decision, not a smaller one. + fn as_diff(&self, edit: &Edit, room: usize, width: usize, theme: &Theme) -> Vec> { + let mut lines = vec![Line::from(vec![ + Span::styled(format!(" +{}", edit.lines_added), theme.style(Tone::Added)), + Span::styled(" ".to_string(), theme.style(Tone::Muted)), + Span::styled( + format!("-{}", edit.lines_removed), + theme.style(Tone::Removed), + ), + Span::styled( + match &edit.hunk { + Some(_) => format!("{SEPARATOR}{}", edit.tool), + // Absent is a fact and not an empty diff — the counts beside + // it are what say the file did change. + None => format!("{SEPARATOR}{}{SEPARATOR}no diff stored", edit.tool), + }, + theme.style(Tone::Muted), + ), + ])]; + + // Past the cell's own header, which the row above replaces, and without + // the blank line it ends with: an overlay four rows tall cannot spend one + // on breathing. Every line is fitted, because this viewport clips. + lines.extend( + crate::diff::cell(edit, theme, u16::try_from(width).unwrap_or(u16::MAX)) + .into_iter() + .skip(1) + .filter(|line| line.width() > 0) + .map(|line| fit_line(line, width, theme)), + ); + + let total = lines.len(); + let shown = total.min(room); + let cut = total.saturating_sub(shown); + lines.truncate(shown); + // Said, not silently dropped. A reader who thinks they have seen a whole + // change and has seen its first row is worse off than one who was told. + if cut > 0 { + if let Some(last) = lines.last_mut() { + last.spans.push(Span::styled( + format!(" ⋯ {cut} more lines"), + theme.style(Tone::Muted), + )); + } + } + lines + } + /// `› allow once · allow this session · deny`, on one row. /// /// One row rather than a list, because the viewport is four rows and the facts @@ -424,6 +517,83 @@ impl Approval { /// The word for an act. io-harness spells these in its own serde tags and in /// `EventKind::Refused`; this is the same vocabulary so a refusal and a request /// read as the same product. +/// The change a write would make, as an `io_harness::Edit`. +/// +/// **io-cli still computes no diff.** `Edit::with_hunk` is the harness's own +/// renderer — the same one that produced every hunk in the durable trace — so an +/// approval and a transcript show a change in exactly the same words. What io-cli +/// supplies is the *old* side, because the write has not happened yet and the +/// harness has nothing stored for it: the approver is handed the whole resulting +/// file, never a patch. +/// +/// **Only the request's own target is read**, and only for a write. This is the +/// one workspace read the interface performs, it is of the file the operator is +/// being asked about, and it exists so they can see what changes rather than what +/// the file will end up containing. +/// +/// The three outcomes are deliberately different: +/// - the file is not there → an empty old side, so a new file reads as all +/// addition, which is what it is; +/// - the file is there → a real diff against it; +/// - the file is there and cannot be read, or is not UTF-8 → `None`, and the +/// overlay shows the proposed content plainly. Diffing against an empty old +/// side here would show every existing line as new, which is a lie about the +/// size of the change and exactly the wrong direction to be wrong in. +fn diff_of(ask: &Ask, root: &std::path::Path) -> Option { + if ask.act() != Act::Write { + return None; + } + let after = ask.content()?; + let target = ask.target(); + + // `join` with an absolute target returns the target, so one line covers both + // shapes — and the relative one is what a real run actually sends. + let path = root.join(target); + + let before = match std::fs::read_to_string(&path) { + Ok(before) => before, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(_) => return None, + }; + + Some(Edit::measure(0, "write_file", target, &before, after).with_hunk(&before, after)) +} + +/// Cut a rendered line to `width`, keeping its styles and saying it was cut. +/// +/// The overlay draws into a fixed-height viewport, where ratatui **clips** a row +/// that does not fit rather than wrapping it — and clips it silently. A diff line +/// is the widest thing this surface draws, so without this a hunk at eighty +/// columns loses its right-hand end with nothing on screen to say so. +/// +/// Spans are kept whole while they fit and the one that crosses the edge is cut, +/// so syntax colour survives as far as the line does. +fn fit_line(line: Line<'static>, width: usize, theme: &Theme) -> Line<'static> { + if line.width() <= width { + return line; + } + // One cell for the marker that says something went. + let room = width.saturating_sub(1); + let mut spans: Vec> = Vec::new(); + let mut used = 0; + for span in line.spans { + let length = span.content.chars().count(); + if used + length <= room { + used += length; + spans.push(span); + continue; + } + let take = room - used; + if take > 0 { + let cut: String = span.content.chars().take(take).collect(); + spans.push(Span::styled(cut, span.style)); + } + break; + } + spans.push(Span::styled("…".to_string(), theme.style(Tone::Muted))); + Line::from(spans) +} + pub fn act_word(act: Act) -> &'static str { match act { Act::Read => "read", diff --git a/src/clipboard.rs b/src/clipboard.rs new file mode 100644 index 0000000..77d182f --- /dev/null +++ b/src/clipboard.rs @@ -0,0 +1,124 @@ +//! Putting text on the clipboard of the terminal a session is actually being +//! watched on, which over SSH is not this machine. +//! +//! io-cli is used through an SSH connection and inside tmux. A local clipboard +//! call — pasteboard, X11, Wayland — sets the clipboard of the host the agent is +//! running on, which is the wrong computer: the reader is at the far end of the +//! connection, and the text they asked for would land somewhere they cannot +//! reach. OSC 52 is the one mechanism that goes the other way. It is an escape +//! sequence, so it travels back up the same stream the transcript does and is +//! interpreted by the terminal emulator in front of the reader. +//! +//! # Nothing ever answers +//! +//! A terminal does not reply to an OSC 52 write. There is no acknowledgement, no +//! error and no refusal on the wire — the sequence is written and that is the end +//! of what this process can observe. Meanwhile the two most common deployments +//! both drop it quietly: tmux discards the sequence unless the user has set +//! `set -g set-clipboard on`, and most terminals silently ignore a payload past +//! some internal ceiling they do not publish. +//! +//! So a `copied` message here would be a claim io-cli cannot support, printed at +//! exactly the moment the reader goes to paste and finds the old contents. That +//! is why this module hands back [`describe`] and not a `bool`: there is no +//! success value to return, and the shape of the API is what keeps a call site +//! from inventing one. What can honestly be said is what was sent and how big it +//! was, which is also the useful thing — a reader whose paste comes up empty can +//! compare the size against their terminal's limit and their tmux setting. +//! +//! # The size ceiling +//! +//! io-cli sends the payload whatever its size. The cap is the terminal's, it +//! differs between emulators, and nothing on this side can query it, so refusing +//! to send at some invented threshold would fail writes that would have worked. +//! Sending and reporting the size leaves the reader holding the one fact that +//! makes the failure diagnosable. Note that the sequence on the wire is about a +//! third larger than the payload, because base64 spends four characters on every +//! three bytes, and it is the encoded form the terminal measures. +//! +//! Nothing here wraps the sequence for tmux's DCS passthrough. Passthrough has to +//! be applied only when actually inside tmux, and the sequence sent to a terminal +//! that is not tmux is garbage printed into the transcript; `$TMUX` is not a +//! reliable enough signal to bet the transcript on, and `set-clipboard on` is a +//! setting the user can make once and keep. + +/// Standard base64, the `+/` alphabet of RFC 4648 — not the URL-safe variant. +/// OSC 52 payloads are read by terminal emulators that implement the original, +/// and a `-_` payload decodes to different bytes rather than failing loudly. +const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +/// The full sequence to write to the terminal to put `payload` on the system +/// clipboard: `ESC ] 52 ; c ; BEL`. +/// +/// Selection `c` is the system clipboard, the one a paste reads. The other +/// selections OSC 52 can address — the X11 primary and the cut buffers — are not +/// where a reader who pressed the copy key expects to find their text. +/// +/// The encoding is not decoration. An OSC string ends at the first BEL or ST, so +/// a payload carrying either byte — and a transcript can carry both, since tool +/// output is arbitrary — would terminate the sequence early and leave the rest of +/// itself printed on screen as text. Base64 removes every byte that could do +/// that, which is why the sequence is built here rather than by a caller. +/// +/// Terminated with BEL rather than ST because the whole xterm lineage and tmux +/// accept BEL, while a handful of terminals only ever learned that form. +pub fn sequence(payload: &str) -> String { + format!("\x1b]52;c;{}\x07", encode(payload.as_bytes())) +} + +/// Base64-encode `bytes`, with `=` padding. +/// +/// Hand-written because it is twenty lines against a dependency, and this crate's +/// dependency list is asserted by `tests/dependencies.rs` — the list is the claim +/// that io-cli contains no agent machinery of its own, and it is worth more than +/// the twenty lines cost. +pub fn encode(bytes: &[u8]) -> String { + let mut encoded = String::with_capacity(bytes.len().div_ceil(3) * 4); + for chunk in bytes.chunks(3) { + // The missing bytes of a short final chunk encode as zero bits, and the + // characters that carry only those bits become padding below. + let second = chunk.get(1).copied().unwrap_or(0); + let third = chunk.get(2).copied().unwrap_or(0); + let group = (u32::from(chunk[0]) << 16) | (u32::from(second) << 8) | u32::from(third); + for position in 0..4 { + // One input byte fills two output characters, two fill three, three + // fill four; everything past that is `=`. + if position <= chunk.len() { + let index = (group >> (18 - position * 6)) & 0b11_1111; + encoded.push(ALPHABET[index as usize] as char); + } else { + encoded.push('='); + } + } + } + encoded +} + +/// What can truthfully be said after writing [`sequence`]: `sent 4.2 kB to the +/// terminal clipboard`. +/// +/// Deliberately not a past-tense claim about the clipboard's contents. See the +/// module documentation — no terminal answers an OSC 52 write, so `copied` is a +/// statement about something this process never observes. This wording says only +/// what io-cli did, which is the part it knows. +/// +/// The size is the payload's, in bytes rather than characters, because bytes are +/// what a terminal's limit is expressed in and a line of CJK text is three times +/// its own length on the wire. +pub fn describe(payload: &str) -> String { + format!( + "sent {} to the terminal clipboard", + format_bytes(payload.len()) + ) +} + +/// `1 byte`, `840 bytes`, `4.2 kB`. Decimal kilobytes, matching the `kB` it is +/// spelled with: the number exists to be held against a terminal's documented +/// limit, and those limits are quoted in round decimal thousands. +fn format_bytes(bytes: usize) -> String { + match bytes { + 1 => "1 byte".to_string(), + 0..=999 => format!("{bytes} bytes"), + _ => format!("{:.1} kB", bytes as f64 / 1000.0), + } +} diff --git a/src/commands.rs b/src/commands.rs index ea224b7..cf3064b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1,9 +1,16 @@ -//! The five slash commands, and the keybinding table they document. +//! The slash commands, and the keybinding table they document. //! -//! Five, and each one is a [`Picker`](crate::picker::Picker) or a print. The -//! fuzzy palette that reaches every command and every harness skill is 0.7.0's; -//! what matters now is that `/setup` exists, because it is what makes the wizard -//! reachable after the first run. +//! Each one is a [`Picker`](crate::picker::Picker), a print, or something +//! committed into the terminal's own scrollback. The fuzzy palette that reaches +//! every command and every harness skill is 0.7.0's; what matters now is that +//! `/setup` exists, because it is what makes the wizard reachable after the +//! first run. +//! +//! **Everything that shows more of something commits upward.** The viewport is +//! four rows and cannot grow, so `/expand` and `Ctrl+T` do not open a pane — they +//! write into the scrollback, where the terminal's own search, selection and +//! copy-mode already work. That is one answer to "show me more" rather than +//! three, and it is the same answer the transcript gives. use ratatui::text::{Line, Span}; @@ -30,6 +37,10 @@ pub const KEYS: &[(&str, &str)] = &[ "cycle the permission posture, from the next turn", ), ("Ctrl+L", "clear the viewport, never the scrollback"), + ( + "Ctrl+T", + "put the whole conversation back into the scrollback", + ), ( "y / a / n", "answer an approval: allow once, allow this session, deny", @@ -44,6 +55,15 @@ pub const COMMANDS: &[(&str, &str)] = &[ ("/setup", "run the first-run wizard again"), ("/theme", "change the theme for this session"), ("/model", "change the model for this session"), + ( + "/expand", + "commit the last step's full detail into the scrollback", + ), + ("/copy", "put the last answer on the system clipboard"), + ( + "/copy diff", + "put the whole run's patch on the system clipboard", + ), ]; /// What the driver should do about a slash command. @@ -57,6 +77,24 @@ pub enum Action { Theme, /// Open the model picker. Model, + /// Commit the last step's stored detail into the scrollback. + /// + /// The detail is in the run's durable trace already — this reads it back + /// rather than the screen having been the archive. + Expand, + /// Put something on the system clipboard over OSC 52. + Copy(Copied), + /// Put the whole conversation back into the scrollback. + Transcript, +} + +/// What `/copy` was asked for. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Copied { + /// The last thing the agent said. + Answer, + /// Every change the run made, as one patch. + Diff, } /// Resolve a command. The leading `/` has already been removed. @@ -70,6 +108,13 @@ pub fn parse(input: &str, theme: &Theme) -> Action { "setup" => Action::Setup, "theme" => Action::Theme, "model" => Action::Model, + "expand" => Action::Expand, + "copy" => match input.split_whitespace().nth(1) { + // `/copy diff` and `/copy patch` mean the same thing. A reader who + // has just been shown a diff will type the word they were shown. + Some("diff") | Some("patch") => Action::Copy(Copied::Diff), + _ => Action::Copy(Copied::Answer), + }, unknown => { let mut lines = vec![theme.notice( Tone::Warning, diff --git a/src/diff.rs b/src/diff.rs new file mode 100644 index 0000000..198f758 --- /dev/null +++ b/src/diff.rs @@ -0,0 +1,508 @@ +//! Rendering an edit the agent made. +//! +//! **io-cli computes no diff.** io-harness renders a unified diff for every edit +//! its tools make and keeps it in the run's durable trace, so what this module +//! draws is text that already exists — `io_harness::Edit::hunk`, a hunk body +//! whose `@@` line numbers are the *file's* rather than the fragment's. That is +//! the difference between a diff this product can show and one it could only +//! approximate: io-cli never sees the file, so it could not number a hunk +//! correctly even if it wanted to compute one. +//! +//! Three decisions shape everything here. +//! +//! **The hunk is passed through, never reconstructed.** Its markers, its +//! spacing and its header are the harness's; this module decides colour and +//! nothing else. A reader who copies a rendered line into `patch` gets what +//! `patch` expects. +//! +//! **An absent hunk is a fact, not an empty one.** `Edit::hunk` is `None` when +//! the row predates the harness release that added hunks, when the file's +//! previous contents were not kept (over the store's snapshot cap, or not +//! UTF-8), or when the rendered diff would itself have exceeded that cap. Not +//! one of those is "nothing changed" — and `lines_added` and `lines_removed` +//! are still there to prove it — so the cell says the counts and says the diff +//! is missing, rather than drawing an empty body that reads as an untouched +//! file. +//! +//! **Green means added, and it never also means "this is a string".** The +//! obvious way to combine syntax highlighting with a diff is to colour the whole +//! changed line green or red, which leaves no colour for the code, or to syntax +//! colour the whole line, which leaves no colour for the change. Neither is +//! taken. The marker keeps the diff's colour, the parts of a line that both +//! sides share are syntax coloured, and **the words that actually changed are +//! drawn in the diff's colour and emphasised** — so the add/remove colour is +//! still on screen and is now pointing at the exact words rather than washing +//! the line. A line with no partner to compare against gets the whole wash, +//! because on that line everything did change. + +use std::ops::Range; +use std::sync::OnceLock; + +use io_harness::Edit; +use ratatui::text::{Line, Span}; +use syntect::easy::ScopeRangeIterator; +use syntect::parsing::{ParseState, Scope, ScopeStack, SyntaxReference, SyntaxSet}; + +use crate::settings::DiffStyle; +use crate::status::SEPARATOR; +use crate::theme::{Theme, Tone}; + +/// What every line of the cell is indented by, so a hunk sits under its header +/// the way a tool call sits under a step. +const INDENT: &str = " "; + +/// The most body lines one cell will draw. +/// +/// A hunk is capped by the harness at a mebibyte, which is on the order of +/// twenty-six thousand lines, and two separate things go wrong long before that. +/// Highlighting is a fresh parse per line and runs synchronously on the loop that +/// also delivers keystrokes, so a large hunk makes `Ctrl+C` unreachable for as +/// long as it takes. And `Screen::commit` hands `insert_before` a `u16` height, +/// which silently clamps: past sixty-five thousand rows the content is truncated +/// rather than wrapped, and the buffer it allocates first is the terminal's width +/// times that height. +/// +/// Five hundred is far more of a change than anybody reads in a terminal and far +/// less than either failure. What is cut is said, never dropped quietly — the +/// whole change is still in the trace, and `/copy diff` still carries all of it. +pub const MAX_BODY_LINES: usize = 500; + +/// The width below which word-level emphasis drops to the line. +/// +/// A hunk line is committed into the terminal's own scrollback, where a line +/// wider than the terminal wraps rather than truncating — nothing is lost. What +/// is lost is *findability*: a bolded fragment in the middle of a line that now +/// occupies three rows is harder to locate than a whole row that is red. So +/// below a hundred columns the emphasis is the line, which is the same +/// information at a scale the terminal can still show. +/// +/// A hundred rather than eighty because the floor is about the wrap, not about +/// the supported size: an eighty-column terminal is supported, and a +/// ninety-column one wraps just as badly. +pub const EMPHASIS_FLOOR: u16 = 100; + +/// The syntax highlighter, built once and only if a diff is ever drawn. +/// +/// **Never on the startup path.** `SyntaxSet::load_defaults_newlines` +/// decompresses a dump of every grammar syntect ships, which is real work and +/// real memory, and this product's own bar is a first paint inside a hundred +/// milliseconds. A session that never edits a file never pays for it — which is +/// most sessions, since a question does not touch the workspace. +/// +/// A `OnceLock` rather than a field on anything: the set is immutable once +/// built, every diff in the session shares it, and threading it through [`cell`] +/// would put a parameter on a function whose callers have no business knowing a +/// highlighter exists. +static HIGHLIGHTER: OnceLock = OnceLock::new(); + +/// The grammar set, plus the four scopes worth colouring. +struct Highlighter { + syntaxes: SyntaxSet, + /// Matched by prefix, so `keyword` catches `keyword.control.rust` and every + /// other language's spelling of the same idea without a table per language. + keyword: Scope, + /// `storage`, and it is not redundant with `keyword`. Sublime's scope + /// vocabulary — which syntect's grammars are written in — files `let`, + /// `const`, `fn`, `struct` and `pub` under `storage.type` and + /// `storage.modifier`, leaving `keyword` for control flow and operators. A + /// table without this entry highlights `=` and misses every declaration on + /// the line, which is the opposite of what a reader wants. Found by a test, + /// not by reading the vocabulary. + storage: Scope, + string: Scope, + comment: Scope, + constant: Scope, +} + +impl Highlighter { + fn get() -> &'static Self { + HIGHLIGHTER.get_or_init(|| Self { + syntaxes: SyntaxSet::load_defaults_newlines(), + // `Scope::new` fails only on a malformed selector and these four are + // literals in this file. The fallback is the empty scope, which is a + // prefix of everything — so the impossible case colours a line + // wrongly rather than panicking inside a renderer. + keyword: Scope::new("keyword").unwrap_or_default(), + storage: Scope::new("storage").unwrap_or_default(), + string: Scope::new("string").unwrap_or_default(), + comment: Scope::new("comment").unwrap_or_default(), + constant: Scope::new("constant").unwrap_or_default(), + }) + } + + /// The tone for the scope stack a token was parsed under. + /// + /// Read from the top of the stack downwards, because the innermost scope is + /// the specific one: a keyword inside a string is a string. + fn tone(&self, stack: &ScopeStack) -> Tone { + for scope in stack.as_slice().iter().rev() { + if self.comment.is_prefix_of(*scope) { + return Tone::Muted; + } + if self.string.is_prefix_of(*scope) { + return Tone::StringLiteral; + } + if self.constant.is_prefix_of(*scope) { + return Tone::Literal; + } + if self.keyword.is_prefix_of(*scope) || self.storage.is_prefix_of(*scope) { + return Tone::Keyword; + } + } + Tone::Normal + } + + /// `text` split into byte runs that share a tone. + /// + /// **A hunk is a fragment of a file, and this parses each line from a clean + /// state.** A block comment or a multi-line string opened above the hunk is + /// therefore not known here and its lines read as code. Carrying state would + /// mean two parsers — one walking the old side of the diff and one the new — + /// fed an interleaved stream, for at most seven lines of context. The + /// ceiling is worth naming and is not worth the machinery. + fn runs(&self, text: &str, syntax: &SyntaxReference) -> Vec<(Range, Tone)> { + // The grammars are the newline-terminated set, so a line without its + // terminator can leave a rule unmatched at the end. + let terminated = format!("{text}\n"); + let mut state = ParseState::new(syntax); + let Ok(ops) = state.parse_line(&terminated, &self.syntaxes) else { + return vec![(0..text.len(), Tone::Normal)]; + }; + + let mut stack = ScopeStack::new(); + let mut runs: Vec<(Range, Tone)> = Vec::new(); + let mut at = 0; + for (range, op) in ScopeRangeIterator::new(&ops, &terminated) { + if stack.apply(op).is_err() { + return vec![(0..text.len(), Tone::Normal)]; + } + // Clamped: the newline added above is not part of the caller's text. + let end = range.end.min(text.len()); + if end <= at { + continue; + } + let tone = self.tone(&stack); + match runs.last_mut() { + Some((last, seen)) if *seen == tone => last.end = end, + _ => runs.push((at..end, tone)), + } + at = end; + } + if at < text.len() { + runs.push((at..text.len(), Tone::Normal)); + } + if runs.is_empty() { + runs.push((0..text.len(), Tone::Normal)); + } + runs + } +} + +/// The grammar for a path, by extension, or `None` when nothing claims it. +/// +/// `None` is not a failure. A diff of a file syntect has no grammar for renders +/// in the terminal's own foreground, which is what it should look like. +fn syntax_for(path: &str) -> Option<&'static SyntaxReference> { + let extension = std::path::Path::new(path).extension()?.to_str()?; + Highlighter::get() + .syntaxes + .find_syntax_by_extension(extension) +} + +/// The edits a given step made, out of everything the run recorded. +/// +/// `Store::edits` answers for the whole run, so a caller that draws what it +/// returns re-renders every earlier edit at every later step — the transcript +/// grows quadratically and the same diff appears four times. Kept here rather +/// than in the driver so that it is reachable from a test; the driver is a +/// binary and an integration test cannot link one. +pub fn for_step(edits: Vec, step: u32) -> Vec { + edits.into_iter().filter(|edit| edit.step == step).collect() +} + +/// One edit, as lines for the terminal's scrollback. +/// +/// The header first — the path, then the counts, then the tool — because the +/// path is the content and the rest is metadata, which is the rule the whole +/// interface follows. +pub fn cell(edit: &Edit, theme: &Theme, width: u16) -> Vec> { + cell_styled(edit, theme, width, DiffStyle::Unified) +} + +/// The same, at a chosen style. +pub fn cell_styled(edit: &Edit, theme: &Theme, width: u16, style: DiffStyle) -> Vec> { + let mut lines = vec![header(edit, theme)]; + + let Some(hunk) = edit.hunk.as_deref() else { + // The header already carried the absence. Returning one line here is not + // a shortcut: a body of zero lines under a header is exactly the empty + // diff this must not draw. + return lines; + }; + + let mut drawn = body(hunk, &edit.path, theme, width >= EMPHASIS_FLOOR, style); + let cut = drawn.len().saturating_sub(MAX_BODY_LINES); + if cut > 0 { + drawn.truncate(MAX_BODY_LINES); + drawn.push(Line::from(Span::styled( + format!("{INDENT}⋯ {cut} more lines of this change — the whole of it is in the trace, and `/copy diff` carries it"), + theme.style(Tone::Muted), + ))); + } + lines.extend(drawn); + lines.push(Line::from("")); + lines +} + +/// ` src/theme.rs · +1 -1 · edit_file`, and `· no diff stored` when there is +/// no hunk to draw under it. +fn header(edit: &Edit, theme: &Theme) -> Line<'static> { + let mut spans = vec![ + Span::styled(INDENT.to_string(), theme.style(Tone::Muted)), + Span::styled(edit.path.clone(), theme.style(Tone::Accent)), + Span::styled(SEPARATOR.to_string(), theme.style(Tone::Muted)), + Span::styled(format!("+{}", edit.lines_added), theme.style(Tone::Added)), + Span::styled(" ".to_string(), theme.style(Tone::Muted)), + Span::styled( + format!("-{}", edit.lines_removed), + theme.style(Tone::Removed), + ), + Span::styled( + format!("{SEPARATOR}{}", edit.tool), + theme.style(Tone::Muted), + ), + ]; + if edit.hunk.is_none() { + spans.push(Span::styled( + format!("{SEPARATOR}no diff stored"), + theme.style(Tone::Muted), + )); + } + Line::from(spans) +} + +/// A hunk body, with the words that changed emphasised where they can be known. +/// +/// **The pairing rule is deliberately timid.** A run of removals is paired with +/// the run of additions immediately after it, and only when the two runs are the +/// same length — then line *n* of one is compared with line *n* of the other. +/// Anything else is drawn without a pairing, which means the whole line reads as +/// changed. +/// +/// The reason is in io-harness's own diff module: an `edit_file` is one +/// contiguous replacement by construction, but a `write_file` that rewrote two +/// distant regions arrives as **one hunk spanning both**. A greedy rule would +/// then emphasise the difference between lines that have nothing to do with each +/// other, which is worse than no emphasis — it invents a relationship and paints +/// it. +fn body( + hunk: &str, + path: &str, + theme: &Theme, + emphasis: bool, + style: DiffStyle, +) -> Vec> { + let syntax = syntax_for(path); + let raw: Vec<&str> = hunk.lines().collect(); + let mut out = Vec::new(); + let mut at = 0; + + while at < raw.len() { + // A `\ No newline at end of file` marker is emitted on its own line + // immediately after the line it applies to, so it can land BETWEEN a + // removal and its addition. Scanned through rather than treated as the + // end of a run: otherwise the additions run comes back empty, the pair is + // never made, and the word-level emphasis is lost on exactly the lines + // where a file's last line changed. + let marker = |line: &str| line.starts_with('\\'); + + let removals_from = at; + while at < raw.len() + && (raw[at].starts_with('-') || (at > removals_from && marker(raw[at]))) + { + at += 1; + } + let removals = &raw[removals_from..at]; + + let additions_from = at; + while at < raw.len() + && (raw[at].starts_with('+') || (at > additions_from && marker(raw[at]))) + { + at += 1; + } + let additions = &raw[additions_from..at]; + + if removals.is_empty() && additions.is_empty() { + // `minimal` keeps the `@@` header — a change with no line numbers is a + // change that does not say where it is — and drops the context, which + // is the only thing it drops. + let keep = style == DiffStyle::Unified || raw[at].starts_with("@@"); + if keep { + out.push(unchanged(raw[at], syntax, theme)); + } + at += 1; + continue; + } + + // Below the emphasis floor nothing is paired, so every changed line takes + // the whole wash — see `EMPHASIS_FLOOR`. + // Pairing counts only the changed lines; a marker is punctuation about + // the bytes and has no partner on the other side. + let removed: Vec<&&str> = removals.iter().filter(|l| !marker(l)).collect(); + let added: Vec<&&str> = additions.iter().filter(|l| !marker(l)).collect(); + let paired = emphasis && !removed.is_empty() && removed.len() == added.len(); + + let mut emit = |lines: &[&str], partners: &[&&str], tone| { + let mut nth = 0; + for line in lines { + if marker(line) { + out.push(unchanged(line, syntax, theme)); + continue; + } + let other = paired.then(|| *partners[nth]); + out.push(changed(line, other, tone, syntax, theme)); + nth += 1; + } + }; + emit(removals, &added, Tone::Removed); + emit(additions, &removed, Tone::Added); + } + + out +} + +/// A line that is not an addition or a removal: context, a `@@` header, or the +/// no-final-newline marker. +fn unchanged(line: &str, syntax: Option<&SyntaxReference>, theme: &Theme) -> Line<'static> { + match line.as_bytes().first() { + // `@@ … @@` — where in the file this is. The one part of a hunk a reader + // navigates by, so it takes the product's own colour and no highlighting: + // it is not code. + Some(b'@') => Line::from(Span::styled( + format!("{INDENT}{line}"), + theme.style(Tone::Accent), + )), + // `\ No newline at end of file`. Real information about the bytes, and + // not a change, so it reads as neither. + Some(b'\\') => Line::from(Span::styled( + format!("{INDENT}{line}"), + theme.style(Tone::Muted), + )), + // A context line. Its leading space is part of the diff and stays. + _ => { + let (marker, text) = line.split_at(usize::from(!line.is_empty())); + spans(marker, text, Tone::Normal, None, syntax, theme) + } + } +} + +/// One changed line, against its partner on the other side where there is one. +/// +/// With a partner, the shared head and tail are syntax coloured and only the +/// middle takes the diff's colour. Without one — or when the two share neither a +/// head nor a tail, in which case "what changed" is the whole line — the body +/// takes the diff's colour throughout. +fn changed( + mine: &str, + other: Option<&str>, + tone: Tone, + syntax: Option<&SyntaxReference>, + theme: &Theme, +) -> Line<'static> { + // The marker is the first byte and is ASCII, so this never splits a + // character. It stays on the line — it is what carries the meaning when + // colour does not. + let (marker, text) = mine.split_at(1); + + let range = other.and_then(|other| { + let other = &other[1..]; + let head = common_head(text, other); + // Measured on what is left after the head, so `a` against `aa` cannot + // count the same byte twice and produce a middle of negative length. + let tail = common_tail(&text[head..], &other[head..]); + // Sharing neither a head nor a tail means the whole line changed, which + // is the same situation as having no partner at all. + (head != 0 || tail != 0).then(|| head..text.len() - tail) + }); + + let Some(range) = range else { + // No partner, so nothing to emphasise *against* — the whole body takes + // the diff's colour and no emphasis. Bolding a whole line says "look + // here" about every word on it, which is the same as saying nothing. + return Line::from(Span::styled( + format!("{INDENT}{marker}{text}"), + theme.style(tone), + )); + }; + + spans(marker, text, tone, Some(range), syntax, theme) +} + +/// Build a body line: the marker in `tone`, the text syntax coloured, and +/// `changed` — where there is one — in `tone` and emphasised. +fn spans( + marker: &str, + text: &str, + tone: Tone, + changed: Option>, + syntax: Option<&SyntaxReference>, + theme: &Theme, +) -> Line<'static> { + let mut out = vec![Span::styled(format!("{INDENT}{marker}"), theme.style(tone))]; + + let runs = match syntax { + Some(syntax) => Highlighter::get().runs(text, syntax), + // No grammar for this file, so every run is one run. + None => vec![(0..text.len(), Tone::Normal)], + }; + + for (run, run_tone) in runs { + let mut at = run.start; + while at < run.end { + // Three cases, and the loop advances in every one: before the change, + // inside it, or past it. + let (end, inside) = match &changed { + Some(range) if at < range.start => (run.end.min(range.start), false), + Some(range) if at < range.end => (run.end.min(range.end), true), + _ => (run.end, false), + }; + if end <= at { + break; + } + let piece = text[at..end].to_string(); + out.push(match inside { + true => Span::styled(piece, theme.emphasis(tone)), + false => Span::styled(piece, theme.style(run_tone)), + }); + at = end; + } + } + + // A changed line whose text is empty still needs its marker to be a line. + Line::from(out) +} + +/// How many bytes `a` and `b` agree on from the front, always on a character +/// boundary. +fn common_head(a: &str, b: &str) -> usize { + let mut bytes = 0; + for (one, two) in a.chars().zip(b.chars()) { + if one != two { + break; + } + bytes += one.len_utf8(); + } + bytes +} + +/// The same from the back. +fn common_tail(a: &str, b: &str) -> usize { + let mut bytes = 0; + for (one, two) in a.chars().rev().zip(b.chars().rev()) { + if one != two { + break; + } + bytes += one.len_utf8(); + } + bytes +} diff --git a/src/events.rs b/src/events.rs index ff6a2e0..57bc8e8 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1,6 +1,6 @@ //! Turning io-harness run events into lines. //! -//! Two rules shape this module. +//! Three rules shape this module. //! //! **Nothing is dropped.** `EventKind` is `#[non_exhaustive]` and has fifty //! variants today, so a wildcard arm is not a shortcut here, it is required by @@ -12,18 +12,65 @@ //! live buffer that the viewport draws, and the whole passage is committed to //! scrollback once when it is finished. Committing per token would put a line in //! the terminal's scrollback for every few characters the model produced. +//! +//! **A tool call is a cell, not a line.** io-harness announces a call before it +//! runs — `EventKind::ToolCall` carries the tool and its target and nothing about +//! what came back, because nothing has come back yet — and reports the result +//! only in the `Step` that commits afterwards. So a call is held open from its +//! announcement, shown live in the viewport while it runs, and committed to +//! scrollback once, complete with its result and how long it took, when the step +//! lands. 0.2.0 committed the announcement immediately, which is why a +//! transcript said what the agent was about to do and never what happened. + +use std::time::Duration; -use io_harness::{EventKind, RunEvent}; +use io_harness::{EventKind, RunEvent, MCP_TOOL_PREFIX, NAMESPACE}; use ratatui::text::{Line, Span}; use crate::status::SEPARATOR; use crate::theme::{Theme, Tone}; +/// A tool call that has been announced and not yet closed. +/// +/// Held rather than committed because the two facts a reader actually wants — +/// what came back and how long it took — are not on the announcing event at all. +struct Pending { + name: String, + target: String, + /// The session age at which the call was announced. An age handed in by the + /// driver, never a clock read here: this module has no timer, and N1 is what + /// keeps it that way. + opened_at: Duration, + /// A duration io-harness itself measured, when it reports one. + /// + /// `EventKind::Mcp` is the only event in the whole enum that carries a + /// per-tool duration, so for an MCP tool this is a real measurement of how + /// long the tool ran. For every other tool it stays `None` and the cell + /// falls back to io-cli's own observation, which is a different kind of + /// number and is printed as one. + measured: Option, +} + /// Accumulates streaming text and turns events into committed lines. pub struct Events { theme: Theme, /// Model text received since the last commit. live: String, + /// Calls announced and not yet closed, in the order they were announced. + /// + /// A `Vec` and never an `Option`: `read_batch` announces every call in a + /// parallel batch up front and only then runs any of them, so two or more + /// open calls before a single `Step` is the ordinary case rather than an + /// edge one. A single slot would be wrong on the first parallel read. + open: Vec, + /// Whether the permission boundary refused something during the step now in + /// flight. + /// + /// A refusal does not end a step. io-harness turns it into an observation + /// fed back to the model and the step commits anyway, so this *marks* the + /// step rather than closing a cell — and it is the only honest result word + /// available when the step's own decisions cannot be paired to the calls. + refused_this_step: bool, } impl Events { @@ -31,6 +78,8 @@ impl Events { Self { theme, live: String::new(), + open: Vec::new(), + refused_this_step: false, } } @@ -38,15 +87,60 @@ impl Events { self.theme = theme; } - /// The unfinished tail of what is streaming: everything since the last - /// newline. Drawn in the viewport, where it can be replaced as it grows; a - /// committed line cannot be. - pub fn live(&self) -> &str { - &self.live + /// The one live row the viewport draws: an open tool call if there is one, + /// otherwise the unfinished tail of what is streaming. + /// + /// It is arbitrated rather than concatenated because the row is not free. + /// No `Token` arrives while a tool is dispatching, so the buffer is frozen — + /// but it is frozen holding the assistant's last unterminated line, which is + /// usually not empty. The obvious fix, flushing at `ToolCall` to empty it, + /// is wrong: committing the tail appends a blank line after it, so every tool + /// cell in the transcript would arrive with a blank row in front of it. + /// An open call is the more urgent thing to say, so it wins the row, and the + /// tail stays live until something legitimately commits it. + pub fn live(&self) -> String { + let Some(call) = self.open.last() else { + return self.live.clone(); + }; + let mut row = format!("⋅ {}", call.name); + if !call.target.is_empty() { + row.push(' '); + row.push_str(&call.target); + } + row.push_str(" …"); + if self.open.len() > 1 { + row.push_str(&format!(" (+{} more)", self.open.len() - 1)); + } + row } - /// Commit whatever has streamed so far, if anything. + /// End the turn: commit whatever streamed, then close every call still open. + /// + /// The public entry point, called when a turn finishes or is interrupted. A + /// `Step` may never arrive — io-harness skips `commit_step` when a sub-agent's + /// child deferred — so this is the only place that can honestly close a call + /// that nothing ever reported on. + /// + /// Those cells carry no duration. io-cli knows when the call was announced + /// and nothing at all about when it stopped, and a number printed there would + /// be a guess wearing a measurement's clothes. pub fn flush(&mut self) -> Vec> { + let mut lines = self.flush_text(); + let theme = self.theme; + for call in std::mem::take(&mut self.open) { + lines.push(cell_line(theme, &call, "unfinished", None)); + } + self.refused_this_step = false; + lines + } + + /// Commit whatever text has streamed so far, if any, leaving open calls open. + /// + /// Separate from the public `flush` because most of the callers below are + /// mid-step — a refusal and an approval request both arrive while the call + /// they are about is still running — and closing that call as unfinished + /// there would report the opposite of what happened. + fn flush_text(&mut self) -> Vec> { if self.live.trim().is_empty() { self.live.clear(); return Vec::new(); @@ -65,9 +159,16 @@ impl Events { lines } - /// What this event commits to scrollback. Empty means "nothing yet" — which - /// for a token is the correct answer, not a dropped event. - pub fn event(&mut self, event: &RunEvent) -> Vec> { + /// What this event commits to scrollback, given the session age the driver + /// read the clock for. + /// + /// Empty means "nothing yet" — which for a token, and for a tool call that + /// has not finished, is the correct answer rather than a dropped event. + /// + /// `at` is handed in for the same reason `App::tick` takes one: nothing here + /// may read a clock, so a test can state the interval between two events by + /// hand and assert on it without anything being timed. + pub fn event(&mut self, event: &RunEvent, at: Duration) -> Vec> { let theme = self.theme; match &event.kind { EventKind::Token { text } => { @@ -104,9 +205,52 @@ impl Events { tokens, changed, } => { + // Taken before anything else, so that the tail flush below + // cannot close these as unfinished a line before the step that + // finished them says otherwise. + let open = std::mem::take(&mut self.open); + let refused = std::mem::replace(&mut self.refused_this_step, false); + // A step's own narration commits after whatever streamed before // it, so the transcript reads in the order it happened. - let mut lines = self.flush(); + let mut lines = self.flush_text(); + + // Then the calls it ran, in the order they were announced, then + // the step line: chronological, which is the only order a + // transcript can be skimmed in. + // + // `decision` is `decisions.join("; ")`, one sentence per thing the + // step did, each written after that thing ran. Pairing them to the + // calls positionally is right whenever there is one of each — but + // the count is not guaranteed: the workspace loop pushes extra + // decisions for spawned children, and both loops push an "awaiting + // approval" segment. So the pairing is used only when the two + // counts agree, and never indexed into blind. + // An empty segment is not a result either, and a step whose + // decision is blank pairs to a cell with nothing in its result + // column — so a blank disqualifies the pairing rather than being + // printed as an answer. + let parts: Vec<&str> = decision.split("; ").collect(); + let complete = parts.iter().all(|part| !part.trim().is_empty()); + let paired = complete && parts.len() == open.len(); + let verdict = if *changed { + "changed files" + } else { + "no change" + }; + for (index, call) in open.iter().enumerate() { + // Never invented. The harness's own sentence when it can be + // matched, otherwise the coarsest true thing known about the + // step — a refusal marked it, or the step's own verdict. + let result = if paired { + parts[index] + } else if refused { + "refused" + } else { + verdict + }; + lines.push(cell_line(theme, call, result, Some(at))); + } // What was decided, what it ran, what came back — then the // metadata. 0.1.0 put the step number and the token count in the @@ -116,7 +260,10 @@ impl Events { let mut spans = vec![Span::styled(decision.clone(), theme.style(Tone::Normal))]; if !tool_call.is_empty() { spans.push(Span::styled(SEPARATOR, theme.style(Tone::Muted))); - spans.push(Span::styled(tool_call.clone(), theme.style(Tone::Accent))); + spans.push(Span::styled( + tool_names(tool_call), + theme.style(Tone::Accent), + )); } // Always said, in both directions. A result that appears only // sometimes is a column a reader cannot skim down, and `changed` @@ -138,19 +285,22 @@ impl Events { lines } EventKind::ToolCall { name, target } => { - // One line per call. The full output goes to the run's durable - // trace rather than to the screen; uncollapsed tool output is what - // makes a transcript unreadable. - let target = if target.is_empty() { - String::new() - } else { - format!(" {target}") - }; - vec![Line::from(vec![ - Span::styled(" ⋅ ", theme.style(Tone::Muted)), - Span::styled(name.clone(), theme.style(Tone::Accent)), - Span::styled(target, theme.style(Tone::Muted)), - ])] + // Nothing is committed here, and that is the point: this event is + // emitted before the call runs, so a line written now could only + // say what the agent was about to do. The call is held open, shown + // by `live()` while it runs, and committed once — with its result + // and its duration — by the `Step` above. + // + // One cell per call either way. The full output goes to the run's + // durable trace rather than to the screen; uncollapsed tool output + // is what makes a transcript unreadable. + self.open.push(Pending { + name: name.clone(), + target: target.clone(), + opened_at: at, + measured: None, + }); + Vec::new() } EventKind::Refused { act, @@ -158,6 +308,20 @@ impl Events { rule, layer, } => { + // Marks the step, and closes nothing. A refusal is fed back to the + // model as an observation and the step commits anyway, so there is + // no call to close here — and often no open call at all, since a + // dial, a verification and an MCP server spawn are all refused + // outside any step. Nothing below indexes into the open list, which + // is what keeps this arm correct at step zero. + // + // The act and the target are deliberately not matched against an + // open call: `act` is a policy verb such as "write" or literally + // "tool", and `target` is the policy's resolved path, while a + // call's target is the raw argument the model wrote. Matching them + // by string would pair the wrong things confidently. + self.refused_this_step = true; + // Act, target, rule, layer — in that order, and the last two are // the facts no other terminal agent can print, because no other // core records them. Asserted by position rather than by presence: @@ -177,7 +341,7 @@ impl Events { "{SEPARATOR}no rule named it: the tier default decided" )), } - let mut lines = self.flush(); + let mut lines = self.flush_text(); lines.push(theme.notice(Tone::Refused, text)); lines } @@ -188,7 +352,7 @@ impl Events { // drawn from those. This is the transcript's note that the run // stopped, not the question itself — the question must never be // committed, which is what F1 asserts. - let mut lines = self.flush(); + let mut lines = self.flush_text(); lines .push(theme.notice(Tone::Warning, format!("{act} {target} — waiting for you"))); lines @@ -201,7 +365,7 @@ impl Events { // The harness's own record of what it was told, which is not the // same line as io-cli's. They agree because the answer travelled // one way; if they ever disagree, this is where it shows. - let mut lines = self.flush(); + let mut lines = self.flush_text(); lines.push(theme.notice( if decision == "deny" { Tone::Refused @@ -217,7 +381,7 @@ impl Events { steps, tokens, } => { - let mut lines = self.flush(); + let mut lines = self.flush_text(); let tone = outcome_tone(outcome); lines.push(theme.notice(tone, format!("{outcome} · {steps} steps · {tokens} tok"))); if let Some(help) = outcome_help(outcome) { @@ -229,7 +393,42 @@ impl Events { lines.push(Line::from("")); lines } - // The other forty-three kinds. Not styled in this release, and not + EventKind::Mcp { + server, + tool, + millis, + .. + } => { + // The one place io-harness reports how long a tool actually ran. + // It is harvested onto the open cell rather than printed here, so + // that an MCP call closes with a measured duration while every + // other call closes with io-cli's observation of the interval — + // the difference the `~` prefix exists to show. + // + // Matched by rebuilding io-harness's own namespaced tool name, + // from io-harness's own two constants rather than from a spelling + // of them typed here, which is what `announce` puts on the + // `ToolCall`. Newest first, because a batch may hold two calls to + // the same tool and the later one is the one still running. + // + // A connect, a discover or a disconnect carries no tool and no + // duration, and matches nothing. + if let (Some(tool), Some(millis)) = (tool, millis) { + let name = format!("{MCP_TOOL_PREFIX}{server}{NAMESPACE}{tool}"); + if let Some(call) = self.open.iter_mut().rev().find(|c| c.name == name) { + call.measured = Some(Duration::from_millis(*millis)); + } + } + // Still rendered as itself. Harvesting a number off an event is not + // the same as having designed a line for it, and an event that + // vanished from the transcript because something read a field off + // it is exactly the silence this module refuses. + vec![Line::from(vec![ + Span::styled(" · ", theme.style(Tone::Muted)), + Span::styled(kind_name(&event.kind), theme.style(Tone::Muted)), + ])] + } + // The other forty-two kinds. Not styled in this release, and not // discarded either: each arrives as one muted line naming itself, so a // release that starts emitting something new is visible rather than // silent. @@ -243,6 +442,66 @@ impl Events { } } +/// One committed tool cell: the tool, its target, what came back, how long it +/// took. +/// +/// Content before metadata, like every other line in this interface, and every +/// fact in words rather than in colour — the result reads the same under +/// `NO_COLOR` and in a screen reader as it does on a colour terminal. +/// +/// `at` is the session age this cell is being closed at, or `None` when the cell +/// is being closed without anything having reported on it. +fn cell_line(theme: Theme, call: &Pending, result: &str, at: Option) -> Line<'static> { + let mut spans = vec![ + Span::styled(" ⋅ ", theme.style(Tone::Muted)), + Span::styled(call.name.clone(), theme.style(Tone::Accent)), + ]; + // `!= call.name` because io-harness falls the target back to the tool's own + // name when the call carries no path, pattern or key — so a `git_diff` with + // no argument arrived as `git_diff git_diff`, which reads like a stutter. + // Found in a live run, not in the suite. + if !call.target.is_empty() && call.target != call.name { + spans.push(Span::styled( + format!(" {}", call.target), + theme.style(Tone::Muted), + )); + } + spans.push(Span::styled(SEPARATOR, theme.style(Tone::Muted))); + spans.push(Span::styled(result.to_string(), theme.style(Tone::Normal))); + + // Two different kinds of number, told apart on the line itself. A measured + // duration is io-harness's own and is printed plainly; anything else is the + // interval io-cli observed between two events — which includes the model's + // own turnaround and the queue in front of the tool — and wears a `~` to say + // that it is an observation rather than how long the tool ran. + // + // `at` is `None` when the cell is closed with nothing having reported on it. + // io-cli does not know a duration there and says none, rather than printing + // the age of the announcement as though it were one. + let observed = at.map(|at| format!("~{}", format_millis(at.saturating_sub(call.opened_at)))); + if let Some(duration) = call.measured.map(format_millis).or(observed) { + spans.push(Span::styled( + format!("{SEPARATOR}{duration}"), + theme.style(Tone::Muted), + )); + } + Line::from(spans) +} + +/// `420ms`, `1.4s`, `92.0s`. A tool cell's own duration. +/// +/// Separate from [`format_elapsed`](crate::status::format_elapsed), which floors +/// to whole seconds and would print `0s` for every tool call that took less than +/// one — which is most of them. Two formats because they answer two questions: +/// how long the session has been open, and how long one call took. +fn format_millis(duration: Duration) -> String { + let millis = duration.as_millis(); + if millis < 1_000 { + return format!("{millis}ms"); + } + format!("{:.1}s", duration.as_secs_f64()) +} + /// How a run's outcome should read. /// /// The vocabulary is io-harness's, and the distinction that matters here is @@ -310,6 +569,26 @@ pub fn outcome_help(outcome: &str) -> Option<&'static str> { } } +/// The tools a step called, by name. +/// +/// `Step.tool_call` is `name:arguments` per call, joined by `" | "`, and the +/// arguments are raw JSON. 0.1.1 put the whole thing on the step line, which was +/// the best available then — a reader had nothing else saying what ran. A live +/// run of 0.3.0 showed what it costs now that tool cells exist: every step +/// printed its arguments as a wall of escaped JSON directly under a cell that had +/// just said the same thing readably. +/// +/// So the names are kept and the arguments are dropped. 0.1.1's F5 asks that a +/// step read as decision, then what it ran, then what came back, and it still +/// does — the tool cell above it is where the arguments live now. +fn tool_names(tool_call: &str) -> String { + tool_call + .split(" | ") + .map(|call| call.split_once(':').map_or(call, |(name, _)| name)) + .collect::>() + .join(", ") +} + /// The snake-case name of a kind, taken from its `Debug` form. /// /// `EventKind` is `#[non_exhaustive]` with no accessor for its own tag, and its diff --git a/src/lib.rs b/src/lib.rs index 520f58d..0815813 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,8 +14,10 @@ pub mod app; pub mod approval; pub mod bridge; pub mod cli; +pub mod clipboard; pub mod commands; pub mod composer; +pub mod diff; pub mod events; pub mod picker; pub mod settings; @@ -23,5 +25,6 @@ pub mod splash; pub mod status; pub mod term; pub mod theme; +pub mod transcript; pub mod verify; pub mod wizard; diff --git a/src/main.rs b/src/main.rs index 08f38f1..a69572f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,13 +22,14 @@ use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; use io_cli::app::{App, Command}; use io_cli::cli::{Cli, Command as Subcommand}; -use io_cli::commands::{self, Action}; +use io_cli::commands::{self, Action, Copied}; use io_cli::picker::{Outcome, Picker, Row}; use io_cli::settings::{self, CliSettings, Posture}; use io_cli::term::Screen; use io_cli::theme::{Theme, Tone}; use io_cli::wizard::{Progress, Wizard}; use io_cli::{approval, bridge, splash, verify}; +use ratatui::text::{Line, Span}; fn main() -> ExitCode { let runtime = match tokio::runtime::Runtime::new() { @@ -143,6 +144,11 @@ async fn drive( }; let policy = config.policy().unwrap_or_default(); + // `[app.io-cli]` again, read through the harness rather than parsed here. It + // is read in `drive` rather than in `run` because `run` may hand control to + // the wizard, which writes the file this then reads back. + let stored: Option = config.app(settings::APP_KEY).unwrap_or_default(); + let diff_style = settings::DiffStyle::from_setting(stored.and_then(|s| s.diff).as_deref()); let store = store_path().ok_or("no place to keep the run store")?; let store = Store::open(&store).map_err(|error| error.to_string())?; let session = Session::open(&store, root).map_err(|error| error.to_string())?; @@ -159,6 +165,7 @@ async fn drive( store, session, policy, + diff_style, theme, model, ) @@ -173,6 +180,7 @@ async fn drive( store, session, policy, + diff_style, theme, model, ) @@ -187,6 +195,7 @@ async fn drive( store, session, policy, + diff_style, theme, model, ) @@ -213,7 +222,7 @@ async fn drive( } }; loop_over( - screen, inputs, provider, store, session, policy, theme, model, + screen, inputs, provider, store, session, policy, diff_style, theme, model, ) .await } @@ -232,10 +241,15 @@ async fn loop_over( store: Store, mut session: Session, policy: Policy, + diff_style: settings::DiffStyle, theme: Theme, model: String, ) -> Result<(), String> { let mut app = App::new(theme, model); + app.set_diff_style(diff_style); + // Asked of the session rather than threaded down from `run`, so there is one + // answer to "which workspace is this" and it is the harness's. + app.set_root(session.root()); // What the file already says, read back rather than assumed. `None` means the // file holds a policy that is none of the three, which io-harness's own // configuration can express and this session must not relabel. @@ -314,6 +328,7 @@ async fn loop_over( // The viewport, and nothing above it. paint(screen, &mut app)?; } + Command::Transcript => commit_transcript(screen, &session, &store, &app.theme)?, Command::Slash(text) => match commands::parse(&text, &app.theme) { Action::Print(lines) => { screen.commit(&lines).map_err(|error| error.to_string())?; @@ -344,6 +359,26 @@ async fn loop_over( )]; picker = Some((Picker::new("Which model?", rows), Pick::Model)); } + Action::Expand => { + let lines = expand(&session, &store, &app.theme); + screen.commit(&lines).map_err(|error| error.to_string())?; + } + Action::Copy(what) => { + let (payload, said) = to_copy(&session, &store, what); + match payload { + Some(payload) => { + screen + .escape(&io_cli::clipboard::sequence(&payload)) + .map_err(|error| error.to_string())?; + // What was sent and how big it was — never "copied". + // Nothing answers an OSC 52 write, so a success + // message would be a claim this product cannot make. + app.say(Tone::Muted, io_cli::clipboard::describe(&payload)); + } + None => app.say(Tone::Muted, said), + } + } + Action::Transcript => commit_transcript(screen, &session, &store, &app.theme)?, }, Command::Submit(text) => { // Rebuilt every turn rather than kept, because `remembered` grows @@ -413,8 +448,13 @@ async fn turn( } } Some(event) = events.recv() => { - app.event(&event); - app.status.elapsed = started.elapsed(); + // One clock read, used for both. `main.rs` is the only module in + // `src/` allowed one, which is what keeps a tool cell's duration + // assertable without a test ever measuring anything. + let at = started.elapsed(); + app.status.elapsed = at; + app.event(&event, at); + commit_edits(app, store, &event, screen.width()); paint(screen, app)?; } Some(ask) = asks.recv() => { @@ -451,8 +491,17 @@ async fn turn( // Drain whatever the run emitted between its last event and its return, or // the tail of a turn is lost. + // One age for the whole drain. These events all arrived while the loop was + // not looking, and inventing distinct ages for them would be fiction. + let at = started.elapsed(); + let width = screen.width(); while let Ok(event) = events.try_recv() { - app.event(&event); + app.event(&event, at); + // The diff belongs here too. Without it a `Step { changed: true }` still + // queued when the turn's future won the select loses its diff silently — + // 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); } app.finished(); @@ -463,6 +512,129 @@ async fn turn( paint(screen, app) } +/// Draw what a step changed, by asking the store what it recorded. +/// +/// **Anchored on `Step` and never on `ToolCall`.** io-harness documents +/// `ToolCall` as *a tool was invoked, before its result is known*, while `Step` +/// is emitted after the step has been committed to the store — so a read at the +/// tool call is a read of a row that may not be there yet, and the two events +/// are one line apart in a transcript, which is what would make that invisible +/// until it was a bug. +/// +/// This is the first time this product reads the durable trace while a run is +/// live. It is safe because it happens on the drain half of the select, where +/// the turn future is suspended, and because every `Store` call is synchronous — +/// nothing this function does is held across an await, so there is no shape here +/// that can deadlock against the turn holding the same `&Store`. +/// +/// A read that fails degrades to a line saying so. A run whose work succeeded is +/// not a run to panic over because the trace could not be re-read, and silence +/// would say the step changed nothing. +fn commit_edits(app: &mut App, store: &Store, event: &io_harness::RunEvent, width: u16) { + let io_harness::EventKind::Step { changed: true, .. } = &event.kind else { + return; + }; + match store.edits(event.run_id) { + Ok(edits) => app.edits(&io_cli::diff::for_step(edits, event.step), width), + Err(error) => app.say( + Tone::Muted, + format!("the diff for this step could not be read: {error}"), + ), + } +} + +/// Put the whole conversation back into the terminal's own scrollback. +/// +/// Upward and never into a pane. The viewport is four rows and cannot grow, and +/// there is no alternate screen in this product — so the place a reader reads +/// something long is the terminal's own buffer, where its search, its selection +/// and tmux copy-mode already work. A failure to read the store says so and +/// changes nothing else: a session is not worth ending over a transcript. +fn commit_transcript( + screen: &mut Screen>, + session: &Session, + store: &Store, + theme: &Theme, +) -> Result<(), String> { + let lines = match session.transcript(store) { + Ok(transcript) => io_cli::transcript::lines(&transcript, theme), + Err(error) => vec![theme.notice( + Tone::Muted, + format!("the transcript could not be read: {error}"), + )], + }; + screen.commit(&lines).map_err(|error| error.to_string()) +} + +/// The last run of this session, if it has had one. +fn last_run(session: &Session, store: &Store) -> Option { + session + .transcript(store) + .ok()? + .turns + .into_iter() + .rfind(|turn| turn.on_path) +} + +/// The last step's full detail, from the run's durable trace. +/// +/// This is the other half of collapsing a tool cell: the screen is not the +/// archive, so the output goes to the store when it happens and comes back here +/// when somebody asks for it. Committed upward like everything else that shows +/// more of something. +fn expand(session: &Session, store: &Store, theme: &Theme) -> Vec> { + let Some(turn) = last_run(session, store) else { + return vec![theme.notice(Tone::Muted, "nothing has run in this session yet")]; + }; + let steps = match store.steps(turn.run_id) { + Ok(steps) => steps, + Err(error) => { + return vec![theme.notice(Tone::Muted, format!("the trace could not be read: {error}"))] + } + }; + let Some(step) = steps.last() else { + return vec![theme.notice(Tone::Muted, "that turn recorded no steps")]; + }; + if step.result.trim().is_empty() { + return vec![theme.notice( + Tone::Muted, + format!("step {} recorded no detail", step.step), + )]; + } + + let mut lines = vec![theme.notice( + Tone::Muted, + format!("step {} — {}", step.step, step.decision), + )]; + lines.extend( + step.result + .lines() + .map(|line| Line::from(Span::styled(format!(" {line}"), theme.style(Tone::Muted)))), + ); + lines.push(Line::from("")); + lines +} + +/// What `/copy` should put on the clipboard, or why there is nothing to put. +fn to_copy(session: &Session, store: &Store, what: Copied) -> (Option, String) { + let Some(turn) = last_run(session, store) else { + return (None, "nothing has run in this session yet".into()); + }; + match what { + Copied::Answer => match turn.reply { + Some(reply) if !reply.trim().is_empty() => (Some(reply), String::new()), + // A turn that stopped on a ceiling, a refusal or an interrupt has no + // closing message, and inventing one would misreport the ending. + _ => (None, "that turn ended without an answer to copy".into()), + }, + Copied::Diff => match store.patch(turn.run_id) { + Ok(patch) if !patch.trim().is_empty() => (Some(patch), String::new()), + Ok(_) => (None, "that turn changed no files".into()), + Err(error) => (None, format!("the patch could not be read: {error}")), + }, + } +} + /// The first-run wizard. Returns the theme chosen, or `None` if it was abandoned. async fn wizard( screen: &mut Screen>, diff --git a/src/settings.rs b/src/settings.rs index 63bfad7..2ab21a3 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -22,6 +22,41 @@ pub struct CliSettings { /// The theme by name. Absent means "detect from the terminal background". #[serde(default, skip_serializing_if = "Option::is_none")] pub theme: Option, + /// How much of a change to show: `unified` or `minimal`. + /// + /// Absent means `unified`, which is what every configuration file written + /// before 0.3.0 means — so this key needs no migration and an older binary + /// reading a file that has it ignores it, because `[app.io-cli]` is the one + /// section io-harness deliberately does not validate. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff: Option, +} + +/// How much of a change a diff shows. +/// +/// Two, not a number of context lines. The counter-pressure this answers is +/// approval fatigue: someone reviewing by file rather than by hunk wants the +/// changed lines and nothing else, and a dial from 0 to 3 is a dial nobody sets. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum DiffStyle { + /// The hunk as the harness stored it, context and all. + #[default] + Unified, + /// Changed lines only, with the `@@` header kept so the change still says + /// where in the file it is. + Minimal, +} + +impl DiffStyle { + /// What a configured value means. An unrecognised one is `Unified` rather + /// than an error: `[app.io-cli]` is unvalidated by design, and refusing to + /// start a session over a typo in a cosmetic key would be the wrong trade. + pub fn from_setting(value: Option<&str>) -> Self { + match value { + Some("minimal") => Self::Minimal, + _ => Self::Unified, + } + } } /// A default permission posture, in the words the wizard offers it in. @@ -157,6 +192,11 @@ pub fn render( app: AppSection { io_cli: CliSettings { theme: Some(theme.to_string()), + // Left out of the file the wizard writes. Its absence is + // `unified`, and a key written with its own default is a key a + // reader has to wonder about — and one that would have to be + // rewritten if the default ever changed. + diff: None, }, }, }; diff --git a/src/term.rs b/src/term.rs index afd4ab6..1f5a828 100644 --- a/src/term.rs +++ b/src/term.rs @@ -209,6 +209,25 @@ impl Screen { Write::flush(self.terminal.backend_mut()) } + /// Write a raw escape sequence straight to the terminal. + /// + /// For OSC 52, which is a message to the *terminal emulator* rather than + /// content for the screen: it puts a payload on the system clipboard and + /// draws nothing. It cannot be a widget, and it must not go through + /// [`Screen::commit`] — a clipboard sequence in the scrollback would be + /// text the user scrolls past rather than an instruction the terminal acted + /// on. + /// + /// Nothing here can confirm it worked. No terminal answers an OSC 52 write; + /// several cap the payload silently and tmux ignores it entirely without + /// `set -g set-clipboard on`. The `Ok` this returns means the bytes left the + /// process, which is the only thing that can honestly be reported. + pub fn escape(&mut self, sequence: &str) -> io::Result<()> { + let backend = self.terminal.backend_mut(); + backend.write_all(sequence.as_bytes())?; + Write::flush(backend) + } + /// Tell the renderer the terminal is a different size now. /// /// Recomputing the inline viewport is the whole job: the committed lines above diff --git a/src/theme.rs b/src/theme.rs index 4c3f2f0..04dd8dc 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -1,4 +1,4 @@ -//! The theme: nine tokens, two shipped themes, and a rule that colour is never +//! The theme: twelve tokens, two shipped themes, and a rule that colour is never //! the only thing carrying a meaning. //! //! The token set is deliberately small. Restraint is what makes a terminal @@ -6,6 +6,16 @@ //! consistently, and every extra token is another thing a second theme has to get //! right. //! +//! It was nine until 0.3.0, when syntax highlighting inside a diff added three: +//! keyword, string and literal. They are here rather than in the highlighter's +//! own theme format on purpose — `syntect` ships themes and this product does not +//! load them, so a highlighted diff and the rest of the interface stay one +//! aesthetic, and `NO_COLOR` keeps working because there is still exactly one +//! place that decides whether colour happens at all. A comment did not get a +//! fourth token: a comment is muted, `muted` already exists, and a token whose +//! value would always equal another's is a token that can drift out of agreement +//! with itself. +//! //! Nothing here paints a background. io-cli does not own the screen — the //! transcript is the terminal's own scrollback, sitting on whatever background //! the user chose — so a background token exists to *detect* against, not to @@ -75,6 +85,17 @@ pub enum Tone { /// An act the permission boundary refused. Its own tone because it is not an /// error — the system worked. Refused, + /// A line a change added. The `diff_add` token, which has been in the theme + /// since 0.1.0 waiting for the release that draws a diff. + Added, + /// A line a change removed. + Removed, + /// A language keyword inside a diff. + Keyword, + /// A string literal inside a diff. + StringLiteral, + /// A number, a boolean, a named constant. + Literal, } impl Tone { @@ -82,7 +103,17 @@ impl Tone { /// than meaning. pub fn word(self) -> Option<&'static str> { match self { - Self::Normal | Self::Muted | Self::Accent => None, + // A diff line's carrier is the `+` or the `-` the harness already + // put on it, which is why these two need no word of their own: the + // meaning survives `NO_COLOR` without one. + Self::Normal + | Self::Muted + | Self::Accent + | Self::Added + | Self::Removed + | Self::Keyword + | Self::StringLiteral + | Self::Literal => None, Self::Success => Some("ok"), Self::Warning => Some("warning"), Self::Error => Some("error"), @@ -97,7 +128,7 @@ impl fmt::Display for Tone { } } -/// The nine tokens. +/// The twelve tokens. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Theme { pub name: &'static str, @@ -112,6 +143,13 @@ pub struct Theme { pub error: Color, pub diff_add: Color, pub diff_delete: Color, + /// The three syntax tokens. Comments are deliberately not among them: a + /// comment is muted, and `muted` already exists — a fourth token whose value + /// would always equal an existing one is a token that can drift out of + /// agreement with itself. + pub syntax_keyword: Color, + pub syntax_string: Color, + pub syntax_literal: Color, /// Whether this theme emits colour at all. The `NO_COLOR` theme does not. pub coloured: bool, } @@ -131,6 +169,14 @@ pub const DARK: Theme = Theme { error: Color::LightRed, diff_add: Color::LightGreen, diff_delete: Color::LightRed, + // Indexed rather than the sixteen, and the reason is crowding: the eight + // bright ANSI colours are already spoken for by accent, success, warning, + // error and the two diff tokens, so a syntax colour taken from them would + // read as one of those meanings. These three are muted enough to sit under + // a diff without competing with the green and red that carry it. + syntax_keyword: Color::Indexed(176), + syntax_string: Color::Indexed(114), + syntax_literal: Color::Indexed(180), coloured: true, }; @@ -150,6 +196,9 @@ pub const LIGHT: Theme = Theme { error: Color::Red, diff_add: Color::Green, diff_delete: Color::Red, + syntax_keyword: Color::Indexed(90), + syntax_string: Color::Indexed(28), + syntax_literal: Color::Indexed(130), coloured: true, }; @@ -165,6 +214,9 @@ pub const PLAIN: Theme = Theme { error: Color::Reset, diff_add: Color::Reset, diff_delete: Color::Reset, + syntax_keyword: Color::Reset, + syntax_string: Color::Reset, + syntax_literal: Color::Reset, coloured: false, }; @@ -222,7 +274,28 @@ impl Theme { Tone::Refused => Style::default() .fg(self.warning) .add_modifier(Modifier::BOLD), + Tone::Added => Style::default().fg(self.diff_add), + Tone::Removed => Style::default().fg(self.diff_delete), + Tone::Keyword => Style::default().fg(self.syntax_keyword), + Tone::StringLiteral => Style::default().fg(self.syntax_string), + Tone::Literal => Style::default().fg(self.syntax_literal), + } + } + + /// A tone, emphasised — for the words inside a diff line that actually + /// changed. + /// + /// Bold rather than a background colour: a background paints the full cell + /// width of every character it covers, which on a diff line means a block + /// that survives being copied out of the terminal as trailing whitespace. + /// Under `NO_COLOR` this collapses to nothing along with everything else, + /// which is correct — the `+` and the `-` are the carriers there, and a + /// modifier is still a presentation-only channel. + pub fn emphasis(&self, tone: Tone) -> Style { + if !self.coloured { + return Style::default(); } + self.style(tone).add_modifier(Modifier::BOLD) } /// One line carrying a state, with the tone's word in front of it. diff --git a/src/transcript.rs b/src/transcript.rs new file mode 100644 index 0000000..fd9d8ad --- /dev/null +++ b/src/transcript.rs @@ -0,0 +1,156 @@ +//! The whole conversation, rendered for the terminal's own scrollback. +//! +//! This is what `Ctrl+T` commits. It is not a screen and cannot become one: the +//! viewport is four fixed rows that ratatui will not let anything grow, and there +//! is no alternate screen in this product to page a long conversation in. So the +//! output here is a `Vec` handed to `Screen::commit`, which pushes it above +//! the viewport with `insert_before` and leaves it in the terminal's real +//! scrollback — where the terminal's own scroll, search and copy already work. +//! Anything that paginated this would be re-implementing the scrollback the +//! product deliberately kept. +//! +//! **Branched-away turns are rendered, and said out loud.** +//! [`io_harness::Session::history`] returns only the turns the model can still +//! see, which is right for driving a next turn and wrong for reading back what +//! happened: a branch leaves earlier turns off the path, and +//! [`TranscriptTurn::on_path`] is the only place in this whole product where they +//! are visible at all. A renderer that skipped them would silently disagree with +//! the database about what the operator did, in the one surface built to answer +//! that — so every turn appears, and the ones the model has lost carry the words +//! saying so rather than a colour saying so. +//! +//! The compaction summaries each turn carries are not drawn. Nothing else in this +//! release renders a fold, and one is a paragraph standing in for steps whose own +//! narration is not on screen either; drawing it alone would raise a question the +//! interface has no other answer to yet. + +use io_harness::{Transcript, TranscriptTurn}; +use ratatui::text::{Line, Span}; + +use crate::events::outcome_tone; +use crate::status::SEPARATOR; +use crate::theme::{Theme, Tone}; + +/// The words a turn the model can no longer see is labelled with. +/// +/// Words rather than a tone, because a tone is not readable under `NO_COLOR`, on +/// a monochrome terminal, or to a colour-blind reader — and this is the single +/// fact about a conversation that a reader cannot reconstruct from anywhere else +/// in the product. +pub const BRANCHED_AWAY: &str = "left behind by a branch"; + +/// The conversation as lines, oldest turn first. +/// +/// Opens and closes with a marker so the transcript has visible edges in a +/// scrollback that also holds every earlier turn, every command's output and +/// whatever the shell printed before `io` started. The closing marker carries the +/// turn count, which is what tells a reader that the passage they just scrolled +/// past was the whole conversation rather than as much of it as fitted. +pub fn lines(transcript: &Transcript, theme: &Theme) -> Vec> { + let mut out = vec![Line::from(Span::styled( + format!( + "─── transcript begins{SEPARATOR}session {}{SEPARATOR}{}", + transcript.session_id, + transcript.root.display() + ), + theme.style(Tone::Accent), + ))]; + + if transcript.turns.is_empty() { + // A sentence, not an empty gap. Committing nothing at all is + // indistinguishable from the key having done nothing at all. + out.push(Line::from(Span::styled( + "This session has no turns yet, so there is nothing to show.".to_string(), + theme.style(Tone::Muted), + ))); + } + + for turn in &transcript.turns { + out.extend(turn_lines(turn, theme)); + } + + let count = transcript.turns.len(); + let branched = transcript.turns.iter().filter(|turn| !turn.on_path).count(); + let mut closing = format!( + "─── transcript ends{SEPARATOR}{count} turn{}", + if count == 1 { "" } else { "s" } + ); + if branched > 0 { + closing.push_str(&format!("{SEPARATOR}{branched} {BRANCHED_AWAY}")); + } + out.push(Line::from(Span::styled(closing, theme.style(Tone::Accent)))); + out +} + +/// One turn: what was asked, what was answered, and how it ended. +/// +/// The prompt comes before the turn id on its line, never after it — the +/// codebase-wide rule that content precedes metadata. A reader skims the left +/// edge for what was said, an id at the head of every line makes that column +/// unreadable, and the id is only ever wanted once the line has already been +/// found. +fn turn_lines(turn: &TranscriptTurn, theme: &Theme) -> Vec> { + // An empty prompt is still one row: `str::lines` yields nothing for an empty + // string, and a turn with no row at all would lose its id and its branch + // label along with it. + let mut rows: Vec<&str> = turn.prompt.lines().collect(); + if rows.is_empty() { + rows.push(""); + } + + let mut lines: Vec> = Vec::new(); + for (index, row) in rows.iter().enumerate() { + let marker = if index == 0 { "› " } else { " " }; + lines.push(Line::from(vec![ + Span::styled(marker, theme.style(Tone::Accent)), + Span::styled((*row).to_string(), theme.style(Tone::Normal)), + ])); + } + + // The tail rides the LAST prompt row rather than the first, so a multi-line + // prompt is not interrupted by its own metadata halfway down. + let tail = lines.last_mut().expect("a prompt is at least one row"); + if !turn.on_path { + tail.spans + .push(Span::styled(SEPARATOR, theme.style(Tone::Muted))); + tail.spans + .push(Span::styled(BRANCHED_AWAY, theme.style(Tone::Warning))); + } + tail.spans.push(Span::styled( + format!("{SEPARATOR}turn {}", turn.turn_id), + theme.style(Tone::Muted), + )); + + match turn + .reply + .as_deref() + .filter(|reply| !reply.trim().is_empty()) + { + Some(reply) => { + for row in reply.lines() { + let span = Span::styled(row.to_string(), theme.style(Tone::Normal)); + lines.push(Line::from(span)); + } + } + // Said, not omitted. A turn that produced no text is a different fact from + // a turn missing from the transcript, and only one of those is worth + // going to look into. + None => lines.push(Line::from(Span::styled( + " no reply".to_string(), + theme.style(Tone::Muted), + ))), + } + + if let Some(outcome) = &turn.outcome { + // `notice` is how every toned line in this product is built, so the tone's + // word is in front of the sentence by construction rather than by anyone + // remembering to put it there. + lines.push(theme.notice( + outcome_tone(outcome), + format!("{outcome}{SEPARATOR}run {}", turn.run_id), + )); + } + + lines.push(Line::from("")); + lines +} diff --git a/tests/approval.rs b/tests/approval.rs index f9a0f69..e232b8d 100644 --- a/tests/approval.rs +++ b/tests/approval.rs @@ -200,7 +200,7 @@ async fn f2_the_overlay_states_act_then_target_then_rule_then_layer() { let (request, context) = flagged(); let (ask, deciding) = asked(request, context).await; - let approval = Approval::new(ask); + let approval = Approval::new(ask, std::path::Path::new("")); let (mut screen, _recorder) = support::screen(80, 24); screen .draw(|frame| approval.render(frame, frame.area(), &DARK)) @@ -237,7 +237,7 @@ async fn f2_the_overlay_states_act_then_target_then_rule_then_layer() { async fn f2_an_unnamed_action_says_the_tier_default_decided() { let (ask, deciding) = asked(write(), ApprovalContext::new("tidy the parser")).await; - let approval = Approval::new(ask); + let approval = Approval::new(ask, std::path::Path::new("")); let (mut screen, _recorder) = support::screen(80, 24); screen .draw(|frame| approval.render(frame, frame.area(), &DARK)) @@ -253,11 +253,19 @@ async fn f2_an_unnamed_action_says_the_tier_default_decided() { deciding.await.expect("the approver did not panic"); } -/// The proposed content is what makes an answer an informed one, and it is the +/// What a write would do is what makes an answer an informed one, and it is the /// only part that can be arbitrarily long. It is fitted into the rows that are /// left and says how many it did not show. +/// +/// **Restated in 0.3.0.** Until then the overlay showed the proposed content as +/// plain lines, so this asserted the first of them was on screen. Now a write +/// whose target can be read is shown as a change, and at a session's four rows +/// the one row available carries `+40 -855` rather than the first line of the +/// file — which is the more useful row, and the one this test now pins. What has +/// not changed is the part that mattered: whatever is not shown is counted out +/// loud. #[tokio::test] -async fn the_overlay_shows_the_proposed_content_and_says_what_it_cut() { +async fn the_overlay_says_what_it_did_not_show() { let long: String = (0..40).map(|n| format!("line {n}\n")).collect(); let (ask, deciding) = asked( Request::new(Act::Write, "src/main.rs").with_content(long), @@ -265,17 +273,23 @@ async fn the_overlay_shows_the_proposed_content_and_says_what_it_cut() { ) .await; - let approval = Approval::new(ask); + let approval = Approval::new(ask, std::path::Path::new("")); let (mut screen, _recorder) = support::screen(80, 24); screen .draw(|frame| approval.render(frame, frame.area(), &DARK)) .expect("frame"); let text = screen.viewport_text(); - assert!(text.contains("line 0"), "the content is shown: {text:?}"); assert!( text.contains("more lines"), - "what was cut has to be said, or the reader is approving a file they think they read: {text:?}", + "what was cut has to be said, or the reader is approving a file they think \ + they read: {text:?}", + ); + // The size of the change, which is the decision. `src/main.rs` exists, so + // this is a real diff against a real file rather than an all-addition wall. + assert!( + text.contains('+') && text.contains('-'), + "the counts are the row that always survives: {text:?}", ); approval.answer(approval::Answer::Deny); @@ -468,3 +482,131 @@ async fn nothing_remembered_changes_nothing() { let base = Policy::default(); assert_eq!(approval::effective_policy(&base, &[]), base); } + +// --------------------------------------------------------------------------- +// F7 — an approval shows a proposed write as a diff against the file on disk, +// and a new file is all addition. +// --------------------------------------------------------------------------- + +/// Open an overlay for a write of `content` to `target`, and return what it +/// draws at `width` columns in `height` rows. +/// +/// Rendered through a real `Screen` rather than by calling the private layout, +/// because what matters is what reaches the terminal — and because the overlay +/// is height-constrained, which is the whole reason its content flexes. +async fn overlay_for(target: &std::path::Path, content: &str, height: u16) -> String { + use io_cli::approval::{self, Approval}; + use io_cli::theme::DARK; + use io_harness::{Act, ApprovalContext, Approver, Decision, Request}; + + let (asker, mut asks) = approval::channel(); + let request = Request::new(Act::Write, target.to_string_lossy().to_string()) + .with_content(content.to_string()); + let deciding = tokio::spawn(async move { + let asker = asker; + asker + .decide_in_context(&request, &ApprovalContext::new("tidy the parser")) + .await + }); + let ask = asks.recv().await.expect("the question arrived"); + + // An empty root leaves an absolute target resolving to itself, which is what + // these fixtures use; the relative case is covered by the live run, which is + // where it was found. + let approval = Approval::new(ask, std::path::Path::new("")); + // `height` is the VIEWPORT's height, which is what the overlay is drawn + // into — not the terminal's. A session's is four; a taller one is what a + // reader gets on a terminal with room, and both are worth asserting. + let (mut screen, _recorder) = support::screen_of(100, height + 4, height); + screen + .draw(|frame| approval.render(frame, frame.area(), &DARK)) + .expect("frame"); + let drawn = screen.viewport_text().to_string(); + + approval.answer(approval::Answer::Deny); + let decision = deciding.await.expect("the approver did not panic"); + assert!(matches!(decision, Decision::Deny { .. })); + drawn +} + +#[tokio::test] +async fn f7_a_write_over_an_existing_file_is_shown_as_what_changes() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let target = directory.path().join("parse.rs"); + let before = "fn one() {}\nfn two() {}\nfn three() {}\n"; + let after = "fn one() {}\nfn two(s: &str) {}\nfn three() {}\n"; + std::fs::write(&target, before).expect("the file exists first"); + + let drawn = overlay_for(&target, after, 8).await; + + // The counts are the single most useful row: this write touches one line. + assert!(drawn.contains("+1"), "{drawn}"); + assert!(drawn.contains("-1"), "{drawn}"); + // And the change itself, from the harness's own renderer. + assert!(drawn.contains("-fn two() {}"), "{drawn}"); + assert!(drawn.contains("+fn two(s: &str) {}"), "{drawn}"); + // The version that would ship by accident: every existing line shown as new. + assert!( + !drawn.contains("+fn one() {}"), + "an unchanged line was drawn as an addition, which means the old side was \ + empty — the write reads as four hundred lines when it is one: {drawn}", + ); +} + +#[tokio::test] +async fn f7_a_write_of_a_file_that_does_not_exist_yet_is_all_addition() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let target = directory.path().join("brand-new.rs"); + let after = "fn hello() {}\nfn goodbye() {}\n"; + + let drawn = overlay_for(&target, after, 8).await; + + assert!(drawn.contains("+2"), "two lines arrive: {drawn}"); + assert!(drawn.contains("-0"), "and none leave: {drawn}"); + assert!(drawn.contains("+fn hello() {}"), "{drawn}"); +} + +#[tokio::test] +async fn f7_no_file_other_than_the_request_s_own_target_is_read() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let target = directory.path().join("parse.rs"); + std::fs::write(&target, "fn one() {}\n").expect("the target"); + // A neighbour with a distinctive string in it. io-cli is not a file browser: + // the one workspace read this interface performs is of the file the operator + // is being asked about, and nothing else in the directory is its business. + std::fs::write( + directory.path().join("secrets.env"), + "TOKEN=NEVERREADTHISFILE\n", + ) + .expect("the neighbour"); + + let drawn = overlay_for(&target, "fn one() {}\nfn two() {}\n", 8).await; + + assert!( + !drawn.contains("NEVERREADTHISFILE"), + "a file the request never named reached the screen: {drawn}", + ); +} + +#[tokio::test] +async fn f7_at_the_tightest_size_the_counts_are_what_survives() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let target = directory.path().join("big.rs"); + std::fs::write(&target, "a\n".repeat(400)).expect("the target"); + + // Four rows: the question, its rule row, one row of content, the answers. + let drawn = overlay_for(&target, &"b\n".repeat(400), 4).await; + + // One row for the change, and it is spent on the size of the change rather + // than on the first line of it. Four hundred lines out and four hundred in is + // a different decision from one line out and one in. + assert!(drawn.contains("+400"), "{drawn}"); + assert!(drawn.contains("-400"), "{drawn}"); + // And the overlay still says how to answer. + assert!( + drawn.contains("allow once") && drawn.contains("deny"), + "{drawn}" + ); + // Nothing was silently dropped. + assert!(drawn.contains('⋯'), "the cut has to be said: {drawn}"); +} diff --git a/tests/clipboard.rs b/tests/clipboard.rs new file mode 100644 index 0000000..359903c --- /dev/null +++ b/tests/clipboard.rs @@ -0,0 +1,161 @@ +//! The clipboard write: a well-formed OSC 52 sequence, and wording that does not +//! claim an acknowledgement the terminal never sends. +//! +//! The base64 vectors here are the RFC 4648 ones, checked by hand rather than +//! produced by the encoder under test — an expectation generated from the code it +//! is testing agrees with every bug in it. + +use io_cli::clipboard::{describe, encode, sequence}; + +const ALPHABET: &str = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +/// The other direction, written out here so the round trip is checked against +/// something independent of `encode`. +fn decode(text: &str) -> Vec { + let mut bytes = Vec::new(); + let mut bits: u32 = 0; + let mut held = 0u32; + for character in text.chars().filter(|character| *character != '=') { + let value = ALPHABET + .find(character) + .unwrap_or_else(|| panic!("{character:?} is not a base64 character")); + bits = (bits << 6) | value as u32; + held += 6; + if held >= 8 { + held -= 8; + bytes.push((bits >> held) as u8); + } + } + bytes +} + +/// Split a sequence into its three parts by position, never by searching for the +/// payload inside it: a `contains` check passes just as happily on a sequence +/// whose prefix and payload came out the wrong way round. +fn payload_of(sequence: &str) -> &str { + let body = sequence + .strip_prefix("\x1b]52;c;") + .expect("the sequence opens with OSC 52, selection c"); + body.strip_suffix('\x07') + .expect("the sequence is terminated by BEL") +} + +#[test] +fn the_sequence_is_osc_52_around_the_encoded_payload() { + let text = "the quick brown fox"; + let written = sequence(text); + + let encoded = payload_of(&written); + assert_eq!( + encoded, "dGhlIHF1aWNrIGJyb3duIGZveA==", + "the middle of the sequence is not the base64 of the payload", + ); + assert_eq!( + decode(encoded), + text.as_bytes(), + "the payload did not survive the round trip", + ); + + // Positions, not membership. The introducer belongs at the front and the + // terminator at the back, and only one of each may exist. + assert_eq!(written.find('\x1b'), Some(0), "got {written:?}"); + assert_eq!( + written.find('\x07'), + Some(written.len() - 1), + "a BEL before the end would terminate the sequence early: {written:?}", + ); + assert_eq!(written.matches('\x07').count(), 1, "got {written:?}"); +} + +#[test] +fn a_payload_carrying_a_terminator_cannot_end_the_sequence_early() { + // Tool output is arbitrary bytes, so a transcript really can contain a BEL or + // an ESC. Encoded, neither reaches the terminal as itself. + let text = "before\x07after\x1b]0;title\x07"; + let written = sequence(text); + let encoded = payload_of(&written); + + assert!( + !encoded.contains('\x07') && !encoded.contains('\x1b'), + "a control byte reached the wire unencoded: {encoded:?}", + ); + assert_eq!( + decode(encoded), + text.as_bytes(), + "the round trip lost bytes" + ); +} + +#[test] +fn base64_pads_correctly_at_every_length() { + // RFC 4648's own vectors, at the three residues modulo three and either side + // of the first full group. + let vectors: [(&str, &str); 5] = [ + ("", ""), + ("f", "Zg=="), + ("fo", "Zm8="), + ("foo", "Zm9v"), + ("foob", "Zm9vYg=="), + ]; + for (input, expected) in vectors { + assert_eq!( + encode(input.as_bytes()), + expected, + "{input:?} encoded wrong", + ); + } +} + +#[test] +fn the_last_two_characters_of_the_alphabet_are_plus_and_slash() { + // The URL-safe variant differs from the standard one in exactly these two + // characters, and it differs silently: a `-_` payload decodes to other bytes + // rather than being rejected, so a terminal would paste something else. + assert_eq!(encode(&[0xFB, 0xFF, 0xFE]), "+//+"); + assert_eq!(encode(&[0xFF, 0xFF, 0xFF]), "////"); +} + +#[test] +fn multi_byte_utf8_survives_the_round_trip() { + let text = "café ☕ — 日本語 🙂"; + let written = sequence(text); + let decoded = decode(payload_of(&written)); + + assert_eq!(decoded, text.as_bytes(), "the bytes changed"); + assert_eq!( + String::from_utf8(decoded).expect("still valid UTF-8"), + text, + "the text changed", + ); +} + +#[test] +fn the_description_states_a_size_and_claims_nothing_about_the_clipboard() { + // Nothing on the wire answers an OSC 52 write, and tmux drops it outright + // without `set -g set-clipboard on`. Any of these words would be io-cli + // asserting an outcome it never observed. + let large = "a".repeat(4200); + for payload in ["", "x", "the quick brown fox", large.as_str()] { + let description = describe(payload).to_lowercase(); + for claim in ["copied", "success", "done"] { + assert!( + !description.contains(claim), + "{description:?} claims {claim:?}, which the terminal never confirmed", + ); + } + assert!( + description.starts_with("sent "), + "the description should open with what io-cli did: {description:?}", + ); + } + + assert_eq!(describe(""), "sent 0 bytes to the terminal clipboard"); + assert_eq!(describe("x"), "sent 1 byte to the terminal clipboard"); + assert_eq!(describe(&large), "sent 4.2 kB to the terminal clipboard"); + // Bytes, not characters: a terminal's undocumented ceiling is a byte count, + // and this string is four characters long. + assert_eq!( + describe("日本語 "), + "sent 10 bytes to the terminal clipboard" + ); +} diff --git a/tests/commands.rs b/tests/commands.rs index 45c6ee0..9db9e21 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -17,12 +17,22 @@ fn text(lines: &[ratatui::text::Line<'_>]) -> String { } #[test] -fn the_five_commands_are_the_five_commands() { +fn the_commands_are_the_commands() { let names: Vec<&str> = COMMANDS.iter().map(|(name, _)| *name).collect(); assert_eq!( names, - ["/help", "/quit", "/setup", "/theme", "/model"], - "the palette is 0.7.0; this release ships five", + [ + "/help", + "/quit", + "/setup", + "/theme", + "/model", + "/expand", + "/copy", + "/copy diff", + ], + "the fuzzy palette is still 0.7.0; this list is written out so that adding \ + a command is a decision somebody makes rather than a line somebody adds", ); for (name, what) in COMMANDS { assert!(name.starts_with('/'), "{name}"); @@ -37,6 +47,22 @@ fn each_command_resolves() { assert_eq!(commands::parse("setup", &DARK), Action::Setup); assert_eq!(commands::parse("theme", &DARK), Action::Theme); assert_eq!(commands::parse("model", &DARK), Action::Model); + assert_eq!(commands::parse("expand", &DARK), Action::Expand); + // `/copy` with no argument is the answer; `diff` and `patch` are the same + // thing, because a reader who has just been shown a diff types the word they + // were shown. + assert_eq!( + commands::parse("copy", &DARK), + Action::Copy(io_cli::commands::Copied::Answer), + ); + assert_eq!( + commands::parse("copy diff", &DARK), + Action::Copy(io_cli::commands::Copied::Diff), + ); + assert_eq!( + commands::parse("copy patch", &DARK), + Action::Copy(io_cli::commands::Copied::Diff), + ); // Arguments are tolerated; the first word decides. assert_eq!(commands::parse("model gpt-5", &DARK), Action::Model); @@ -87,13 +113,14 @@ fn the_key_table_covers_every_key_this_release_binds() { "Ctrl+L", "Esc", "Shift+Tab", + "Ctrl+T", "y / a / n", ] { assert!(documented.contains(&key), "{key} is bound but undocumented"); } assert_eq!( documented.len(), - 9, + 10, "a key was added to the table without being added to this list, or the \ other way round", ); diff --git a/tests/dependencies.rs b/tests/dependencies.rs index 454aa00..8b60f68 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -27,6 +27,11 @@ const ALLOWED: &[&str] = &[ "tokio", "serde", "toml", + // Ninth, and the first name added since 0.1.0. Syntax highlighting inside a + // diff, argued in 0.3.0's release record. Its FEATURE SET is asserted below + // as well as its name — see `n2_syntect_is_taken_with_exactly_the_features_ + // the_cross_compiles_survive`. + "syntect", ]; /// Crates that would mean a subsystem had been rebuilt here. Matched against @@ -231,3 +236,93 @@ fn n1_the_crate_is_never_published() { remembered", ); } + +/// N2 — the feature set, which is the load-bearing half of taking syntect. +/// +/// The name alone is not the contract. `syntect`'s DEFAULT features include +/// `regex-onig`, which is oniguruma — a C library. Two of this product's four +/// release artifacts are cross-compiled, to `x86_64-unknown-linux-musl` and +/// `x86_64-pc-windows-msvc`, and a native build step in that path fails inside +/// the release workflow rather than in a test. So a later `default-features = +/// true`, or a merge that drops the feature list, has to fail here — where it +/// costs a red check — instead of there, where it costs the Release. +#[test] +fn n2_syntect_is_taken_with_exactly_the_features_the_cross_compiles_survive() { + let manifest = manifest(); + let syntect = manifest + .get("dependencies") + .and_then(|table| table.get("syntect")) + .and_then(toml::Value::as_table) + .expect("syntect is a table, not a bare version string"); + + assert_eq!( + syntect + .get("default-features") + .and_then(toml::Value::as_bool), + Some(false), + "syntect's default features include regex-onig, which is a C library and \ + breaks the musl and MSVC cross-compiles", + ); + + let features: Vec<&str> = syntect + .get("features") + .and_then(toml::Value::as_array) + .expect("a features array") + .iter() + .filter_map(toml::Value::as_str) + .collect(); + assert_eq!( + features, + ["default-syntaxes", "regex-fancy"], + "the smallest syntect that highlights: the bundled grammars and the \ + pure-Rust regex engine. `default-themes` is deliberately absent — the \ + colours are io-cli's own tokens, which is what keeps a highlighted diff \ + and the rest of the interface one aesthetic and NO_COLOR working from \ + one place.", + ); +} + +/// N4 — the grammar table is never touched on the startup path. +/// +/// `SyntaxSet::load_defaults_newlines` decompresses every grammar syntect ships. +/// This product's own bar is a first paint inside a hundred milliseconds, and a +/// session that never edits a file should never pay for a highlighter. Asserted +/// structurally rather than by timing anything: the load appears once, in the +/// module that draws diffs, behind a `OnceLock`. +#[test] +fn n4_the_syntax_set_is_loaded_lazily_and_only_by_the_diff_renderer() { + let mut loaders = Vec::new(); + for (path, text) in sources() { + if text.contains("load_defaults") { + loaders.push(path.clone()); + } + } + + assert_eq!( + loaders.len(), + 1, + "the grammar table should be loaded in exactly one place: {loaders:?}", + ); + assert!( + loaders[0].ends_with("diff.rs"), + "only the diff renderer has any reason to load grammars: {loaders:?}", + ); + + let diff = std::fs::read_to_string(&loaders[0]).expect("the diff renderer"); + assert!( + diff.contains("OnceLock"), + "the load has to sit behind a once-cell, or it happens at startup", + ); + + // And the startup path does not reach it. `main.rs` builds the terminal, the + // config and the session before anything is drawn; a highlighter mentioned + // there is a highlighter built before the first frame. + let main = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"), + ) + .expect("the driver"); + assert!( + !main.contains("Highlighter") && !main.contains("SyntaxSet"), + "the driver reached for the highlighter, which puts it on the startup path", + ); +} diff --git a/tests/diff.rs b/tests/diff.rs new file mode 100644 index 0000000..40a9135 --- /dev/null +++ b/tests/diff.rs @@ -0,0 +1,585 @@ +//! F1 — an edit renders as the harness's own stored hunk. +//! F2 — an absent hunk reads as absent, never as an empty diff. +//! +//! The subject of both is `io_cli::diff`, which takes an `io_harness::Edit` and +//! returns lines. It is deliberately a function of a value rather than of a +//! store: the store read belongs to the driver, which is the only thing that +//! holds a `Store`, and keeping it out of here is what lets these tests state a +//! hunk by hand instead of standing up a database to hold one. + +mod support; + +use io_cli::diff::cell; +use io_cli::theme::DARK; +use io_harness::Edit; + +/// A terminal wide enough that word-level emphasis applies. Every test that is +/// not about the narrow form states it, so none of them depends on the floor by +/// accident. +const WIDE: u16 = 120; + +/// The text of a rendered cell, one line per line, as a reader would see it. +fn rendered(edit: &Edit) -> String { + cell(edit, &DARK, WIDE) + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") +} + +/// A hunk as io-harness renders one: a body, no `---`/`+++` headers, and `@@` +/// line numbers that are the *file's* rather than the hunk's own. +/// +/// The numbers here are the load-bearing part of F1. They say the change is at +/// line 12 of a file whose contents this test never supplies, which is precisely +/// the fact a diff computed inside io-cli from two strings could not produce. +const HUNK: &str = "\ +@@ -12,5 +12,5 @@ + fn one() {} + +- Tone::Muted => Style::default().fg(GREY), ++ Tone::Muted => Style::default().fg(self.muted), + + fn two() {} +"; + +fn edit_with_hunk() -> Edit { + Edit { + step: 3, + tool: "edit_file".to_string(), + path: "src/theme.rs".to_string(), + lines_added: 1, + lines_removed: 1, + hunk: Some(HUNK.to_string()), + } +} + +#[test] +fn f1_the_hunk_is_the_harness_s_own_text() { + let text = rendered(&edit_with_hunk()); + + // Every body line of the stored hunk survives, in order, with its marker. + for line in HUNK.lines() { + assert!( + text.contains(line), + "the rendered cell dropped {line:?}\n\n{text}", + ); + } +} + +#[test] +fn f1_the_line_numbers_are_the_file_s_and_are_not_recomputed() { + let text = rendered(&edit_with_hunk()); + + // The whole of F1. A diff io-cli computed from the two texts it was given + // would number this hunk from 1, because it has never seen the eleven lines + // above it. The header is passed through, character for character. + assert!( + text.contains("@@ -12,5 +12,5 @@"), + "the file's own line numbers are not in the cell:\n\n{text}", + ); + assert!( + !text.contains("@@ -1,5 +1,5 @@"), + "the hunk was renumbered, which means it was recomputed:\n\n{text}", + ); +} + +#[test] +fn f1_the_path_comes_before_the_counts() { + let text = rendered(&edit_with_hunk()); + let first = text.lines().next().expect("a header line"); + + let path = first + .find("src/theme.rs") + .expect("the path is on the header"); + let counts = first.find("+1").expect("the counts are on the header"); + // Content before metadata, asserted by position rather than by presence — + // 0.1.1 paid for the lesson that a `contains` assertion is just as green + // when the sentence is inside out. + assert!( + path < counts, + "the counts came before the path, which is metadata before content: {first:?}", + ); +} + +#[test] +fn f2_an_absent_hunk_says_so_and_keeps_its_counts() { + // `Edit.hunk` is `None` for three reasons and not one of them is "nothing + // changed": the row predates the harness release that added hunks, the + // file's previous contents were not kept, or the rendered diff would have + // exceeded the store's snapshot cap. The counts are still there in every one + // of those cases, and they are what says the file did change. + let edit = Edit { + step: 2, + tool: "write_file".to_string(), + path: "src/big.rs".to_string(), + lines_added: 812, + lines_removed: 3, + hunk: None, + }; + + let text = rendered(&edit); + + assert!(text.contains("src/big.rs"), "{text}"); + assert!(text.contains("+812"), "the additions are lost:\n\n{text}"); + assert!(text.contains("−3") || text.contains("-3"), "{text}"); + assert!( + text.contains("no diff stored"), + "an absent hunk has to say it is absent:\n\n{text}", + ); +} + +#[test] +fn f2_an_absent_hunk_never_renders_as_an_empty_diff() { + let edit = Edit { + step: 2, + tool: "write_file".to_string(), + path: "src/big.rs".to_string(), + lines_added: 812, + lines_removed: 3, + hunk: None, + }; + + let text = rendered(&edit); + + // The version that would ship by accident: `None` treated as an empty patch, + // which draws a cell that says the file was untouched while the counts + // beside it say it was not. + assert!( + !text.contains("@@"), + "an absent hunk drew a diff body:\n\n{text}", + ); + // Counted over the rendered lines and NOT over `str::lines()` of the joined + // text. That is not a stylistic preference: `"header\n".lines()` yields one + // element, so a text-level count cannot see the trailing blank line an + // `unwrap_or_default()` leaves behind — which is precisely the empty-patch + // shape this assertion exists to catch. The sabotage arm found it. + assert_eq!( + cell(&edit, &DARK, WIDE).len(), + 1, + "an absent hunk is one line and nothing else:\n\n{text}", + ); +} + +#[test] +fn f1_only_this_step_s_edits_are_drawn_at_this_step() { + // `Store::edits` answers for the whole run. A caller that draws what it + // returns re-renders every earlier edit at every later step, so the same + // diff appears once, then twice, then three times — a transcript that grows + // quadratically in the length of the turn. + let edits = vec![ + Edit { + step: 2, + path: "first.rs".to_string(), + ..Default::default() + }, + Edit { + step: 3, + path: "second.rs".to_string(), + ..Default::default() + }, + Edit { + step: 3, + path: "third.rs".to_string(), + ..Default::default() + }, + ]; + + let mine = io_cli::diff::for_step(edits, 3); + + let paths: Vec<&str> = mine.iter().map(|edit| edit.path.as_str()).collect(); + assert_eq!(paths, ["second.rs", "third.rs"]); +} + +/// The read this release adds, against a real store rather than against values. +/// +/// It is here because this is the first time io-cli reads the durable trace at +/// all: everything before 0.3.0 rendered events as they streamed past. The +/// round trip is what proves the hunk survives being written and read back, +/// which is the whole premise of F1 — that the diff on screen is the harness's +/// text and not io-cli's. +#[test] +fn f1_a_hunk_survives_the_store_round_trip() { + let directory = tempfile::tempdir().expect("a temporary directory"); + let store = io_harness::Store::open(directory.path().join("io.db")).expect("a store"); + let run_id = 1; + + store + .record_edit(run_id, &edit_with_hunk()) + .expect("the edit is recorded"); + + let read = store.edits(run_id).expect("the edits are read back"); + let mine = io_cli::diff::for_step(read, 3); + assert_eq!(mine.len(), 1, "one edit was recorded at step 3"); + assert_eq!( + mine[0].hunk.as_deref(), + Some(HUNK), + "the hunk came back changed", + ); + + let text = rendered(&mine[0]); + assert!(text.contains("@@ -12,5 +12,5 @@"), "{text}"); +} + +#[test] +fn f2_a_hunk_that_is_present_is_not_confused_for_an_absent_one() { + // The other direction, so the F2 sabotage arm cannot pass by rendering the + // absence sentence on everything. + let text = rendered(&edit_with_hunk()); + assert!(!text.contains("no diff stored"), "{text}"); +} + +// --------------------------------------------------------------------------- +// F3 — word-level emphasis inside a changed line. +// --------------------------------------------------------------------------- + +/// The spans of the one line in a rendered cell that starts with `marker`, +/// as (text, emphasised) pairs. +/// +/// Emphasis is read off the style rather than off the text, because the whole +/// point of F3 is *which part of the line* is marked — a test that looked at the +/// text alone could not tell a line emphasised in one piece from a line +/// emphasised in three. +fn spans_of(edit: &Edit, marker: char) -> Vec<(String, bool)> { + let lines = cell(edit, &DARK, WIDE); + let line = lines + .iter() + .find(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + .trim_start() + .starts_with(marker) + }) + .unwrap_or_else(|| panic!("no line starting with {marker:?}")); + line.spans + .iter() + .map(|span| { + ( + span.content.to_string(), + span.style + .add_modifier + .contains(ratatui::style::Modifier::BOLD), + ) + }) + .collect() +} + +#[test] +fn f3_only_the_changed_words_are_emphasised() { + let removed = spans_of(&edit_with_hunk(), '-'); + let added = spans_of(&edit_with_hunk(), '+'); + + let emphasised: Vec<&str> = removed + .iter() + .filter(|(_, on)| *on) + .map(|(text, _)| text.as_str()) + .collect(); + assert_eq!( + emphasised, + ["GREY"], + "the removed side should emphasise exactly what went: {removed:?}", + ); + + let emphasised: Vec<&str> = added + .iter() + .filter(|(_, on)| *on) + .map(|(text, _)| text.as_str()) + .collect(); + assert_eq!( + emphasised, + ["self.muted"], + "the added side should emphasise exactly what arrived: {added:?}", + ); +} + +#[test] +fn f3_the_emphasis_sits_between_the_common_head_and_the_common_tail() { + let removed = spans_of(&edit_with_hunk(), '-'); + + // Asserted by POSITION: head, then the change, then tail. A membership + // assertion is just as green when the line is inside out, which 0.1.1 paid + // for and 0.2.0 paid for again. + let at = removed + .iter() + .position(|(_, on)| *on) + .expect("something is emphasised"); + assert!( + at > 0, + "there is a common head before the change: {removed:?}" + ); + assert!( + at + 1 < removed.len(), + "there is a common tail after the change: {removed:?}", + ); + assert!( + removed[at - 1].0.ends_with(".fg("), + "the head stops where the lines stop agreeing: {removed:?}", + ); + assert_eq!( + removed[at + 1].0, + "),", + "the tail starts where they agree again: {removed:?}", + ); +} + +#[test] +fn f3_an_unpaired_line_is_emphasised_at_the_line_and_not_within_it() { + // Two lines out and one in. There is no honest pairing here — the harness + // renders a `write_file` that rewrote two distant regions as ONE hunk + // spanning both, so a rule that paired by position would emphasise the + // difference between lines that have nothing to do with each other. + let edit = Edit { + step: 1, + tool: "write_file".to_string(), + path: "a.rs".to_string(), + lines_added: 1, + lines_removed: 2, + hunk: Some( + "@@ -1,3 +1,2 @@\n-let a = 1;\n-let b = 2;\n+let everything = 0;\n ok\n".to_string(), + ), + }; + + for (text, emphasised) in spans_of(&edit, '-') { + assert!( + !emphasised, + "an unpaired removal was emphasised within the line: {text:?}", + ); + } + assert_eq!( + spans_of(&edit, '-').len(), + 1, + "an unpaired line is one span", + ); +} + +// --------------------------------------------------------------------------- +// F4 — highlighting is drawn in io-cli's own tokens and disappears under +// NO_COLOR. N2 — the dependency set grows by exactly one name, with its +// features pinned. N4 — the syntax set is never loaded on the startup path. +// --------------------------------------------------------------------------- + +/// A hunk of Rust with a keyword, a string and a number in it, so each of the +/// three syntax tokens has something to colour. +fn edit_with_code() -> Edit { + Edit { + step: 1, + tool: "edit_file".to_string(), + path: "src/lib.rs".to_string(), + lines_added: 1, + lines_removed: 1, + hunk: Some( + "@@ -1,3 +1,3 @@\n \ + // a comment\n\ + -let name = \"old\";\n\ + +let name = \"new\";\n \ + const N: u32 = 42;\n" + .to_string(), + ), + } +} + +#[test] +fn f4_the_colours_come_from_io_cli_s_own_tokens() { + let edit = edit_with_code(); + let coloured: Vec<(String, Option)> = cell(&edit, &DARK, WIDE) + .iter() + .flat_map(|line| { + line.spans + .iter() + .map(|span| (span.content.to_string(), span.style.fg)) + .collect::>() + }) + .collect(); + let used: Vec = coloured.iter().filter_map(|(_, fg)| *fg).collect(); + + // Every colour on a context line is one this theme declares. A colour from + // syntect's own theme set would be an RGB value that appears nowhere in + // `theme.rs` — which is the whole reason `default-themes` is off. + let mine = [ + DARK.foreground, + DARK.muted, + DARK.accent, + DARK.syntax_keyword, + DARK.syntax_string, + DARK.syntax_literal, + DARK.diff_add, + DARK.diff_delete, + ]; + for colour in &used { + assert!( + mine.contains(colour), + "{colour:?} is not one of io-cli's tokens: {coloured:?}", + ); + } + assert!( + !used.is_empty(), + "nothing was coloured at all, so this asserts nothing: {coloured:?}", + ); +} + +#[test] +fn f4_a_keyword_a_string_and_a_number_each_get_their_own_token() { + let edit = edit_with_code(); + let all: Vec<(String, Option)> = cell(&edit, &DARK, WIDE) + .iter() + .flat_map(|line| { + line.spans + .iter() + .map(|span| (span.content.to_string(), span.style.fg)) + .collect::>() + }) + .collect(); + + let has = |text: &str, colour: ratatui::style::Color| { + all.iter() + .any(|(content, fg)| content.contains(text) && *fg == Some(colour)) + }; + + // `let` and `const` are `storage.*` in Sublime's scope vocabulary rather + // than `keyword.*`. Asserting on them rather than on an operator is what + // caught the missing `storage` entry in the scope table. + assert!(has("let", DARK.syntax_keyword), "let is a keyword: {all:?}"); + assert!( + has("const", DARK.syntax_keyword), + "const is a keyword: {all:?}" + ); + assert!( + has("old", DARK.diff_delete), + "the changed word keeps the diff colour: {all:?}" + ); + assert!(has("42", DARK.syntax_literal), "literal: {all:?}"); + assert!( + has("a comment", DARK.muted), + "comment reads as muted: {all:?}" + ); +} + +#[test] +fn f4_no_color_leaves_the_markers_carrying_the_meaning() { + use io_cli::theme::PLAIN; + + let lines = cell(&edit_with_code(), &PLAIN, WIDE); + for line in &lines { + for span in &line.spans { + assert_eq!( + span.style.fg, None, + "NO_COLOR emitted a colour: {:?}", + span.content, + ); + assert!( + span.style.add_modifier.is_empty(), + "NO_COLOR emitted a modifier, which is still a presentation-only \ + carrier: {:?}", + span.content, + ); + } + } + + // And the meaning survives, because the markers are text. + let text: String = lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(text.contains("-let name = \"old\";"), "{text}"); + assert!(text.contains("+let name = \"new\";"), "{text}"); +} + +#[test] +fn f4_a_file_with_no_grammar_still_renders() { + // `None` from `find_syntax_by_extension` is not a failure — it is what a + // file syntect has never heard of should look like. + let edit = Edit { + step: 1, + tool: "write_file".to_string(), + path: "notes.zzz".to_string(), + lines_added: 1, + lines_removed: 0, + hunk: Some("@@ -0,0 +1 @@\n+hello\n".to_string()), + }; + let text = rendered(&edit); + assert!(text.contains("+hello"), "{text}"); +} + +// --------------------------------------------------------------------------- +// F6 — `minimal` shows changed lines only, and comes from the harness's own +// configuration. +// --------------------------------------------------------------------------- + +#[test] +fn f6_minimal_keeps_the_changed_lines_and_the_header_and_drops_the_context() { + use io_cli::diff::cell_styled; + use io_cli::settings::DiffStyle; + + let edit = edit_with_hunk(); + let text = |style| { + cell_styled(&edit, &DARK, WIDE, style) + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") + }; + + let unified = text(DiffStyle::Unified); + let minimal = text(DiffStyle::Minimal); + + // What changed survives both. + for style in [&unified, &minimal] { + assert!( + style.contains("- Tone::Muted => Style::default().fg(GREY),"), + "{style}" + ); + assert!( + style.contains("+ Tone::Muted => Style::default().fg(self.muted),"), + "{style}" + ); + } + + // The context is what `minimal` drops, and only that. + assert!(unified.contains("fn one() {}"), "{unified}"); + assert!( + !minimal.contains("fn one() {}"), + "minimal kept a context line:\n{minimal}" + ); + assert!(!minimal.contains("fn two() {}"), "{minimal}"); + + // The `@@` header stays. A change with no line numbers is a change that does + // not say where in the file it is, which is not a smaller diff — it is a + // worse one. + assert!( + minimal.contains("@@ -12,5 +12,5 @@"), + "minimal dropped the line numbers:\n{minimal}", + ); + assert!(minimal.contains("src/theme.rs"), "{minimal}"); +} + +#[test] +fn f6_the_style_is_read_from_the_setting_and_an_unknown_value_is_unified() { + use io_cli::settings::DiffStyle; + + assert_eq!(DiffStyle::from_setting(Some("minimal")), DiffStyle::Minimal); + assert_eq!(DiffStyle::from_setting(Some("unified")), DiffStyle::Unified); + // Absent means unified, which is what every file written before 0.3.0 means. + assert_eq!(DiffStyle::from_setting(None), DiffStyle::Unified); + // `[app.io-cli]` is the one section io-harness deliberately does not + // validate, so a typo in a cosmetic key must not stop a session starting. + assert_eq!(DiffStyle::from_setting(Some("minmal")), DiffStyle::Unified); + assert_eq!(DiffStyle::default(), DiffStyle::Unified); +} diff --git a/tests/events.rs b/tests/events.rs index 77be9af..dab965f 100644 --- a/tests/events.rs +++ b/tests/events.rs @@ -7,13 +7,21 @@ mod support; +use std::time::Duration; + use io_cli::events::{kind_name, Events}; use io_cli::theme::DARK; use io_harness::{EventKind, RunEvent}; -/// The kinds this release styles, in the contract's own words: `started`, +/// The kinds this release handles, in the contract's own words: `started`, /// `token`, `step`, `tool_call` and `finished` rendered fully, `refused` and /// `approval_requested` as a plain one-line notice. +/// +/// `mcp` is here from 0.3.0 for a reason that is not visible in its own line: it +/// is the only event io-harness emits carrying how long a tool actually ran, and +/// that number is harvested onto the open tool cell. Its own line is still the +/// muted one naming the kind — an event is never consumed silently — but it is no +/// longer a kind this release merely passes through. const STYLED: &[&str] = &[ "started", "token", @@ -22,6 +30,7 @@ const STYLED: &[&str] = &[ "finished", "refused", "approval_requested", + "mcp", ]; /// Every other kind io-harness 0.60.1 emits. Each renders as a muted single line @@ -50,7 +59,6 @@ const FALLS_THROUGH: &[&str] = &[ "reasoning", "server_tool_used", "sandbox", - "mcp", "handle_started", "handle_polled", "handle_killed", @@ -78,9 +86,22 @@ fn event(kind: EventKind) -> RunEvent { RunEvent::new(1, 1, kind) } +/// What one event commits, at a session age nothing measured. +/// +/// `Duration::ZERO` is the right default for every test that is not about a tool +/// cell's duration: the age is an argument handed in by the driver, so a test +/// that does not care about it states zero rather than arranging for a clock. fn rendered(events: &mut Events, kind: EventKind) -> String { - events - .event(&event(kind)) + rendered_at(events, kind, Duration::ZERO) +} + +/// The same, at a stated session age. Stated, never measured — N1. +fn rendered_at(events: &mut Events, kind: EventKind, at: Duration) -> String { + flatten(events.event(&event(kind), at)) +} + +fn flatten(lines: Vec>) -> String { + lines .iter() .flat_map(|line| line.spans.iter()) .map(|span| span.content.as_ref()) @@ -90,8 +111,8 @@ fn rendered(events: &mut Events, kind: EventKind) -> String { #[test] fn f5_a_step_reads_decision_then_tool_then_result_then_its_metadata() { let mut events = Events::new(DARK); - let line: String = events - .event(&RunEvent::new( + let line: String = flatten(events.event( + &RunEvent::new( 1, 7, EventKind::Step { @@ -100,11 +121,9 @@ fn f5_a_step_reads_decision_then_tool_then_result_then_its_metadata() { tokens: 1234, changed: true, }, - )) - .iter() - .flat_map(|line| line.spans.iter()) - .map(|span| span.content.as_ref()) - .collect(); + ), + Duration::ZERO, + )); // The order is the assertion, not the contents. A line carrying all five // facts in the wrong order is the line 0.1.0 shipped. @@ -136,8 +155,8 @@ fn f5_a_step_reads_decision_then_tool_then_result_then_its_metadata() { #[test] fn f5_a_step_that_changed_nothing_still_says_so() { let mut events = Events::new(DARK); - let line: String = events - .event(&RunEvent::new( + let line: String = flatten(events.event( + &RunEvent::new( 1, 2, EventKind::Step { @@ -146,11 +165,9 @@ fn f5_a_step_that_changed_nothing_still_says_so() { tokens: 88, changed: false, }, - )) - .iter() - .flat_map(|line| line.spans.iter()) - .map(|span| span.content.as_ref()) - .collect(); + ), + Duration::ZERO, + )); // The result is always present, so a skim down the transcript reads the same // column of answers whether or not a step touched anything. @@ -179,15 +196,29 @@ fn f8_every_styled_kind_renders_its_own_facts() { ); assert!(started.contains("openrouter"), "{started:?}"); - let tool = rendered( - &mut events, - EventKind::ToolCall { + // A tool call's facts are asserted where they are committed, which is the step + // that finished it. The announcement itself commits nothing, because when it + // arrives io-harness does not yet know what came back. + events.event( + &event(EventKind::ToolCall { name: "exec".into(), target: "cargo test".into(), + }), + Duration::ZERO, + ); + let tool = rendered_at( + &mut events, + EventKind::Step { + decision: "ran cargo test".into(), + tool_call: "exec".into(), + tokens: 12, + changed: false, }, + Duration::from_millis(900), ); assert!(tool.contains("exec"), "{tool:?}"); assert!(tool.contains("cargo test"), "{tool:?}"); + assert!(tool.contains("ran cargo test"), "{tool:?}"); let step = rendered( &mut events, @@ -261,7 +292,10 @@ fn f8_a_kind_this_release_does_not_style_still_renders_and_names_itself() { fn f8_tokens_are_coalesced_rather_than_committed_one_at_a_time() { let mut events = Events::new(DARK); for word in ["Here ", "is ", "the ", "answer."] { - let committed = events.event(&event(EventKind::Token { text: word.into() })); + let committed = events.event( + &event(EventKind::Token { text: word.into() }), + Duration::ZERO, + ); assert!( committed.is_empty(), "a token committed a line to scrollback on its own", @@ -282,8 +316,16 @@ fn f8_tokens_are_coalesced_rather_than_committed_one_at_a_time() { assert_eq!(events.live(), "", "the live buffer should be emptied"); } +/// **F8.** No event is dropped. +/// +/// The invariant is not "every event commits a line" — a token does not, and from +/// 0.3.0 neither does a tool call. Both are *deferred* rather than discarded: the +/// token into the live buffer, the call into an open cell that `live()` shows +/// immediately and that the step commits complete. So each kind must leave a mark +/// somewhere a reader can see it, and the two that defer are named here rather +/// than exempted, so that a third one cannot be added silently. #[test] -fn f8_no_event_renders_to_nothing_except_a_token() { +fn f8_no_event_is_dropped_though_a_token_and_a_tool_call_are_deferred() { let mut events = Events::new(DARK); let kinds = vec![ EventKind::Started { @@ -297,7 +339,7 @@ fn f8_no_event_renders_to_nothing_except_a_token() { changed: false, }, EventKind::ToolCall { - name: "n".into(), + name: "read_file".into(), target: String::new(), }, EventKind::Refused { @@ -321,12 +363,32 @@ fn f8_no_event_renders_to_nothing_except_a_token() { for kind in kinds { let name = kind_name(&kind); - let lines = events.event(&event(kind)); + let lines = events.event(&event(kind), Duration::ZERO); + if name == "tool_call" { + // Deferred, and visible while it is deferred. An open call that + // committed nothing AND showed nothing would be the discarded event + // this test exists to catch. + assert!( + lines.is_empty(), + "a tool call committed a line before its result was known", + ); + assert!( + events.live().contains("read_file"), + "a deferred tool call must be visible in the viewport: {:?}", + events.live(), + ); + continue; + } assert!( !lines.is_empty(), "{name} rendered to nothing, which is an event silently discarded", ); } + + // The turn ends, and the call that nothing ever reported on is still accounted + // for rather than lost with the turn. + let closed = flatten(events.flush()); + assert!(closed.contains("read_file"), "{closed:?}"); } #[test] @@ -541,6 +603,215 @@ fn f8_a_refusal_with_no_rule_says_the_tier_default_decided() { ); } +/// **F2.** A call announced is not a call finished. +/// +/// `EventKind::ToolCall` is emitted before the tool runs, so anything committed +/// here could only say what the agent was about to do — which is the line 0.2.0 +/// shipped and the reason a transcript never said what came back. It is held +/// open instead, and shown live, which is the half of the design that keeps the +/// deferral from being a disappearance. +#[test] +fn f2_a_tool_call_commits_nothing_until_its_step_lands() { + let mut events = Events::new(DARK); + let lines = events.event( + &event(EventKind::ToolCall { + name: "read_file".into(), + target: "src/lib.rs".into(), + }), + Duration::ZERO, + ); + + assert!( + lines.is_empty(), + "a call committed a line before anything knew its result: {lines:?}", + ); + let live = events.live(); + assert!(live.contains("read_file"), "{live:?}"); + assert!(live.contains("src/lib.rs"), "{live:?}"); +} + +/// **F2.** The whole cell, in one line, when the step that ran it commits. +/// +/// Asserted by position rather than by presence. A line carrying all four facts +/// inside out passes `contains` just as green, which this repository has now paid +/// to learn twice. +#[test] +fn f2_a_tool_cell_commits_its_result_and_its_observed_duration() { + let mut events = Events::new(DARK); + events.event( + &event(EventKind::ToolCall { + name: "read_file".into(), + target: "src/lib.rs".into(), + }), + Duration::ZERO, + ); + let line = rendered_at( + &mut events, + EventKind::Step { + // io-harness's own sentence about what the call did, written after it + // ran. The cell says this and never a word io-cli made up. + decision: "read src/lib.rs".into(), + tool_call: "read_file".into(), + tokens: 5, + changed: false, + }, + Duration::from_millis(250), + ); + + let at = |needle: &str| { + line.find(needle) + .unwrap_or_else(|| panic!("{needle:?} is missing from {line:?}")) + }; + assert!( + at("read_file") < at("src/lib.rs"), + "the tool before its target: {line:?}", + ); + assert!( + at("src/lib.rs") < at("read src/lib.rs"), + "the target before what came back: {line:?}", + ); + assert!( + at("read src/lib.rs") < at("~250ms"), + "content before metadata: {line:?}", + ); + + // The `~` is load-bearing. io-cli did not time the tool; it observed the + // interval between two events, which also contains the model's turnaround and + // whatever queued in front of the call. A bare `250ms` here would be io-cli + // claiming a measurement it never made. + assert!( + !line.contains(" 250ms"), + "an observed interval must not read as a measured duration: {line:?}", + ); +} + +/// **F2.** A refused call never closes as a completed one. +/// +/// A refusal does not end the step — io-harness feeds it back to the model as an +/// observation and the step commits anyway — so the cell is still closed by that +/// step. What it must not do is close wearing the step's own verdict, which would +/// report a call that was stopped as a call that ran. +#[test] +fn f2_a_refused_call_does_not_close_as_a_completed_one() { + let mut events = Events::new(DARK); + events.event( + &event(EventKind::ToolCall { + name: "write_file".into(), + target: "src/main.rs".into(), + }), + Duration::ZERO, + ); + rendered( + &mut events, + EventKind::Refused { + act: "write".into(), + target: "/repo/src/main.rs".into(), + rule: Some("fs.deny".into()), + layer: Some("workspace".into()), + }, + ); + + // Two decisions against one open call: the count does not pair, which is the + // ordinary shape of a step that also waited on an approval. The refusal is + // then the only true thing left to say about the call. + let line = rendered_at( + &mut events, + EventKind::Step { + decision: "write refused; awaiting approval (write)".into(), + tool_call: "write_file".into(), + tokens: 40, + changed: false, + }, + Duration::from_millis(300), + ); + + assert!(line.contains("refused"), "{line:?}"); + assert!( + !line.contains("changed files"), + "a refused call must not close as one that changed something: {line:?}", + ); + assert!( + line.find("refused") < line.find("no change"), + "the cell closes before the step it belonged to: {line:?}", + ); +} + +/// **F2.** A call nothing ever reported on closes as unfinished, with no number. +/// +/// `Step` may never arrive: io-harness skips `commit_step` when a sub-agent's +/// child deferred, and a turn can be interrupted mid-call. The cell is still +/// accounted for — and carries no duration, because io-cli knows when the call +/// was announced and nothing whatever about when it stopped. +#[test] +fn f2_an_unfinished_call_closes_without_inventing_a_duration() { + let mut events = Events::new(DARK); + events.event( + &event(EventKind::ToolCall { + name: "read_file".into(), + target: "src/lib.rs".into(), + }), + Duration::ZERO, + ); + + let line = flatten(events.flush()); + assert!(line.contains("read_file"), "{line:?}"); + assert!(line.contains("unfinished"), "{line:?}"); + assert!( + !line.contains('~') && !line.contains("ms"), + "a duration here would be a guess wearing a measurement's clothes: {line:?}", + ); + + // Closed once. A cell that survived its own closing would arrive again in the + // next turn's scrollback. + assert_eq!(events.live(), ""); +} + +/// **F2.** A parallel batch is the ordinary case, not the edge one. +/// +/// `read_batch` announces every call in a batch up front and only then runs any of +/// them, so two calls before one step is what a normal parallel read looks like. A +/// single-slot design is wrong on the first one, and a single shared duration +/// would hide which of the two was slow. +#[test] +fn f2_two_calls_before_one_step_each_close_with_their_own_duration() { + let mut events = Events::new(DARK); + events.event( + &event(EventKind::ToolCall { + name: "read_file".into(), + target: "src/a.rs".into(), + }), + Duration::ZERO, + ); + events.event( + &event(EventKind::ToolCall { + name: "read_file".into(), + target: "src/b.rs".into(), + }), + Duration::from_millis(100), + ); + + let line = rendered_at( + &mut events, + EventKind::Step { + decision: "read src/a.rs; read src/b.rs".into(), + tool_call: "read_file".into(), + tokens: 60, + changed: false, + }, + Duration::from_millis(300), + ); + + let at = |needle: &str| { + line.find(needle) + .unwrap_or_else(|| panic!("{needle:?} is missing from {line:?}")) + }; + // In the order they were announced, each paired with the decision written for + // it — and each measured from its own announcement, so the second call is not + // charged for the time the first one spent. + assert!(at("src/a.rs") < at("src/b.rs"), "{line:?}"); + assert!(at("~300ms") < at("~200ms"), "{line:?}"); +} + /// A rule with no layer is possible and is not the same as no rule at all. It is /// rendered as what it is rather than being rounded to either neighbour. #[test] diff --git a/tests/interrupt.rs b/tests/interrupt.rs index c7c3fc3..e55c474 100644 --- a/tests/interrupt.rs +++ b/tests/interrupt.rs @@ -51,7 +51,10 @@ fn f6_ctrl_c_during_a_turn_interrupts_it_and_keeps_the_partial_output() { // The model streams some of an answer. for word in ["Looking ", "at ", "the ", "parser"] { - app.event(&RunEvent::new(1, 1, EventKind::Token { text: word.into() })); + app.event( + &RunEvent::new(1, 1, EventKind::Token { text: word.into() }), + std::time::Duration::ZERO, + ); } assert_eq!(app.events.live(), "Looking at the parser"); @@ -215,6 +218,9 @@ fn f9_a_streaming_answer_of_any_length_leaves_the_viewport_the_same_size() { fn app_lines(app: &mut App, text: &str) -> usize { let before = app.take_pending().len(); assert_eq!(before, 0, "the caller should have drained first"); - app.event(&RunEvent::new(1, 1, EventKind::Token { text: text.into() })); + app.event( + &RunEvent::new(1, 1, EventKind::Token { text: text.into() }), + std::time::Duration::ZERO, + ); app.take_pending().len() } diff --git a/tests/live.rs b/tests/live.rs index b4311d3..6a5410a 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -21,6 +21,8 @@ //! driven through the real bridge and the real event renderer, editing a real //! file inside the sandbox. +mod support; + use std::sync::{Arc, Mutex}; use io_cli::events::Events; @@ -160,7 +162,7 @@ async fn live_f1_a_real_turn_streams_and_edits_a_file() { let mut renderer = Events::new(DARK); let mut committed = Vec::new(); for event in &events { - committed.extend(renderer.event(event)); + committed.extend(renderer.event(event, std::time::Duration::ZERO)); } committed.extend(renderer.flush()); let transcript: String = committed @@ -252,9 +254,11 @@ async fn live_f10_a_real_turn_stops_and_asks_before_it_writes() { let seen = Arc::new(Mutex::new(Vec::new())); let answering = tokio::spawn({ let seen = Arc::clone(&seen); + // Owned, because the task outlives this borrow of the tempdir. + let root = root.to_path_buf(); async move { while let Some(ask) = asks.recv().await { - let approval = Approval::new(ask); + let approval = Approval::new(ask, &root); let question = approval.ask(); let note = format!( "{} {} — rule {:?}, layer {:?}, {} bytes of content", @@ -344,13 +348,16 @@ async fn live_f10_a_denial_leaves_the_file_alone() { events: Arc::clone(&collected), }; - let answering = tokio::spawn(async move { - let mut denied = 0usize; - while let Some(ask) = asks.recv().await { - Approval::new(ask).answer(Answer::Deny); - denied += 1; + let answering = tokio::spawn({ + let root = root.to_path_buf(); + async move { + let mut denied = 0usize; + while let Some(ask) = asks.recv().await { + Approval::new(ask, &root).answer(Answer::Deny); + denied += 1; + } + denied } - denied }); let result = session @@ -383,7 +390,7 @@ async fn live_f10_a_denial_leaves_the_file_alone() { let mut renderer = Events::new(DARK); let transcript: String = events .iter() - .flat_map(|event| renderer.event(event)) + .flat_map(|event| renderer.event(event, std::time::Duration::ZERO)) .map(|line| { line.spans .iter() @@ -431,15 +438,18 @@ async fn live_allowing_for_the_session_stops_the_reasking_inside_one_turn() { events: Arc::clone(&collected), }; - let answering = tokio::spawn(async move { - let mut targets = Vec::new(); - while let Some(ask) = asks.recv().await { - let approval = Approval::new(ask); - targets.push(approval.ask().target().to_string()); - println!("asked about {}", approval.ask().target()); - approval.answer(Answer::Session); + let answering = tokio::spawn({ + let root = root.to_path_buf(); + async move { + let mut targets = Vec::new(); + while let Some(ask) = asks.recv().await { + let approval = Approval::new(ask, &root); + targets.push(approval.ask().target().to_string()); + println!("asked about {}", approval.ask().target()); + approval.answer(Answer::Session); + } + targets } - targets }); let result = session @@ -479,3 +489,269 @@ async fn live_allowing_for_the_session_stops_the_reasking_inside_one_turn() { targets[0], ); } + +// =========================================================================== +// F12 — a real turn against a live provider edits a file, and every surface +// this release ships is exercised on it. +// +// This replaced the human gate the first three releases each ended at. The +// owner withdrew manual release testing on 2026-08-17; see +// `.ultraship/iterations/US-IO-CLI-0.3.0-I01.yaml`. What a gate proved and this +// cannot is whether a diff is *pleasant* to read, and the release record says so +// rather than implying otherwise. +// +// What it does prove is the thing no assertion over a hand-written hunk can: +// that the hunk io-harness ACTUALLY stores for a real edit, by a real model, +// through the real tool layer, is the one this renderer draws. +// =========================================================================== + +/// The whole cell, as a reader would see it. +fn as_text(lines: &[ratatui::text::Line<'static>]) -> String { + lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") +} + +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f12_a_real_edit_renders_as_the_hunk_the_harness_stored() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + // Several lines, so a one-line edit produces a hunk with real context and + // real `@@` numbers rather than a whole-file replacement that would look the + // same however it was computed. + std::fs::write( + root.join("greeting.rs"), + "fn one() {}\nfn two() {}\nfn three() {}\nfn four() {}\nfn five() {}\n\ + fn six() {}\nfn seven() {}\nfn eight() {}\n", + ) + .expect("the fixture file"); + + 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 (_steer, inbox) = Steer::channel(); + let collected = Arc::new(Mutex::new(Vec::new())); + let observer = Collector { + events: Arc::clone(&collected), + }; + + let result = session + .turn_steered( + "In greeting.rs, change only the body of `fn five` so that it reads \ + `fn five() { println!(\"five\"); }`. Change nothing else. Then say done.", + &provider, + &store, + &policy, + &DenyAll, + &observer, + &inbox, + ) + .await + .expect("the turn runs"); + + println!("outcome: {:?} run {}", result.outcome, result.run_id); + + // ---- F1/F2/F3/F4: the diff, from the store, for a real edit ------------- + let edits = store.edits(result.run_id).expect("the edits are readable"); + println!("{} edit(s) recorded", edits.len()); + assert!( + !edits.is_empty(), + "the model did not edit the file, so this proves nothing about the diff. \ + outcome was {:?}", + result.outcome, + ); + + for edit in &edits { + let text = as_text(&io_cli::diff::cell(edit, &DARK, 120)); + println!("--- step {} · {} ---\n{text}\n---", edit.step, edit.tool); + + assert!( + text.contains(&edit.path), + "the path is on the header: {text}" + ); + match &edit.hunk { + Some(hunk) => { + // The load-bearing assertion of the whole release: what is on + // screen is the harness's stored text, not something io-cli + // recomputed. Every body line survives, in order. + for line in hunk.lines() { + assert!( + text.contains(line), + "the rendered cell dropped a line the store holds: {line:?}", + ); + } + assert!( + text.contains("@@"), + "a stored hunk carries the file's own line numbers: {text}", + ); + } + // Honest about the other case rather than failing on it: an absent + // hunk has three causes and none of them is "nothing changed". + None => assert!( + text.contains("no diff stored"), + "an absent hunk has to say so: {text}", + ), + } + } + + // ---- F11: the whole run as one patch, over OSC 52 ----------------------- + let patch = store.patch(result.run_id).expect("the patch is readable"); + assert!( + !patch.trim().is_empty(), + "a run that edited a file has a patch" + ); + let sequence = io_cli::clipboard::sequence(&patch); + assert!(sequence.starts_with("\x1b]52;c;"), "OSC 52 is malformed"); + assert!(sequence.ends_with('\x07'), "OSC 52 is unterminated"); + let described = io_cli::clipboard::describe(&patch); + println!("clipboard: {described}"); + for claim in ["copied", "success", "done"] { + assert!( + !described.to_lowercase().contains(claim), + "nothing acknowledges an OSC 52 write, so {claim:?} is a claim this \ + product cannot support: {described}", + ); + } + + // ---- F10: the conversation, back into the scrollback -------------------- + let transcript = session.transcript(&store).expect("the transcript reads"); + let rendered = as_text(&io_cli::transcript::lines(&transcript, &DARK)); + println!("--- Ctrl+T ---\n{rendered}\n---"); + assert!( + rendered.contains("greeting.rs") || rendered.contains("five"), + "the transcript should carry what was asked: {rendered}", + ); + + // ---- F5: the expander reads the detail back out of the trace ------------ + let steps = store.steps(result.run_id).expect("the steps read"); + println!("{} step(s) in the trace", steps.len()); + assert!( + !steps.is_empty(), + "a run that edited a file recorded steps, and /expand reads them back", + ); + + // ---- the tool cells, through the real renderer -------------------------- + let events = collected.lock().expect("not poisoned").clone(); + let mut renderer = Events::new(DARK); + let mut committed = Vec::new(); + for (nth, event) in events.iter().enumerate() { + // A distinct age per event, stated by the test rather than measured, so + // each cell's duration is arithmetic this file chose. No clock is read + // here or anywhere under `tests/`. + committed.extend(renderer.event(event, std::time::Duration::from_millis(nth as u64 * 100))); + } + committed.extend(renderer.flush()); + let text = as_text(&committed); + println!("--- as the interface renders it ---\n{text}\n---"); + + let announced = events + .iter() + .filter(|event| matches!(event.kind, EventKind::ToolCall { .. })) + .count(); + println!("{announced} tool call(s) announced"); + if announced > 0 { + assert!( + text.contains("~"), + "a committed tool cell carries the interface's observed duration, \ + marked as an observation: {text}", + ); + } +} + +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f12_an_approval_shows_the_write_as_a_diff_against_the_real_file() { + use io_cli::approval::{self, Approval}; + + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + let target = root.join("notes.txt"); + std::fs::write(&target, "one\ntwo\nthree\nfour\nfive\n").expect("the fixture"); + + 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 whose whole point is that it asks. + let policy = Policy { + layers: Policy::default().layers, + defaults: Posture::AskWrites.defaults(), + }; + let (_steer, inbox) = Steer::channel(); + let (asker, mut asks) = approval::channel(); + + // Answer whatever is asked, and render the overlay for the first write on the + // way past. The run stays paused for exactly as long as the `Ask` is held. + let drawn = Arc::new(Mutex::new(Vec::::new())); + let seen = Arc::clone(&drawn); + let answering = tokio::spawn({ + let root = root.to_path_buf(); + async move { + while let Some(ask) = asks.recv().await { + let approval = Approval::new(ask, &root); + // A viewport with room, so the hunk itself is exercised and not only + // the counts row a four-row session would leave. + let (mut screen, _recorder) = support::screen_of(100, 20, 12); + screen + .draw(|frame| approval.render(frame, frame.area(), &DARK)) + .expect("frame"); + seen.lock() + .expect("not poisoned") + .push(screen.viewport_text().to_string()); + approval.answer(approval::Answer::Once); + } + } + }); + + let result = session + .turn_steered( + "Change the third line of notes.txt from `three` to `THREE`. \ + Change nothing else. Then say done.", + &provider, + &store, + &policy, + &asker, + &io_harness::Ignore, + &inbox, + ) + .await + .expect("the turn runs"); + drop(asker); + answering.await.expect("the answering task did not panic"); + + println!("outcome: {:?}", result.outcome); + let overlays = drawn.lock().expect("not poisoned").clone(); + for overlay in &overlays { + println!("--- approval overlay ---\n{overlay}\n---"); + } + assert!( + !overlays.is_empty(), + "the ask-before-writes posture did not ask, so the approval diff was \ + never exercised. outcome was {:?}", + result.outcome, + ); + + // The counts are the row that always survives, and they are the decision: + // a write that touches one line is not the same decision as one that + // rewrites the file. + let any = overlays.join("\n"); + assert!( + any.contains('+') && any.contains('-'), + "the overlay showed no change at all: {any}", + ); + assert!( + !any.contains("+one") || any.contains("-one"), + "an unchanged line was drawn as an addition, which means the old side \ + was empty and the write reads as a whole-file rewrite: {any}", + ); +} diff --git a/tests/narrow.rs b/tests/narrow.rs index a815b01..950d017 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -109,7 +109,7 @@ async fn n5_the_approval_overlay_fits_eighty_columns() { }); let ask = asks.recv().await.expect("the question arrived"); - let approval = Approval::new(ask); + let approval = Approval::new(ask, std::path::Path::new("")); let (mut screen, _recorder) = support::screen(WIDTH, HEIGHT); screen .draw(|frame| approval.render(frame, frame.area(), &DARK)) @@ -130,7 +130,15 @@ async fn n5_the_approval_overlay_fits_eighty_columns() { ); // And so must the act and the target, in some form. assert!(viewport.contains("write"), "{viewport:?}"); - assert!(viewport.contains('…'), "something was fitted: {viewport:?}"); + // Something was cut and the overlay said so. Two markers can carry that: `…` + // when a single value was shortened to fit, and `⋯` when whole rows of a + // change were left out. 0.3.0 made the second the usual one here — the + // overlay's diff row leads with the counts rather than repeating the path, so + // the path no longer needs shortening and the rows do. + assert!( + viewport.contains('…') || viewport.contains('⋯'), + "something was cut and nothing said so: {viewport:?}", + ); // The two facts no other core records must survive the narrow form. A widget // that "fits" by letting ratatui clip the row has not fitted anything — it has // silently cut the half of the sentence this release exists to show. @@ -143,3 +151,126 @@ async fn n5_the_approval_overlay_fits_eighty_columns() { let decision = deciding.await.expect("the approver did not panic"); assert!(matches!(decision, Decision::Deny { .. })); } + +/// **N5.** A diff at eighty columns, and the emphasis floor below a hundred. +/// +/// Nothing here is about truncation — a committed line wraps, which +/// `f9_a_line_wider_than_the_terminal_wraps_rather_than_truncating` already +/// proves for the renderer. What is asserted is that the cell's own facts survive +/// the narrow form, and that word-level emphasis gives way to the line rather +/// than leaving a bolded fragment somewhere in the middle of a three-row wrap. +mod diff { + use io_cli::diff::{cell, EMPHASIS_FLOOR}; + use io_cli::theme::DARK; + use io_harness::Edit; + use ratatui::style::Modifier; + + /// A path longer than eighty columns on its own, and body lines wider still. + fn wide_edit() -> Edit { + Edit { + step: 1, + tool: "edit_file".to_string(), + path: "crates/some-rather-long-crate-name/src/subsystem/module/implementation.rs" + .to_string(), + lines_added: 1, + lines_removed: 1, + hunk: Some(format!( + "@@ -12,3 +12,3 @@\n \ + fn one() {{}}\n\ + -let value = {};\n\ + +let value = {};\n", + "x".repeat(200), + "y".repeat(200), + )), + } + } + + fn text(edit: &Edit, width: u16) -> String { + cell(edit, &DARK, width) + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") + } + + #[test] + fn n5_the_header_keeps_its_facts_at_eighty_columns() { + let edit = wide_edit(); + let rendered = text(&edit, super::WIDTH); + + // The path is longer than the terminal, so the counts sit past the wrap. + // 0.2.0 paid for the lesson that a row can "fit" because ratatui clipped + // the half of the sentence that mattered — here the facts must be in the + // cell whatever the wrap does with them. + assert!(rendered.contains(&edit.path), "{rendered}"); + assert!( + rendered.contains("+1"), + "the additions are gone: {rendered}" + ); + assert!(rendered.contains("-1"), "the removals are gone: {rendered}"); + assert!(rendered.contains("edit_file"), "{rendered}"); + } + + #[test] + fn n5_nothing_in_the_body_is_dropped_at_eighty_columns() { + let rendered = text(&wide_edit(), super::WIDTH); + assert_eq!( + rendered.matches(&"x".repeat(200)).count(), + 1, + "the removed line was cut rather than left to wrap: {rendered}", + ); + assert_eq!(rendered.matches(&"y".repeat(200)).count(), 1, "{rendered}"); + assert!(rendered.contains("@@ -12,3 +12,3 @@"), "{rendered}"); + } + + #[test] + fn n5_below_the_floor_the_emphasis_is_the_line_and_not_a_word() { + let edit = wide_edit(); + + let wide = cell(&edit, &DARK, EMPHASIS_FLOOR); + let emphasised = |lines: &[ratatui::text::Line<'static>]| { + lines + .iter() + .flat_map(|line| line.spans.iter()) + .filter(|span| span.style.add_modifier.contains(Modifier::BOLD)) + .count() + }; + assert!( + emphasised(&wide) > 0, + "at the floor itself the word-level emphasis still applies", + ); + + let narrow = cell(&edit, &DARK, EMPHASIS_FLOOR - 1); + assert_eq!( + emphasised(&narrow), + 0, + "below the floor a changed line takes the whole wash, so no fragment \ + of it is bolded inside a wrap", + ); + + // And the line still reads as changed: the wash is the carrier now. + let removed = narrow + .iter() + .find(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + .trim_start() + .starts_with('-') + }) + .expect("a removed line"); + assert!( + removed + .spans + .iter() + .any(|s| s.style.fg == Some(DARK.diff_delete)), + "a line with no emphasis must still be coloured as removed", + ); + } +} diff --git a/tests/status.rs b/tests/status.rs index 2aee696..ba4393d 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -227,8 +227,8 @@ fn f9_a_field_with_nothing_behind_it_is_absent_rather_than_zero() { #[test] fn the_token_field_is_the_session_and_not_the_last_step() { let mut app = App::new(DARK, "opus-5"); - app.event(&step(1, 1_200)); - app.event(&step(2, 300)); + app.event(&step(1, 1_200), std::time::Duration::ZERO); + app.event(&step(2, 300), std::time::Duration::ZERO); let line = app.status.line(120, &DARK).to_string(); assert!( @@ -244,11 +244,14 @@ fn the_token_field_is_the_session_and_not_the_last_step() { #[test] fn the_containment_field_carries_the_backend_and_not_only_the_mode() { let mut app = App::new(DARK, "opus-5"); - app.event(&event(EventKind::Contained { - mode: "workspace-write".into(), - backend: "portable-floor".into(), - roots: 0, - })); + app.event( + &event(EventKind::Contained { + mode: "workspace-write".into(), + backend: "portable-floor".into(), + roots: 0, + }), + std::time::Duration::ZERO, + ); let line = app.status.line(120, &DARK).to_string(); assert!(line.contains("workspace-write"), "{line:?}"); @@ -266,11 +269,14 @@ fn the_context_field_appears_when_a_fold_reports_one() { let mut app = App::new(DARK, "opus-5"); assert!(!app.status.line(120, &DARK).to_string().contains("ctx")); - app.event(&event(EventKind::Compacted { - through_step: 4, - before_tokens: 11_000, - after_tokens: 6_000, - })); + app.event( + &event(EventKind::Compacted { + through_step: 4, + before_tokens: 11_000, + after_tokens: 6_000, + }), + std::time::Duration::ZERO, + ); let line = app.status.line(120, &DARK).to_string(); assert!( @@ -287,12 +293,15 @@ fn the_context_field_appears_when_a_fold_reports_one() { fn the_new_fields_drop_from_the_right_rather_than_wrapping() { let mut app = App::new(DARK, "opus-5"); app.set_posture(Some(io_cli::settings::Posture::Workspace)); - app.event(&step(1, 12_400)); - app.event(&event(EventKind::Contained { - mode: "workspace-write".into(), - backend: "seatbelt".into(), - roots: 2, - })); + app.event(&step(1, 12_400), std::time::Duration::ZERO); + app.event( + &event(EventKind::Contained { + mode: "workspace-write".into(), + backend: "seatbelt".into(), + roots: 2, + }), + std::time::Duration::ZERO, + ); let wide = app.status.line(160, &DARK).to_string(); assert!(wide.contains("seatbelt"), "{wide:?}"); diff --git a/tests/surfaces.rs b/tests/surfaces.rs new file mode 100644 index 0000000..f52190c --- /dev/null +++ b/tests/surfaces.rs @@ -0,0 +1,527 @@ +//! F5, F10 and F11 — the three surfaces that answer "show me more" and "give me +//! this text", asserted at the boundary each of them actually crosses. +//! +//! The three share one premise and it is the reason they are in one file: **the +//! viewport is four rows and never grows**. Everything that shows more of +//! something is therefore written *upward*, into the terminal's own scrollback, +//! where the terminal's search, selection and copy-mode already work — and +//! everything that hands text to the reader's own machine goes out as an escape +//! sequence, which is neither scrollback nor viewport. A regression in any of the +//! three looks the same from inside a rendered buffer: nothing. So the assertions +//! below are made against the bytes that leave the process and against the +//! viewport buffer at rest, never against a value the renderer handed itself. +//! +//! Order matters on a rendered line and is asserted by position rather than by +//! membership. A line that reads `turn 2 · left behind by a branch · do it with a +//! blue-green cutover` contains every substring an inside-out line contains, and +//! a `contains` assertion is exactly as green for it. + +mod support; + +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use io_cli::app::{App, Command}; +use io_cli::clipboard; +use io_cli::commands::{parse, Action, Copied}; +use io_cli::diff::MAX_BODY_LINES; +use io_cli::term::VIEWPORT_HEIGHT; +use io_cli::theme::DARK; +use io_cli::transcript::{self, BRANCHED_AWAY}; +use io_harness::provider::{CompletionRequest, CompletionResponse}; +use io_harness::{ApproveAll, Edit, Policy, Provider, Session, Store}; +use ratatui::text::Line; +use ratatui::widgets::Paragraph; + +// --------------------------------------------------------------------------- +// F5 — expansion commits upward, and the viewport never grows. +// --------------------------------------------------------------------------- + +/// Far more rows than the terminal under test has, so a commit that tried to +/// make room by growing the viewport would have to grow it past the screen. +const COMMITTED_ROWS: usize = 200; + +/// What the viewport draws while the commit is happening. Deliberately shares no +/// word with the committed lines, so "the viewport holds only this" is a real +/// assertion rather than a coincidence of vocabulary. +const COMPOSER: &str = "> the prompt being typed"; + +#[test] +fn f5_a_large_commit_reaches_the_terminal_and_leaves_the_viewport_at_its_fixed_height() { + // 80x24 on purpose: the smallest supported terminal, where a viewport that + // grew by two hundred rows could not even be drawn. The failure this prevents + // is a "show me more" path that opens a pane instead of committing — which on + // this renderer means the transcript stops being the terminal's own buffer and + // starts being something this process has to redraw and can lose. + let (mut screen, recorder) = support::screen(80, 24); + let before = screen.terminal_mut().get_frame().area().height; + assert_eq!( + before, VIEWPORT_HEIGHT, + "the fixture is only meaningful if the viewport starts at its fixed height", + ); + + let body: Vec> = (0..COMMITTED_ROWS) + .map(|row| Line::from(format!("expanded detail row {row}"))) + .collect(); + screen.commit(&body).expect("a commit of two hundred rows"); + screen + .draw(|frame| { + let area = frame.area(); + frame.render_widget(Paragraph::new(COMPOSER), area); + }) + .expect("a frame after the commit"); + + assert_eq!( + screen.terminal_mut().get_frame().area().height, + before, + "the viewport grew to hold committed content; it is fixed at {VIEWPORT_HEIGHT} rows \ + and everything that shows more has to go above it", + ); + + // The content reached the terminal — which is what "in the scrollback" means + // for a renderer that never owns the rows above its viewport. + let text = recorder.text(); + for row in [0, COMMITTED_ROWS / 2, COMMITTED_ROWS - 1] { + let needle = format!("expanded detail row {row}"); + assert!( + text.contains(&needle), + "{needle:?} never left the process, so it is in no scrollback", + ); + } + + // And none of it is in the viewport, which holds what the frame drew and + // nothing else. This is the half that fails when expansion is rendered into + // the viewport instead of committed above it. + let viewport = screen.viewport_text(); + assert!( + viewport.contains(COMPOSER), + "the viewport should hold the composer, but it holds {viewport:?}", + ); + assert!( + !viewport.contains("expanded detail row"), + "committed content is still in the live viewport: {viewport:?}", + ); +} + +/// The `@@` header plus this many additions, which is far past [`MAX_BODY_LINES`] +/// and is what makes the cap observable at all. +const HUNK_ADDITIONS: usize = 900; + +/// A hunk taller than the cap, on a path syntect has a grammar for, so the cell +/// under test is the one a real edit produces rather than a shortest path through +/// the renderer. +fn tall_edit() -> Edit { + let mut hunk = format!("@@ -1,{HUNK_ADDITIONS} +1,{HUNK_ADDITIONS} @@\n"); + for row in 0..HUNK_ADDITIONS { + hunk.push_str(&format!("+row {row} of a very large change\n")); + } + Edit { + step: 1, + tool: "write_file".to_string(), + path: "notes.txt".to_string(), + lines_added: HUNK_ADDITIONS as u64, + lines_removed: 0, + hunk: Some(hunk), + } +} + +#[test] +fn f5_a_hunk_past_the_cap_is_bounded_and_says_how_many_lines_it_cut() { + let edit = tall_edit(); + let body_rows = edit + .hunk + .as_deref() + .expect("the fixture has a hunk") + .lines() + .count(); + assert!( + body_rows > MAX_BODY_LINES, + "the fixture only tests the cap if it exceeds it: {body_rows} rows against {MAX_BODY_LINES}", + ); + + let rendered = rows(&io_cli::diff::cell(&edit, &DARK, 120)); + + // Bounded. The cap exists because highlighting runs on the loop that also + // delivers keystrokes and because `insert_before` takes a `u16` height: an + // uncapped cell makes `Ctrl+C` unreachable for as long as the parse takes. + assert!( + rendered.len() < body_rows, + "the whole hunk was drawn — {} lines — so the cap did nothing", + rendered.len(), + ); + assert!( + rendered.len() <= MAX_BODY_LINES + 3, + "the cell is {} lines; the cap allows {MAX_BODY_LINES} body rows plus a header, \ + the notice and the trailing blank", + rendered.len(), + ); + + // And it says what it cut. A cap that truncates silently is the defect: the + // reader cannot tell a change that ended from a change that was cut off, and + // the part they cannot see is the part they went looking for. + let cut = body_rows - MAX_BODY_LINES; + let counted = cut.to_string(); + let notice_at = rendered + .iter() + .rposition(|row| !row.trim().is_empty()) + .expect("a cell has at least a header"); + let notice = &rendered[notice_at]; + assert!( + notice.contains(format!("{cut} more lines").as_str()), + "the last line of a capped cell must count what it cut, and this one reads {notice:?}", + ); + + // By position within the line: the count comes before the words that say + // where the rest of it went. A line reading "the whole of it is in the trace + // — 401 more lines" contains both and answers the reader's question in the + // wrong order. + let count_at = notice + .find(counted.as_str()) + .expect("the count is on the line"); + let trace_at = notice + .find("trace") + .expect("the notice says where the rest of the change is"); + assert!( + count_at < trace_at, + "the cut count must lead the line: {notice:?}", + ); + + // The notice is at the end of the body, under the rows it applies to, rather + // than a header nobody reads in context. + assert!( + rendered[notice_at - 1].contains("row "), + "the notice should sit under the last drawn row, but follows {:?}", + rendered[notice_at - 1], + ); +} + +// --------------------------------------------------------------------------- +// F10 — Ctrl+T commits the whole conversation, branched-away turns included. +// --------------------------------------------------------------------------- + +/// What is half-typed when the transcript key is pressed. +const HALF_TYPED: &str = "a thought i was in the middle of"; + +#[test] +fn f10_ctrl_t_asks_for_the_transcript_and_leaves_the_composer_holding_what_was_typed() { + let mut app = App::new(DARK, "opus-5"); + for character in HALF_TYPED.chars() { + app.key(KeyEvent::new(KeyCode::Char(character), KeyModifiers::NONE)); + } + + let command = app.key(KeyEvent::new(KeyCode::Char('t'), KeyModifiers::CONTROL)); + assert_eq!( + command, + Command::Transcript, + "Ctrl+T is the one key that puts the conversation back into the scrollback", + ); + + // The failure this prevents: a key that reaches for the whole conversation and + // eats the prompt on the way, so looking something up costs the sentence being + // written. Nothing about committing the transcript touches the composer, and + // this is what keeps it that way. + assert_eq!( + app.composer.text(), + HALF_TYPED, + "the composer lost what was typed when the transcript was asked for", + ); +} + +/// The three prompts the branched session is built from. The second is the one +/// the branch leaves behind, and it is the only turn in the fixture that +/// `Session::history` would no longer return. +const PLAN: &str = "sketch the cutover"; +const BRANCHED: &str = "take the whole service down for it"; +const KEPT: &str = "keep it up and drain the queue instead"; + +/// Answers every turn with one line and calls nothing, so the turns are real and +/// their content is not. +struct Talker; + +impl Provider for Talker { + async fn complete( + &self, + _request: CompletionRequest, + ) -> io_harness::Result { + Ok(CompletionResponse { + text: Some("here is the sketch".into()), + ..Default::default() + }) + } +} + +async fn say(session: &mut Session, store: &Store, prompt: &str) -> i64 { + session + .turn(prompt, &Talker, store, &Policy::permissive(), &ApproveAll) + .await + .expect("a scripted turn cannot fail") + .turn_id +} + +/// A real session of three turns with the middle one branched away. +/// +/// Built rather than stated: `io_harness::Transcript` and `TranscriptTurn` are +/// both `#[non_exhaustive]`, so neither can be constructed by a struct literal +/// from outside the harness. A hand-made value would also agree with the renderer +/// by construction, which is the opposite of what F10 needs to know — the +/// question is whether the renderer agrees with what the *database* recorded. +async fn branched() -> (tempfile::TempDir, io_harness::Transcript) { + let dir = tempfile::tempdir().expect("a temp dir"); + let store = Store::memory().expect("an in-memory store"); + let mut session = Session::open(&store, dir.path()).expect("a session"); + + let first = say(&mut session, &store, PLAN).await; + say(&mut session, &store, BRANCHED).await; + session + .branch_from(&store, first) + .expect("the first turn is branchable"); + say(&mut session, &store, KEPT).await; + + let transcript = session.transcript(&store).expect("a transcript"); + (dir, transcript) +} + +#[tokio::test] +async fn f10_the_transcript_shows_the_branched_away_turn_and_labels_it_after_its_own_prompt() { + let (_dir, transcript) = branched().await; + assert_eq!( + transcript.turns.iter().filter(|turn| turn.on_path).count(), + 2, + "the fixture is only meaningful if one of the three turns is off the path", + ); + + let rendered = rows(&transcript::lines(&transcript, &DARK)); + + // Rendered at all. This turn is the one thing in the whole product that only + // this surface can show, and a turn that was dropped looks exactly like a turn + // that never happened. + let branched = row_with(&rendered, BRANCHED); + assert!( + branched.contains(BRANCHED_AWAY), + "the branched-away turn must say so in words — a tone is gone under NO_COLOR \ + and gone again once the line is copied out of the terminal: {branched}", + ); + + // By position, not by membership: content, then the branch label, then the id. + // A row carrying all three in any other order passes a `contains` and reads + // inside out, with metadata in the column a reader skims for what was said. + let prompt_at = branched.find(BRANCHED).expect("the prompt is on its row"); + let label_at = branched + .find(BRANCHED_AWAY) + .expect("the label is on its row"); + let id_at = branched + .find("turn ") + .expect("the turn id rides the same row"); + assert!( + prompt_at < label_at && label_at < id_at, + "content, then the label, then the id: {branched}", + ); + + // And the turn the model can still see carries no label. Without this half, + // a renderer that labelled every row would pass everything above. + let kept = row_with(&rendered, KEPT); + assert!( + !kept.contains(BRANCHED_AWAY), + "a turn still on the path must not be labelled: {kept}", + ); +} + +// --------------------------------------------------------------------------- +// F11 — /copy writes OSC 52 and claims nothing about the clipboard. +// --------------------------------------------------------------------------- + +#[test] +fn f11_copy_resolves_to_the_answer_and_copy_diff_and_copy_patch_both_to_the_patch() { + assert_eq!( + parse("copy", &DARK), + Action::Copy(Copied::Answer), + "a bare /copy is the last answer, which is what a reader who just read one wants", + ); + // Two spellings of one thing, because a reader who has just been shown a diff + // types the word they were shown. A parser that knows only one of them fails + // silently into "copy the answer", which puts the wrong text on the clipboard + // rather than reporting that the command was not understood. + for spelling in ["copy diff", "copy patch"] { + assert_eq!( + parse(spelling, &DARK), + Action::Copy(Copied::Diff), + "/{spelling} must reach the patch", + ); + } +} + +/// The text put on the clipboard. It holds spaces, so it cannot appear inside the +/// base64 form of itself — which is what makes "the plaintext never left the +/// process" an assertion rather than a hope. +const PAYLOAD: &str = "the answer the agent gave, verbatim"; + +#[test] +fn f11_the_bytes_that_leave_the_process_are_an_osc_52_sequence_carrying_the_payload() { + let (mut screen, recorder) = support::screen(80, 24); + screen + .escape(&clipboard::sequence(PAYLOAD)) + .expect("the sequence is written"); + + // Asserted over the bytes rather than over `sequence`'s return value: what is + // under test is what the terminal emulator at the far end of an SSH connection + // receives, and a renderer that built the right string and never flushed it + // would satisfy every assertion made against the string. + let bytes = recorder.bytes(); + let introducer = b"\x1b]52;c;"; + let start = find(&bytes, introducer) + .unwrap_or_else(|| panic!("no OSC 52 introducer in {:?}", recorder.text())) + + introducer.len(); + let end = start + + bytes[start..] + .iter() + .position(|byte| *byte == 0x07) + .expect("the sequence is terminated by BEL, which the whole xterm lineage accepts"); + + let encoded = std::str::from_utf8(&bytes[start..end]).expect("base64 is ASCII"); + assert_eq!( + decode(encoded), + PAYLOAD.as_bytes(), + "the payload does not survive the round trip, so the reader's paste is not what they copied", + ); + + // The encoding is not decoration: an OSC string ends at the first BEL or ST, + // and a transcript can carry both. A payload written raw would terminate its + // own sequence early and print the remainder of itself on screen. + assert!( + !encoded.contains(['\x07', '\x1b']), + "the encoded middle carries a byte that ends an OSC string early", + ); +} + +#[test] +fn f11_the_message_states_a_size_and_never_claims_the_clipboard_was_set() { + // Two and a half thousand bytes, so the size is a stated fact with a unit + // rather than a number that could have come from anywhere. + let payload = "x".repeat(2_500); + let said = clipboard::describe(&payload); + assert!( + said.contains("2.5 kB"), + "the message must state the size — it is the one fact that makes a paste that came \ + up empty diagnosable against a terminal's own limit: {said}", + ); + + // No terminal answers an OSC 52 write. tmux drops it without `set -g + // set-clipboard on` and several emulators cap the payload silently, so a + // success word here is a claim this process never observed, printed at exactly + // the moment the reader goes to paste and finds the old contents. + let lowered = said.to_lowercase(); + for claim in ["copied", "success", "done"] { + assert!( + !lowered.contains(claim), + "{claim:?} claims an acknowledgement that does not exist on the wire: {said}", + ); + } +} + +#[test] +fn f11_the_clipboard_sequence_goes_to_the_terminal_and_not_into_the_scrollback() { + let (mut screen, recorder) = support::screen(80, 24); + screen + .escape(&clipboard::sequence(PAYLOAD)) + .expect("the sequence is written"); + screen + .draw(|frame| { + let area = frame.area(); + frame.render_widget(Paragraph::new(COMPOSER), area); + }) + .expect("a frame after the sequence"); + + // An OSC 52 write is a message to the terminal emulator, not content for the + // screen. Routed through `commit` it would become a line of the transcript the + // reader scrolls past — and, because `commit` renders through a widget, the + // escape bytes would be drawn as text rather than acted on. The payload's + // plaintext holds spaces, which base64 never emits, so its absence here is + // proof it was not committed. + let text = recorder.text(); + assert!( + !text.contains(PAYLOAD), + "the payload reached the terminal as text, which means it was committed rather \ + than sent as an escape sequence", + ); + assert!( + text.contains("\x1b]52;c;"), + "the sequence itself never left the process", + ); + + let viewport = screen.viewport_text(); + assert!( + !viewport.contains(PAYLOAD) && !viewport.contains("52;c;"), + "the clipboard sequence is in the viewport: {viewport:?}", + ); +} + +// --------------------------------------------------------------------------- +// Helpers. +// --------------------------------------------------------------------------- + +/// Every rendered line as its own string, spans concatenated. +/// +/// The renderer's unit is the line and every assertion above is about what a +/// single line holds and in what order. Joining the whole output into one blob +/// would make "on the same line" and "somewhere in the output" the same question. +fn rows(lines: &[Line<'_>]) -> Vec { + lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect() +} + +/// The one row holding `needle`, or a failure naming what was rendered instead. +fn row_with<'a>(rows: &'a [String], needle: &str) -> &'a String { + rows.iter() + .find(|row| row.contains(needle)) + .unwrap_or_else(|| panic!("nothing rendered for {needle:?}; got {rows:#?}")) +} + +/// Where `needle` starts in `haystack`, over bytes rather than text — the byte +/// stream holds escape sequences that are not valid on a `char` boundary basis to +/// search through as a string. +fn find(haystack: &[u8], needle: &[u8]) -> Option { + haystack + .windows(needle.len()) + .position(|window| window == needle) +} + +/// Standard base64, the `+/` alphabet of RFC 4648 — the one every terminal +/// emulator implements. Spelled out here rather than imported from the crate +/// under test: decoding with the encoder's own table would assert only that the +/// table is self-consistent, and a URL-safe `-_` payload decodes to different +/// bytes in a real terminal rather than failing loudly. +const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +/// Decode a base64 string back to the bytes it was made from. +fn decode(encoded: &str) -> Vec { + let mut bits: u32 = 0; + let mut held = 0; + let mut out = Vec::new(); + for byte in encoded.bytes() { + if byte == b'=' { + break; + } + let index = ALPHABET + .iter() + .position(|character| *character == byte) + .unwrap_or_else(|| { + panic!( + "{:?} is not a standard base64 character, so a terminal would decode this \ + sequence to something other than what was copied", + byte as char, + ) + }); + bits = (bits << 6) | index as u32; + held += 6; + if held >= 8 { + held -= 8; + out.push((bits >> held) as u8); + } + } + out +} diff --git a/tests/transcript.rs b/tests/transcript.rs new file mode 100644 index 0000000..2ccdb0c --- /dev/null +++ b/tests/transcript.rs @@ -0,0 +1,179 @@ +//! The whole conversation, committed to scrollback. +//! +//! The discriminating assertion is the first one: a session that branched holds a +//! turn `Session::history` no longer returns, and this renderer is the only +//! surface in the product that can show it. A renderer built from the path passes +//! every other assertion here and fails that one — silently, because a turn that +//! is missing looks exactly like a turn that never happened. +//! +//! `io_harness::Transcript` and `io_harness::TranscriptTurn` are both +//! `#[non_exhaustive]`, so neither can be built by a struct literal from outside +//! the harness — not even field by field. The transcript under test is therefore a +//! real one: a scripted provider drives real turns into an in-memory store, and +//! `Session::branch_from` puts one of them off the path the same way an operator +//! would. That also means these assertions are about what the harness actually +//! records, rather than about a hand-made value that agrees with the renderer by +//! construction. + +use io_cli::theme::DARK; +use io_cli::transcript; +use io_harness::provider::{CompletionRequest, CompletionResponse}; +use io_harness::{ApproveAll, Policy, Provider, Session, Store}; + +/// The three prompts the branched session is built from. The second is the one +/// the branch leaves behind. +const PLAN: &str = "draft a migration plan"; +const BRANCHED: &str = "do it with a blue-green cutover"; +const KEPT: &str = "do it with a read-only window instead"; + +/// The words `src/transcript.rs` labels a branched-away turn with, spelled out +/// here rather than imported. Importing the constant would assert that the +/// renderer agrees with itself; what matters is that the label is a phrase a +/// reader understands without knowing this product's vocabulary. +const LABEL: &str = "left behind by a branch"; + +/// Answers every turn with one line and never calls anything. +struct Talker; + +impl Provider for Talker { + async fn complete( + &self, + _request: CompletionRequest, + ) -> io_harness::Result { + Ok(CompletionResponse { + text: Some("here is the plan".into()), + ..Default::default() + }) + } +} + +/// Every rendered line as its own string, spans concatenated. The renderer's unit +/// is the line, and every assertion below is about what a single line holds and in +/// what order — joining them all into one blob would make "on the same line" and +/// "somewhere in the output" indistinguishable. +fn rows(lines: &[ratatui::text::Line<'_>]) -> Vec { + lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect() +} + +/// The one row holding `needle`, or a failure naming what was rendered instead. +fn row_with<'a>(rows: &'a [String], needle: &str) -> &'a String { + rows.iter() + .find(|row| row.contains(needle)) + .unwrap_or_else(|| panic!("nothing rendered for {needle:?}; got {rows:#?}")) +} + +async fn say(session: &mut Session, store: &Store, prompt: &str) -> i64 { + session + .turn(prompt, &Talker, store, &Policy::permissive(), &ApproveAll) + .await + .expect("a scripted turn cannot fail") + .turn_id +} + +/// A plan, an answer the operator changed their mind about, and the answer they +/// took instead — so the middle turn is off the path and the other two are on it. +async fn branched() -> (tempfile::TempDir, io_harness::Transcript) { + let dir = tempfile::tempdir().expect("a temp dir"); + let store = Store::memory().expect("an in-memory store"); + let mut session = Session::open(&store, dir.path()).expect("a session"); + + let first = say(&mut session, &store, PLAN).await; + say(&mut session, &store, BRANCHED).await; + session + .branch_from(&store, first) + .expect("the plan turn is branchable"); + say(&mut session, &store, KEPT).await; + + let transcript = session.transcript(&store).expect("a transcript"); + (dir, transcript) +} + +#[tokio::test] +async fn the_turn_a_branch_left_behind_is_rendered_and_labelled_in_words() { + let (_dir, transcript) = branched().await; + assert_eq!( + session_path_len(&transcript), + 2, + "the fixture is only meaningful if one of the three turns is off the path", + ); + + let rendered = rows(&transcript::lines(&transcript, &DARK)); + + // Rendered at all. `Session::history` would not have returned this turn. + let branched = row_with(&rendered, BRANCHED); + // And labelled in words on its own line, not merely toned: a colour is gone + // under `NO_COLOR` and gone again when the line is copied out of the terminal. + assert!( + branched.contains(LABEL), + "the branched-away turn must say so in words: {branched}", + ); + + let kept = row_with(&rendered, KEPT); + assert!( + !kept.contains(LABEL), + "a turn the model can still see must not be labelled: {kept}", + ); +} + +#[tokio::test] +async fn every_turn_that_was_asked_appears() { + let (_dir, transcript) = branched().await; + let rendered = rows(&transcript::lines(&transcript, &DARK)); + + for turn in &transcript.turns { + row_with(&rendered, &turn.prompt); + } + assert_eq!(transcript.turns.len(), 3, "three turns were taken"); +} + +#[tokio::test] +async fn the_prompt_comes_before_the_turn_id_on_its_line() { + let (_dir, transcript) = branched().await; + let rendered = rows(&transcript::lines(&transcript, &DARK)); + + for turn in &transcript.turns { + let row = row_with(&rendered, &turn.prompt); + let id = format!("turn {}", turn.turn_id); + let prompt_at = row.find(turn.prompt.as_str()).expect("the prompt is here"); + let id_at = row + .find(id.as_str()) + .unwrap_or_else(|| panic!("no {id} on the line holding its prompt: {row}")); + // By position, not by membership: a line that reads "turn 2 · draft a + // migration plan" contains both and is inside out, and a `contains` + // assertion is exactly as green for it. + assert!( + prompt_at < id_at, + "content before metadata: {row} has the id at {id_at} and the prompt at {prompt_at}", + ); + } +} + +#[test] +fn an_empty_transcript_renders_a_sentence() { + let dir = tempfile::tempdir().expect("a temp dir"); + let store = Store::memory().expect("an in-memory store"); + let session = Session::open(&store, dir.path()).expect("a session"); + let transcript = session.transcript(&store).expect("a transcript"); + assert!(transcript.turns.is_empty(), "nothing has been asked yet"); + + let rendered = rows(&transcript::lines(&transcript, &DARK)); + let sentence = row_with(&rendered, "no turns"); + assert!( + sentence.ends_with('.'), + "a sentence, so the reader knows the key worked: {sentence}", + ); +} + +/// How many turns the model can still see. Read through the transcript itself so +/// the fixture's premise is checked with the same data the renderer is given. +fn session_path_len(transcript: &io_harness::Transcript) -> usize { + transcript.turns.iter().filter(|turn| turn.on_path).count() +}