diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e526b5..c786dee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,75 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.27.0] - 2026-08-28 + +The store stops being something you cannot see, undo stops being +all-or-nothing, and the work stops ending when the terminal closes. + +**`/store` says what the run store is holding, and three verbs change it.** +`~/.io-cli/runs.db` has held every session, run, step, event, provider call, +snapshot and restore point since 0.15.0, with no retention policy, no rotation +and no way to look at it. The page reports the file's own page arithmetic — +what it costs on disk, what is already free inside it, and what each session in +it holds. `/store rm ` removes one session, `/store sweep ` removes +every session created before a timestamp, and `/store compact` returns the free +pages to the filesystem. Each descends into a confirmation whose first row is +"leave it", which is the row the cursor starts on. + +**A deletion does not shrink the file, and io says so.** SQLite frees pages into +the database rather than out of it, so a removal moves bytes into the free space +*inside* the file and the file on disk stays the size it was. A `VACUUM` is the +only reclamation available, because every store this product has created was made +without `auto_vacuum` — so `/store compact` is a thing you ask for, it needs +roughly the file's own size in free disk space while it runs, and it reports the +bytes the file actually shrank by rather than an inference from the freelist. + +A sweep refuses a session that still holds a resumable run, and names it. It asks +you to agree to the rule rather than to a count, because io-harness exposes no +reader for the column the sweep filters on and the nearest substitute would +under-state what is about to go — filed as io-harness#216. The figures are +reported the moment it finishes, refusals included. + +**`/undo` is the size of the mistake.** `/undo ` puts one file back, +`/undo step ` reverse-applies one step's diff, and a bare `/undo` is the whole +turn — the same thing the rewind chord has always done, now reaching the same +implementation. One file has four possible answers and they read as four +different sentences: it came back, it was removed because the run created it, or +nothing changed — for either of two different reasons. + +Undoing a step is order-sensitive: a later step still standing on the same lines +makes the revert stale, io-harness leaves the file alone rather than +fuzzy-matching it, and io says why. A restore does not know about an edit you +made after the turn, and the confirmation says that before you agree to it. + +**`/export` writes the conversation as markdown, and `/export trace` writes one +run's canonical trace.** Both go into the workspace under the session's own path +policy, and an existing file is refused rather than overwritten. The trace is +written exactly as io-harness produced it — not parsed, not reserialised, not +pretty-printed — because being canonical is the whole of what it is for. + +**Undoing a turn now announces itself.** `EventKind::Rewound` and +`EventKind::Reverted` are emitted only by the observed forms of io-harness's +rewind calls, and this product had called the plain ones since 0.4.0 — so neither +event had ever fired, and the code drawing one of them had been unreachable since +the day it was written. + +**One event that reached nobody now has a line.** A read started before the model +had finished asking, and thrown away — work that was paid for and not used. It +draws only when something was discarded, because a line in every transcript +saying nothing went wrong is a line nobody reads. The other eight silences in +this interface were reviewed in the same pass and keep their routes, which are +better arguments than drawing them would be. + +**`/contain` moves from *this turn* to *the session*.** It is a posture that +changes how every later turn is driven rather than something that acts on the +turn just finished, and moving it is what makes room for `/undo` without widening +the ten-command bound. `Inspect` reaches ten with `/store` and `/export`. + +No configuration key is added, nothing is read at startup that was not read +before, and no model can reach any of the store operations. + + ## [0.26.0] - 2026-08-28 A turn asks for the model, the vendor and the amount of thinking the work @@ -2191,6 +2260,7 @@ client, tool, sandbox, policy engine or session store of its own. - There is no crates.io publish and `cargo install` is not an install path. - No test in this release asserts on wall-clock time. -[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.26.0...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.27.0...HEAD +[0.27.0]: https://github.com/initorigin/io-cli/releases/tag/v0.27.0 [0.1.1]: https://github.com/initorigin/io-cli/releases/tag/v0.1.1 [0.1.0]: https://github.com/initorigin/io-cli/releases/tag/v0.1.0 diff --git a/Cargo.lock b/Cargo.lock index 2fde11f..b5ff436 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.26.0" +version = "0.27.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index 60e5bf1..f118dbf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.26.0" +version = "0.27.0" edition = "2021" # io-harness declares 1.95, set by `libsqlite3-sys` calling the std `cfg_select!` # macro from its build script. Nothing here can be lower than what the harness diff --git a/README.md b/README.md index 8473a02..b4128aa 100644 --- a/README.md +++ b/README.md @@ -508,6 +508,7 @@ above a row that ranked there for reasons having nothing to do with it. | --- | --- | | `/model` | change the model the next turn is sent to | | `/effort` | how much reasoning the next turn buys: low, medium, high, or off | +| `/undo` | put work back: `` for one file, `step ` for one step, bare for the run | | `/contain` | run turns contained, so the agent can fan out: on, off, or ask | | `/plan` | make turns propose a plan before they work: on, off, or ask | | `/steer` | send what is queued into the turn that is already running | @@ -529,6 +530,8 @@ above a row that ranked there for reasons having nothing to do with it. | `/skills` | every skill, shipped or yours: what it is for, whether it is on, and its file | | `/cost` | commit what this run, this session and this install have spent | | `/stats` | commit how the runs have gone: outcomes, first-try, gates, latency | +| `/store` | commit what the run store holds; `rm `, `sweep ` and `compact` change it | +| `/export` | write this conversation as markdown, or `trace` for a run's canonical trace | **configure** @@ -2021,6 +2024,126 @@ browser at all. io-cli's own `[app.io-cli.browser]` is read from either scope. locale behind `glyphs`. See [Reading it without seeing it](#reading-it-without-seeing-it). +## What the store is holding + +**`/store` commits a page: what the run store costs on disk, what is already free +inside it, and what each session in it holds.** Everything on that page is +io-harness's own arithmetic — `page_size × page_count` for the file, and +`page_size × freelist_count` for the free part — read from the store rather than +computed here. + +The distinction between those two numbers is the whole reason this page exists. + +**A deletion does not shrink the file.** SQLite frees pages *into* the database +rather than out of it, so removing a session moves bytes from the file's size +into the free space inside it and the file on disk stays exactly the size it was. +Every store this product has ever created was made without `auto_vacuum`, so +there is no incremental reclamation either: a `VACUUM` is the only thing that +returns the space, and it is `/store compact`. + +That matters because `~/.io-cli/runs.db` has held every session, run, step, +event, provider call, snapshot and restore point since 0.15.0, with no retention +policy and no rotation. Until this release there was no way to look at it, and no +way to shrink it. + +Three verbs change it, and each one shows what it will do before it does it: + +| | | +|---|---| +| `/store rm ` | remove one session and everything keyed to it | +| `/store sweep ` | remove every session created before that timestamp | +| `/store compact` | rewrite the database without its free pages | + +**Each descends into a confirmation whose first row is "leave it".** That is the +row the cursor starts on, so the keystroke you give by reflex is the one that +changes nothing. + +**A removal is final and takes the session's restore points with it.** Its +*memory* stays — the agent's durable notes belong to the workspace rather than to +the session, so removing a session unlearns nothing — but the rewind for those +turns is gone, and that is the part that bites later. + +**A sweep refuses a session that still holds a resumable run**, and names it. A +date is a policy applied to sessions nobody looked at, and a crash-resumable tree +that vanished because it was old would be the worst thing this command could do. + +**The sweep asks you to agree to the rule rather than to a count**, and the +reason is a gap in io-harness rather than a choice: `sweep_sessions` filters on +`sessions.created_at` and nothing exposes that column, so the set a date selects +cannot be counted until the sweep has run. The nearest substitute — a session's +first turn — is always *later* than the session itself, so a count built on it +would under-state what is about to be deleted. The figures are reported the +moment it finishes, refusals included. Filed as io-harness#216. + +**Compaction is not free while it runs.** It rewrites the whole database and +needs roughly the file's own size in free disk space to do it, so it is a thing +you ask for rather than something a deletion does on your behalf. It reports the +bytes the file actually shrank by, measured, not inferred. + +Nothing here happens on its own — there is no retention setting, no threshold and +no sweep at startup — and no model can reach any of it. + +## Putting work back + +**`/undo` is the size of the mistake.** Until this release the only instrument was +the rewind chord, which undoes a whole turn, and that is the wrong size for *this +one file went wrong*. + +| | | +|---|---| +| `/undo ` | put one file back as it was before the run | +| `/undo step ` | reverse-apply what one step wrote | +| `/undo` | the whole turn — the same thing the rewind chord does | + +**One file has four possible answers and they are four different sentences.** It +came back; it was *removed*, because the run had created it; nothing changed +because the previous contents were not kept; nothing changed because this run +never wrote that path. The last two both mean your file is untouched, and they +mean it for different reasons. + +**Undoing a step is order-sensitive.** Reverse-applying one step's diff while a +later step's change still sits on the same lines finds context that has moved, +and io-harness leaves the file alone rather than fuzzy-matching it into +something nobody wrote. Undo the newest step first and it applies — and io says +so when it happens, because "nothing changed" without that sentence reads as a +bug. + +**A restore does not know about an edit you made afterwards.** The file comes +back from the snapshot taken before the run first wrote it, and that snapshot is +not compared against what is on disk now, so a change you made by hand after the +turn is overwritten. The confirmation says so before you agree to it. + +Every restore goes through the same path policy a write does, and a *removal* +asks the policy separately and refuses anything that is not an outright allow. + +## Taking the work out + +**`/export` writes this conversation as markdown, and `/export trace` writes one +run's canonical trace.** For the review that happens in a pull request, or a text +editor, or a message to somebody who was not there. + +| | | +|---|---| +| `/export` | the conversation, as markdown | +| `/export ` | the same, where you say | +| `/export trace` | the last run's canonical trace | +| `/export trace ` | the same, where you say | + +Both are written into the workspace under the session's own path policy, and +**an existing file is refused rather than overwritten** — an export is a +snapshot, and the next one you take is a different snapshot. + +**The trace is written exactly as io-harness produced it.** Its whole value is +that it is canonical: io-harness leaves wall-clock stamps, measured durations, an +ephemeral sandbox path and autoincrement ids out of it so that two runs of one +case can be compared. A trace io-cli reformatted would compare against nothing, +so io-cli does not parse it, reserialise it, or pretty-print it. It is +pipe-delimited text rather than JSON, and it takes a `.txt` extension for that +reason. + +A turn that never finished is written as a turn that never finished, rather than +as one the agent had nothing to say to. + ## What this release is not The configuration file has reached your terminal since 0.14.0: every section of diff --git a/src/commands.rs b/src/commands.rs index c460680..dbc9df2 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -247,6 +247,13 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/contain", "run turns contained, so the agent can fan out: on, off, or ask", ), + // 0.27.0 — the undo that is the size of the mistake. The bare word is the + // whole run, which is what the rewind chord has always done; the two argued + // forms are what this release adds and are the ones an operator reaches for. + ( + "/undo", + "put work back: `` for one file, `step ` for one step, bare for the run", + ), ( "/plan", "make turns propose a plan before they work: on, off, or ask", @@ -269,6 +276,23 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/stats", "commit how the runs have gone: outcomes, first-try, gates, latency", ), + // 0.27.0 — the third page about work already done, and the first one that can + // also change it. The description names the three verbs rather than only the + // page, because a command whose bare form reports and whose argued forms + // delete is one an operator must be able to tell apart from the row. + ( + "/store", + "commit what the run store holds; `rm `, `sweep ` and `compact` change it", + ), + // Beside `/store` because the two answer the same question from opposite + // ends: that one is what the store is keeping, this one is how the work gets + // out. The description names both files, because markdown and a canonical + // trace are for two different readers and choosing between them is the whole + // decision. + ( + "/export", + "write this conversation as markdown, or `trace` for a run's canonical trace", + ), // Beside `/stats` because `/stats` is the only other row that says the word, // and the two are the halves of one thing: that page counts how the gates // went, and until this release nothing in the product could say what a gate @@ -376,9 +400,24 @@ pub const GROUPS: &[(Group, &[&str])] = &[ // that met it — re-file what is in the wrong group, do not widen the bound — // and a correction that pays for itself is the correction working, not a // loophole in it. + // **`/contain` moved here in 0.27.0, and it is a correction rather than a way + // of making room** — the fourth time this sentence has been written, after + // 0.19.0's `/mcp` and `/provider`, 0.22.0's `/image` and `/copy`, and 0.26.0's + // `/profile`. `Turn` means a command acting on the work the turn just + // finished. `/contain on|off` acts on nothing that has happened: it decides + // whether every *later* turn is driven through the containment entry point, + // and it survives the turn that switched it. That is the identical argument + // that moved `/profile` one release ago, and `/contain` was the other command + // it applied to all along. + // + // It frees the slot `/undo` takes. `Turn` stood at ten of ten, and the rule + // this product wrote for itself when 0.25.0 filled it is to re-file what is in + // the wrong group rather than widen the bound. ( Group::Session, - &["/clear", "/resume", "/fork", "/profile", "/setup", "/exit"], + &[ + "/clear", "/resume", "/fork", "/profile", "/contain", "/setup", "/exit", + ], ), // **`/image`, `/copy` and `/copy diff` moved here in 0.22.0, and it is a // correction rather than a way of making room** — the same sentence 0.19.0 @@ -407,7 +446,12 @@ pub const GROUPS: &[(Group, &[&str])] = &[ // this session's and dies with it, for the reason the release contract // records under `open_questions`. "/effort", - "/contain", + // **`/undo` takes the slot `/contain` left.** It is the most + // `Turn`-shaped command in the product: it acts on the work the turn + // just finished, at whichever granularity the operator meant. See the + // note above `Group::Session` for why `/contain` was the one that + // moved rather than the bound. + "/undo", "/plan", "/steer", "/compact", @@ -438,6 +482,14 @@ pub const GROUPS: &[(Group, &[&str])] = &[ Group::Inspect, &[ "/help", "/status", "/context", "/expand", "/fleet", "/skills", "/cost", "/stats", + // 0.27.0 — the two that answer a question about work that has already + // happened rather than about the turn in flight, which is what this + // group means. `/store` is what the run store is holding; `/export` + // is that work taken somewhere else. They take `Inspect` to ten, + // which is the bound — so the next command that would fill this group + // re-files one that is in the wrong group rather than widening it, + // exactly as `Turn` did for `/undo` in this same release. + "/store", "/export", ], ), // **`/mcp` and `/provider` moved here in 0.19.0, and it is a correction rather @@ -1214,6 +1266,64 @@ pub enum Action { /// that caused them. [`crate::gates::Refusal`] names both while the operator /// is still looking at what they typed. Gates, + /// Report what the run store holds, or change it. + /// + /// `None` is the page and changes nothing. Every `Some` is a verb, and every + /// verb that acts descends into a confirmation first — see [`Keep`], and + /// [`crate::store`] for why a removal and a compaction are different + /// questions with different costs. + Store(Option), + /// Write the work out: the conversation as markdown, or one run's canonical + /// trace, verbatim. + /// + /// See [`Taken`]. The path is optional in both forms and a proposed one is + /// used when it is absent — proposed rather than imposed, and refused rather + /// than overwritten when something is already there. + Export(Taken), + /// Put work back, at the granularity the operator named. + /// + /// `Grain::Run` is what the rewind chord has always done and what a bare + /// `/undo` means. The other two are this release's, and both are public + /// io-harness calls this crate had never made. + Undo(crate::undo::Grain), + /// `/undo step` with no number, or a number that is not one. + /// + /// A variant rather than a fall-through to the whole run, which is the one + /// mistake this command must not make: an operator who typed a step and got + /// the entire run undone would have lost work they never asked to lose. + UndoNoStep, +} + +/// Which export was asked for, and where it goes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Taken { + /// This session's conversation, as markdown. + Conversation(Option), + /// The last run's canonical trace, exactly as io-harness produced + /// it. + Trace(Option), +} + +/// What `/store` was asked to do, once the page is not the answer. +/// +/// The three refusal variants are variants rather than an `Err`, because they +/// are answers this surface gives the operator in its own words. An unparseable +/// verb must never fall through to the page: somebody who typed a delete and got +/// a report would believe the delete had happened. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Keep { + /// Remove one session, by id. + Remove(i64), + /// Remove every session created strictly before this timestamp. + Sweep(String), + /// Return the free pages inside the file to the filesystem. + Compact, + /// `rm` with no id, or an id that is not a number. + NoId, + /// `sweep` with no date. + NoDate, + /// A word this command does not know. + Unknown(String), } /// What `/copy` was asked for. @@ -1860,6 +1970,57 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { "usage" => Action::Cost, "cost" => Action::Cost, "stats" => Action::Stats, + // Three verbs behind one word, and the bare form is the only one that + // does not change anything. A rejected verb reports rather than falling + // through to the page: an operator who typed `/store swep 2026-08-01` + // meant to delete something, and quietly showing them a report would + // leave them believing they had. + "store" => { + let mut rest = input.split_whitespace().skip(1); + match rest.next() { + None => Action::Store(None), + Some("rm" | "remove" | "delete") => match rest.next().map(str::parse::) { + Some(Ok(id)) => Action::Store(Some(Keep::Remove(id))), + // A missing or unparseable id is named rather than defaulted. + // There is no sensible default session to delete, and the one + // that looks sensible — the current one — is the worst. + _ => Action::Store(Some(Keep::NoId)), + }, + Some("sweep") => match rest.next() { + Some(date) => Action::Store(Some(Keep::Sweep(date.to_string()))), + None => Action::Store(Some(Keep::NoDate)), + }, + Some("compact" | "vacuum") => Action::Store(Some(Keep::Compact)), + Some(word) => Action::Store(Some(Keep::Unknown(word.to_string()))), + } + } + // `trace` is a word and everything else is a path, which is the one + // ambiguity worth naming: an operator who wants a file called `trace` + // types `./trace`. The alternative — a `--trace` flag — would be the only + // flag on any command in this product. + // `step` is a word and everything else is a path, the same shape `/export` + // takes for `trace`. A bare `/undo` is the whole run, which is what the + // rewind chord already means, so the word and the chord cannot disagree. + "undo" => { + let mut rest = input.split_whitespace().skip(1); + match rest.next() { + None => Action::Undo(crate::undo::Grain::Run), + Some("step") => match rest.next().map(str::parse::) { + Some(Ok(step)) => Action::Undo(crate::undo::Grain::Step(step)), + // Never a fall-through to the run. See `Action::UndoNoStep`. + _ => Action::UndoNoStep, + }, + Some(path) => Action::Undo(crate::undo::Grain::File(path.to_string())), + } + } + "export" => { + let mut rest = input.split_whitespace().skip(1); + match rest.next() { + Some("trace") => Action::Export(Taken::Trace(rest.next().map(str::to_string))), + Some(path) => Action::Export(Taken::Conversation(Some(path.to_string()))), + None => Action::Export(Taken::Conversation(None)), + } + } "config" | "settings" => { let mut rest = input.split_whitespace().skip(1); match rest.next() { diff --git a/src/events.rs b/src/events.rs index f6ff761..adb0fb9 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1606,6 +1606,39 @@ impl Events { lines.push(Line::from("")); lines } + // **What starting early bought, and what it cost.** 0.27.0, and the + // one silence in `triage::TRIAGE` that had no route to a surface an + // operator uses: it went to `io exec --json` and the durable trace, + // which are both places somebody goes deliberately, afterwards, + // already suspecting something. + // + // `discarded` is the figure worth the line. A read started before the + // model had finished asking and then thrown away is work that was paid + // for and not used, and it is the only number here an operator can act + // on — by turning speculation off. `started` alone would read as a + // brag about concurrency. + // + // **Only when something was discarded.** io-harness emits this + // whenever `started > 0` (`run/step.rs:1272`), so a step that + // speculated perfectly would otherwise put a line in every transcript + // saying nothing happened. A run where speculation always pays is a + // run with nothing to report, which is the same rule the sandbox arm + // above follows for a cap that was not hit. + EventKind::Speculated { + started, + used, + discarded, + } if *discarded > 0 => { + let mut lines = self.flush_text(); + lines.push(Line::from(Span::styled( + format!( + "{}read ahead{separator}{used} of {started} used, {discarded} discarded", + leader(separator), + ), + theme.style(Tone::Muted), + ))); + lines + } // Every kind that commits no line of its own, and the one case that // is a defect rather than a decision. // diff --git a/src/export.rs b/src/export.rs new file mode 100644 index 0000000..12af4f3 --- /dev/null +++ b/src/export.rs @@ -0,0 +1,284 @@ +//! The conversation as markdown, and the run's canonical trace, verbatim. +//! +//! A session is a conversation that ends when the terminal closes. The review +//! that matters usually happens somewhere else — in a pull request, in a text +//! editor, in a message to somebody who was not there — and until this release +//! there was no way to get it out. `/export` writes two files: the conversation +//! for a human to read, and one run's canonical trace for a machine to compare. +//! +//! # The trace is written verbatim, and that is the whole rule +//! +//! [`Store::canonical_trace`] returns a string whose entire value is that it is +//! **canonical**: io-harness excludes wall-clock stamps, measured durations, the +//! argv's ephemeral tempdir path and `AUTOINCREMENT` ids from it precisely so +//! that two runs of one case can be compared, and its documentation says that +//! excluding a field is a decision the crate cannot promise rather than a +//! convenience. +//! +//! So this module does not parse it, does not reserialise it, does not +//! pretty-print it and adds no field of its own. A trace io-cli reformatted +//! compares against nothing, which is why `tests/export.rs` asserts a +//! **byte-identical** string. +//! +//! **It is not JSON, and this release's plan said it was.** The format is +//! pipe-delimited lines built by hand — `step {n} | tokens {n} | decision … ` +//! per step, then `context {step} | {kind} | {detail}` per context event +//! (`state/trace.rs:1019-1036`). io-harness offers no JSON alternative; +//! `StepRecord` does not even derive `Serialize`. io-cli *could* build one — +//! `serde_json` is among its ten dependencies — and must not, because a document +//! this crate assembled would be a second format that compares against nothing, +//! which is the very thing the verbatim rule exists for. So the export takes a +//! `.txt` extension: a file named `.json` that is not JSON is a defect for +//! anyone who pipes it to a JSON tool. Recorded as `US-IO-CLI-0.27.0-I03`, and +//! found by a sabotage that killed nothing. +//! +//! # The markdown is for a person, and nothing reads it back +//! +//! It carries no schema version, nothing in this product parses it, and it is +//! not an interchange format. It is built from [`Store::session_turns`], whose +//! [`io_harness::Turn`] carries `prompt`, `reply`, `outcome`, `run_id` and +//! `created_at` — every one of them written by the store, and the timestamp +//! passed through untouched, because this crate reads no clock outside +//! `src/main.rs` and `tests/timing.rs` enforces it. +//! +//! **A turn with no reply is written as a turn with no reply.** `Turn::reply` is +//! `Option`, and `None` means the turn did not finish — a run that was +//! interrupted, that died, or that is still going. Rendering it as an empty +//! answer would make an unfinished conversation look like one the agent had +//! nothing to say to. +//! +//! **There is no public reader for `runs.goal`.** Recorded in 0.23.0 and still +//! true in 0.69, so a run is named here by its id and its timestamp rather than +//! by the goal it was given. +//! +//! # Both files go through the workspace, under the session's own policy +//! +//! [`Workspace::write_file`] and never `std::fs::write`: an export is a file +//! this product writes on the operator's behalf, so it is subject to exactly the +//! path policy every other write is, and a path outside the workspace is refused +//! by io-harness rather than by a check invented here. +//! +//! **An existing file is refused rather than overwritten.** An export is a +//! snapshot, and the second one an operator takes is a different snapshot; the +//! command that silently replaced the first would destroy the thing they were +//! about to compare against. [`Refused::Exists`] names the file and stops. + +use io_harness::tools::{Workspace, Wrote}; +use io_harness::{Error, Store}; + +/// The extension a conversation export takes. +pub const MARKDOWN: &str = "md"; + +/// The extension a trace export takes. +/// +/// `txt` and not `json`: [`Store::canonical_trace`] is pipe-delimited text. See +/// the module note and `US-IO-CLI-0.27.0-I03`. +pub const TRACE: &str = "txt"; + +/// Why an export did not happen. +/// +/// Both variants are refusals rather than errors: the operator asked for +/// something reasonable and is being told why it did not happen, in words that +/// name the next thing they can do. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Refused { + /// There is already a file at that path. An export is a snapshot and the + /// next one is a different snapshot; overwriting silently would destroy the + /// thing the operator was about to compare against. + Exists(String), + /// The session holds no turns, so there is no conversation to write. Writing + /// an empty document would be a file that says the session was empty when it + /// may simply not have started. + Nothing, +} + +impl Refused { + /// What to say about it. + pub fn said(&self) -> String { + match self { + Refused::Exists(path) => format!( + "{path} is already there — an export is a snapshot, so this one is \ + not written over it; name another path" + ), + // "no turns", not "no finished turns": `conversation` answers `None` + // only when the conversation is empty, and a turn that never finished + // is exported with a line saying so. + Refused::Nothing => { + "there is nothing to export yet — this conversation has no turns".to_string() + } + } + } +} + +/// What one export did. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Written { + /// The path, relative to the workspace root, as it was written. + pub path: String, + /// How many bytes went into it. + pub bytes: usize, + /// What io-harness's own writer said happened. Always [`Wrote::Created`] + /// here, because an existing path is refused before the write — carried + /// anyway so that a future path that stops refusing cannot silently start + /// reporting a creation for an overwrite. + pub wrote: Wrote, +} + +/// The conversation, as markdown. +/// +/// Every turn the store holds for the session, in the order it returns them, +/// each with its prompt and — when the turn finished — its reply. A turn that +/// never finished says so. +pub fn conversation(store: &Store, session: &io_harness::Session) -> Result, Error> { + // **`Session::history` and never `Store::session_turns`.** io-harness + // documents the second as "the whole tree, not one path through it" + // (`state/sessions.rs:265-266`), and a rewind moves the head back **without + // deleting the turn** (`src/rewind.rs`), so a session that was undone once + // and carried on holds turns that are not part of the conversation at all. + // + // Exporting the tree flat would put those turns in the document, in + // sequence, with nothing marking them — in the one artifact whose stated + // purpose is being read by somebody who was not there. It would + // misrepresent both what the agent was asked and what it answered. Found by + // the adversarial review; the first implementation had exactly that defect. + let turns = session.history(store)?; + let session_id = session.id(); + if turns.is_empty() { + return Ok(None); + } + + let mut out = format!("# Session {session_id}\n"); + for turn in turns { + // The store's own stamp, passed through. Never an age, never a + // reformatting: `tests/timing.rs` forbids this crate reading a clock, and + // a document exported twice from the same store must be the same document. + out.push_str(&format!( + "\n## Turn {} · run {} · {}\n\n", + turn.id, turn.run_id, turn.created_at + )); + out.push_str("### Prompt\n\n"); + out.push_str(&turn.prompt); + out.push_str("\n\n### Reply\n\n"); + match &turn.reply { + Some(reply) => { + out.push_str(reply); + out.push('\n'); + } + // Not an empty section. `None` means the turn did not finish, and an + // interrupted conversation must not read as one the agent had nothing + // to say to. + None => out.push_str("*this turn did not finish*\n"), + } + if let Some(outcome) = &turn.outcome { + out.push_str(&format!("\n*outcome: {outcome}*\n")); + } + } + Ok(Some(out)) +} + +/// One run's canonical trace, exactly as io-harness produced it. +/// +/// Passed through. See the module note: the value of this string is that it is +/// canonical, and anything this crate did to it would end that. +pub fn trace(store: &Store, run_id: i64) -> Result { + store.canonical_trace(run_id) +} + +/// The path an export proposes for a session's conversation. +/// +/// Proposed rather than imposed: the operator can name their own, and this is +/// what they get when they do not. Named after what it holds so two exports of +/// two sessions do not collide, which is the same reason it is refused rather +/// than overwritten when it is already there. +pub fn conversation_path(session_id: i64) -> String { + format!("io-session-{session_id}.{MARKDOWN}") +} + +/// The path an export proposes for a run's trace. +pub fn trace_path(run_id: i64) -> String { + format!("io-run-{run_id}.{TRACE}") +} + +/// Write one export, refusing rather than overwriting. +/// +/// Through [`Workspace::write_file`], so the path policy the session is running +/// under is the one that applies and a path outside the workspace is io-harness's +/// refusal rather than a check invented here. +/// +/// **The refusal is checked here, immediately before the write, and not only by +/// the caller.** The confirmation an operator agrees to is shown after an earlier +/// [`occupied`] call, and between the two keystrokes another `io`, an editor +/// autosave or a build can create the file — after which the write would +/// silently replace it while reporting a creation. The doc on [`Written::wrote`] +/// promised `Wrote::Created` and only this check makes that true. Found by the +/// adversarial review, which is also where the earlier version of this comment — +/// describing a `read_bytes` check that was not in the function — was caught. +pub fn write(workspace: &Workspace, path: &str, content: &str) -> Result { + if occupied(workspace, path)? { + return Err(Error::Config(Refused::Exists(path.to_string()).said())); + } + let wrote = workspace.write_file(path, content)?; + Ok(Written { + path: path.to_string(), + bytes: content.len(), + wrote, + }) +} + +/// Whether something is already at `path`, asked through the workspace. +/// +/// `Ok(true)` means there is a file there. An `Err` is the path escaping the +/// root, and is propagated rather than treated as absence — answering "nothing +/// is there" for a path that cannot be written would send the operator into a +/// confirmation for a write that was never going to happen. +/// +/// **[`Workspace::resolve`] and then `Path::exists`, rather than a read.** The +/// first implementation asked [`Workspace::read_bytes`] and treated `Error::Io` +/// as absence, and that was wrong in the way that matters: io-harness answers a +/// missing file with `Error::Config("no such file: …")`, so *every first export* +/// would have been refused with "could not be checked" — the export that has +/// never been taken being exactly the one an operator takes first. Caught by +/// `tests/export.rs`, never by reading. +/// +/// Asking `resolve` also separates the two questions properly. This one is *may +/// this path be addressed at all* — absolute paths and any `..` climbing above +/// the root are refused here, by io-harness, on its own rules. Whether the +/// operator may **write** it is [`Workspace::write_file`]'s own `Act::Write` +/// gate, which is enforced at the write and must not be second-guessed here: a +/// permission check that ran early and a permission check that ran late would be +/// two answers to one question. +pub fn occupied(workspace: &Workspace, path: &str) -> Result { + let resolved = workspace.resolve(path)?; + // **`symlink_metadata` and not `exists()`.** `exists` follows the link, so a + // **dangling** symlink inside the workspace answers "nothing is there" — and + // `write_file` then follows it and creates the file wherever it points, + // outside the root. `Workspace::resolve` cannot catch that: it is purely + // lexical, and `check_path`'s own symlink branch is guarded by a + // `canonicalize` that fails on a broken link. Asking about the link itself is + // what closes it. Found by the adversarial review. + Ok(resolved.symlink_metadata().is_ok()) +} + +/// The confirmation `/export` shows before it writes. +/// +/// Row 0 declines, as it does in every confirmation this product uses for +/// something it cannot take back — and an export can overwrite nothing, but it +/// does put the conversation on disk where the operator may not have expected a +/// file to appear. +pub fn confirm(path: &str, what: &str) -> (String, Vec) { + ( + format!("Write this {what} to {path}?"), + vec![ + crate::picker::Row::with_detail(crate::store::LEAVE_IT, "nothing is written"), + crate::picker::Row::with_detail( + format!("write {path}"), + "under the session's own path policy; an existing file is refused", + ), + ], + ) +} + +/// What to say after an export. +pub fn report(written: &Written) -> String { + format!("wrote {} ({} bytes)", written.path, written.bytes) +} diff --git a/src/lib.rs b/src/lib.rs index 7c16be9..f17c0d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,6 +62,7 @@ pub mod edit; pub mod editor; pub mod events; pub mod exec; +pub mod export; pub mod failure; pub mod fanout; pub mod fleet; @@ -101,9 +102,11 @@ pub mod splash; pub mod stats; pub mod status; pub mod stdin; +pub mod store; pub mod term; pub mod theme; pub mod transcript; pub mod triage; +pub mod undo; pub mod verify; pub mod wizard; diff --git a/src/main.rs b/src/main.rs index 1bb4f93..d27c151 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1580,7 +1580,19 @@ async fn loop_over Result>( } } // Row 0 is "leave it", the default that does nothing — - // the shape `/skills` and `/plugin` both already use. + // the shape `/plugin` and `/provider` also use. + // + // **Corrected in 0.27.0: this comment named `/skills`, + // and `/skills` is the one confirmation in this product + // that does the opposite** — `Pick::SkillToggle` puts the + // verb at row 0 and "leave it as it is" at row 1. That is + // defensible there and only there, because a toggle is + // reversible by repeating it and nothing is destroyed; + // every confirmation that removes something puts the + // declining row first. The comment was load-bearing + // enough to be believed and repeated while `/store` was + // designed, which is why it is corrected rather than + // deleted. // Row 2 is the edit verb, and it descends rather than // acting: which key is half the decision, and the other // half is a value only the operator can type. @@ -2478,6 +2490,170 @@ async fn loop_over Result>( listing is not shown", ), }, + // The three store confirmations, and all three share one + // rule: **index 0 is `store::LEAVE_IT` and does nothing**. + // Asserted on the index rather than on the label, because + // a confirmation whose acting row drifted to the top is + // the defect F5 exists to catch and a label comparison + // would not see it. + Pick::StoreRemove { id } => { + if io_cli::store::acts(index) { + match io_cli::store::remove(&store, *id, session.id()) { + Ok(io_cli::store::Removal::Done(removed)) => { + for line in io_cli::store::removed_report(&removed) { + app.record(Tone::Muted, line); + } + } + // Unreachable from this surface, because + // `confirm_remove` offers no acting row for the + // live session. Kept because the guard belongs + // to the library and a second caller must not + // be able to route around it. + Ok(io_cli::store::Removal::Live) => app.record( + Tone::Refused, + "that is the conversation you are in; io will not \ + remove it" + .to_string(), + ), + Err(error) => app.record( + Tone::Error, + format!( + "session {id} was not removed: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } + Pick::StoreSweep { date } => { + if io_cli::store::acts(index) { + match io_cli::store::sweep(&store, date) { + Ok(swept) => { + for line in io_cli::store::swept_report(&swept) { + app.record(Tone::Muted, line); + } + } + Err(error) => app.record( + Tone::Error, + format!( + "the sweep did not run: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } + // The two new granularities. The step form goes through + // `observing`, which is what makes `EventKind::Reverted` + // fire at all — an event this product had never emitted, + // because only the `_observed` form emits it. The file + // form does not: `io_harness::rewind` has no observed + // twin and emits nothing, which is why it takes none. + Pick::UndoFile { run_id, path } => { + if io_cli::store::acts(index) { + let workspace = io_harness::tools::Workspace::new(session.root()); + match io_cli::undo::one_file(&workspace, &store, *run_id, path) { + Ok(answer) => { + app.record(Tone::Muted, io_cli::undo::said(path, &answer)) + } + Err(error) => app.record( + Tone::Error, + format!( + "{path} was not put back: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } + Pick::UndoStep { run_id, step } => { + if io_cli::store::acts(index) { + let workspace = io_harness::tools::Workspace::new(session.root()); + let run = *run_id; + let at = *step; + let done = observing(&mut app, screen, |observer| { + io_cli::undo::one_step(&workspace, &store, run, at, observer) + })?; + match done { + Ok(answers) if answers.is_empty() => app.record( + Tone::Muted, + format!("step {at} wrote no files, so nothing changed"), + ), + Ok(answers) => { + for (path, answer) in &answers { + app.record( + Tone::Muted, + io_cli::undo::step_said(path, answer), + ); + } + // The order-sensitivity sentence, said only + // when something actually came back stale — + // otherwise it is advice about a problem the + // operator does not have. + if let Some(advice) = io_cli::undo::step_advice(&answers) { + app.record(Tone::Warning, advice); + } + } + Err(error) => app.record( + Tone::Error, + format!( + "step {at} was not undone: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } + // The same `undo_whole_turn` the chord reaches, so the + // word and the keystroke cannot drift apart. + Pick::UndoRun => { + if io_cli::store::acts(index) { + undo_whole_turn(&mut app, screen, &mut session, &store, &seen)?; + } + } + Pick::Export { path, content } => { + if io_cli::store::acts(index) { + let effective = approval::session_policy( + &policy, + app.posture(), + app.remembered(), + ); + let workspace = io_harness::tools::Workspace::with_policy( + session.root(), + effective, + ); + match io_cli::export::write(&workspace, path, content) { + Ok(written) => { + app.record(Tone::Muted, io_cli::export::report(&written)) + } + Err(error) => app.record( + Tone::Error, + format!( + "{path} was not written: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } + Pick::StoreCompact => { + if io_cli::store::acts(index) { + match io_cli::store::compact(&store) { + Ok(freed) => { + for line in io_cli::store::freed_report(&freed) { + app.record(Tone::Muted, line); + } + } + Err(error) => app.record( + Tone::Error, + format!( + "the store was not compacted: {}", + io_cli::failure::said(&error) + ), + ), + } + } + } } // `None` in every arm but the descent, so this closes the // picker exactly as it always did and replaces it in the one @@ -2645,37 +2821,12 @@ async fn loop_over Result>( None => app.say(Tone::Muted, "there is no turn to undo"), }, // The second. This is where the operator's files change. - Command::Rewind => match io_cli::rewind::last_turn(&mut session, &store) { - Ok(Some(undone)) => { - // The undone turn is where those numbers came from. - app.status.forget_run(); - app.forget_fleet(); - // As above: the last request belonged to the undone turn. - seen.forget(); - for (tone, line) in io_cli::rewind::undone_lines(&undone, &app.theme.glyphs) { - app.say(tone, line); - } - } - Ok(None) => app.say(Tone::Muted, "there is no turn to undo"), - // **`failure::said`, and not the raw `Display`.** Since 0.23.0 - // the undo can lose a head race with another `io`, and - // `Error::Conflict`'s own text calls the session id a run id and - // renders an expiry that a head conflict never populates. That - // sentence is exactly what `failure::advice` exists to replace, - // and this arm was the one path in the product still going - // around it. - // - // **And the old line said "nothing was undone", which is false.** - // `rewind::last_turn` restores the files before it attempts the - // head write, so a conflict leaves the operator's files back as - // they were with the conversation head where the other process - // put it. Saying nothing happened would send them looking for - // changes that are already gone. - Err(error) => app.say( - Tone::Error, - format!("the undo did not finish: {}", io_cli::failure::said(&error)), - ), - }, + // **Through `observing` since 0.27.0, which is what finally emits + // `EventKind::Rewound`.** The call was `rewind_run`, whose observed + // twin is the only thing that emits it. + Command::Rewind => { + undo_whole_turn(&mut app, screen, &mut session, &store, &seen)?; + } Command::Slash(text) => match commands::parse(&text, app.keys(), &app.theme) { Action::Print(lines) => { screen.commit(&lines).map_err(|error| error.to_string())?; @@ -3622,6 +3773,224 @@ async fn loop_over Result>( let lines = io_cli::stats::committed(&store, &app.theme, screen.width())?; screen.commit(&lines).map_err(|error| error.to_string())?; } + // The bare word reports and nothing else; every verb that changes + // the store descends into a confirmation whose row 0 does + // nothing. The reads that build each confirmation happen HERE, + // before the operator agrees, so the figures they are shown are + // the ones the operation is about to act on rather than a second + // reading taken afterwards. + Action::Store(None) => { + let lines = io_cli::store::committed(&store, &app.theme, screen.width())?; + screen.commit(&lines).map_err(|error| error.to_string())?; + } + Action::Store(Some(keep)) => match keep { + commands::Keep::Remove(id) => match io_cli::store::sized(&store, id) { + Ok(sized) => { + let (title, rows) = + io_cli::store::confirm_remove(id, &sized, session.id()); + picker = Some((Picker::new(title, rows), Pick::StoreRemove { id })); + } + Err(error) => app.record( + Tone::Error, + format!( + "the store could not be read: {}", + io_cli::failure::said(&error) + ), + ), + }, + // No read at all before this one, because there is nothing + // readable: the set a date selects cannot be counted in + // advance — see io-harness#216 and `US-IO-CLI-0.27.0-I02`. + // The operator agrees to the rule and the report carries the + // figures. + // **Validated before the confirmation is even built.** The + // comparison io-harness makes is lexical against a text + // column, so `2026-8-1` or `yesterday` sorts above every real + // timestamp and sweeps the entire store — while the + // confirmation echoes back exactly what was typed and reads + // as correct. Both adversarial reviewers found this + // independently, which is the strongest signal that gate + // gives. + commands::Keep::Sweep(date) => match io_cli::store::boundary(&date) { + Ok(date) => { + let (title, rows) = io_cli::store::confirm_sweep(&date); + picker = Some((Picker::new(title, rows), Pick::StoreSweep { date })); + } + Err(said) => app.record(Tone::Error, said), + }, + commands::Keep::Compact => match store.store_size() { + Ok(size) => { + let (title, rows) = io_cli::store::confirm_compact(&size); + picker = Some((Picker::new(title, rows), Pick::StoreCompact)); + } + Err(error) => app.record( + Tone::Error, + format!( + "the store could not be read: {}", + io_cli::failure::said(&error) + ), + ), + }, + // Three refusals, each naming what was missing. None of them + // falls through to the page: an operator who typed a deletion + // and was shown a report would believe the deletion happened. + commands::Keep::NoId => app.record( + Tone::Error, + "/store rm needs a session id — `/store` lists them".to_string(), + ), + commands::Keep::NoDate => app.record( + Tone::Error, + "/store sweep needs a date, as the store writes them: \ + `/store sweep 2026-08-01`" + .to_string(), + ), + commands::Keep::Unknown(word) => app.record( + Tone::Error, + format!( + "/store does not know `{word}` — the verbs are `rm `, \ + `sweep ` and `compact`" + ), + ), + }, + Action::UndoNoStep => app.record( + Tone::Error, + "/undo step needs a step number — `/expand` lists them, and a \ + bare `/undo` is the whole run" + .to_string(), + ), + // The two new granularities descend into a confirmation; the whole + // run keeps the chord's own two-press path, which already + // discloses what a restore overwrites. + Action::Undo(grain) => match last_run(&session, &store).map(|turn| turn.run_id) { + None => app.record( + Tone::Error, + "there is nothing to undo — this session has taken no turns".to_string(), + ), + Some(run_id) => match &grain { + io_cli::undo::Grain::File(path) => { + let (title, rows) = io_cli::undo::confirm_file(path); + picker = Some(( + Picker::new(title, rows), + Pick::UndoFile { + run_id, + path: path.clone(), + }, + )); + } + io_cli::undo::Grain::Step(step) => { + let (title, rows) = io_cli::undo::confirm_step(*step); + picker = Some(( + Picker::new(title, rows), + Pick::UndoStep { + run_id, + step: *step, + }, + )); + } + // The bare form confirms like the other two rather than + // arming like the chord. The arming is a property of a + // *keystroke* — one press to warn, a second to act — and + // a typed command has already been deliberate once. Both + // paths end in the same `rewind::last_turn`, so the word + // and the chord can never disagree about what an undo is. + io_cli::undo::Grain::Run => { + match io_cli::rewind::preview(&session, &store) { + Some(about) => { + let title = + io_cli::rewind::armed_line(&about, &app.theme.glyphs); + picker = Some(( + Picker::new( + title, + vec![ + Row::with_detail( + io_cli::store::LEAVE_IT, + "the turn stands", + ), + Row::with_detail( + "undo the whole turn", + "its files, its notes, its queued children \ + and the conversation head", + ), + ], + ), + Pick::UndoRun, + )); + } + None => app.say(Tone::Muted, "there is no turn to undo"), + } + } + }, + }, + // Both halves build their content HERE, before the confirmation, + // so the operator agrees to a file that exists rather than to an + // intention. A build that fails says so and opens nothing. + Action::Export(taken) => { + let workspace = + io_harness::tools::Workspace::with_policy(session.root(), policy.clone()); + let built = match &taken { + commands::Taken::Conversation(path) => { + match io_cli::export::conversation(&store, &session) { + Ok(Some(text)) => Ok(Some(( + path.clone().unwrap_or_else(|| { + io_cli::export::conversation_path(session.id()) + }), + text, + "conversation", + ))), + Ok(None) => Err(io_cli::export::Refused::Nothing.said()), + Err(error) => Err(format!( + "the conversation could not be read: {}", + io_cli::failure::said(&error) + )), + } + } + commands::Taken::Trace(path) => { + match last_run(&session, &store).map(|turn| turn.run_id) { + Some(run_id) => match io_cli::export::trace(&store, run_id) { + Ok(json) => Ok(Some(( + path.clone() + .unwrap_or_else(|| io_cli::export::trace_path(run_id)), + json, + "trace", + ))), + Err(error) => Err(format!( + "the trace could not be read: {}", + io_cli::failure::said(&error) + )), + }, + None => Err(io_cli::export::Refused::Nothing.said()), + } + } + }; + match built { + Ok(Some((path, content, what))) => { + // Asked before the confirmation, so an operator is + // never shown a write that is going to be refused. + match io_cli::export::occupied(&workspace, &path) { + Ok(true) => app.record( + Tone::Error, + io_cli::export::Refused::Exists(path).said(), + ), + Ok(false) => { + let (title, rows) = io_cli::export::confirm(&path, what); + picker = Some(( + Picker::new(title, rows), + Pick::Export { path, content }, + )); + } + Err(error) => app.record( + Tone::Error, + format!( + "{path} could not be checked: {}", + io_cli::failure::said(&error) + ), + ), + } + } + Ok(None) => {} + Err(said) => app.record(Tone::Error, said), + } + } // Reached only at an idle prompt: while a turn runs the driver's // own key handler answers `/steer` before `parse` is ever called, // because that is where the inbox lives. So this arm is the @@ -5270,6 +5639,119 @@ fn commit_transcript( } /// The last run of this session, if it has had one. +/// Undo the whole turn, and say what it did. +/// +/// **One implementation, two doors.** The rewind chord reaches it through +/// `Command::Rewind` after its two keystrokes; `/undo` with no argument reaches +/// it through a confirmation. The word and the chord must never disagree about +/// what an undo *is*, and the only way to guarantee that is for there to be one +/// of them. +/// +/// **Through [`observing`] since 0.27.0, which is what finally emits +/// `EventKind::Rewound`.** The call underneath was `rewind_run`, whose observed +/// twin is the only thing that emits it, so the event had never fired. It draws +/// no line — `rewound` is `Disposition::Silent` and routes to the summary this +/// function commits — but it now reaches hooks, `io exec --json` and the trace. +fn undo_whole_turn( + app: &mut App, + screen: &mut Screen>, + session: &mut Session, + store: &Store, + seen: &io_cli::context::Seen, +) -> Result<(), String> { + match observing(app, screen, |observer| { + io_cli::rewind::last_turn(session, store, observer) + })? { + Ok(Some(undone)) => { + // The undone turn is where those numbers came from. + app.status.forget_run(); + app.forget_fleet(); + // As above: the last request belonged to the undone turn. + seen.forget(); + // **`record`, never `say`.** `App::say` is a one-slot footer notice + // that the next keystroke clears, and `undone_lines` returns at least + // two lines plus one per file that was NOT put back — so every + // "left as the turn left it" warning was being overwritten and the + // operator was told a restore had happened that had not. This is + // 0.26.0's recorded rule (`record`, always, for anything an operator + // must still be able to read) arriving one release late, and it is + // the most destructive act in the product having the least durable + // report. Found by the adversarial review. + for (tone, line) in io_cli::rewind::undone_lines(&undone, &app.theme.glyphs) { + app.record(tone, line); + } + } + Ok(None) => app.record(Tone::Muted, "there is no turn to undo".to_string()), + // **`failure::said`, and not the raw `Display`.** Since 0.23.0 the undo + // can lose a head race with another `io`, and `Error::Conflict`'s own + // text calls the session id a run id and renders an expiry that a head + // conflict never populates. That sentence is exactly what + // `failure::advice` exists to replace, and this arm was the one path in + // the product still going around it. + // + // **And the old line said "nothing was undone", which is false.** + // `rewind::last_turn` restores the files before it attempts the head + // write, so a conflict leaves the operator's files back as they were with + // the conversation head where the other process put it. Saying nothing + // happened would send them looking for changes that are already gone. + Err(error) => app.record( + Tone::Error, + format!("the undo did not finish: {}", io_cli::failure::said(&error)), + ), + } + Ok(()) +} + +/// Run something that wants an `Observer`, and commit whatever it emitted. +/// +/// **This is what makes `EventKind::Rewound` and `EventKind::Reverted` exist at +/// all.** Both are emitted only by the `_observed` forms of the rewind +/// functions, and every one of those forms takes an observer — but an undo +/// happens between turns, at a keystroke, where the driver's own observer +/// composition does not exist. So one is built for the act and drained +/// immediately afterwards. +/// +/// **It does not put either event on screen, and an earlier version of this +/// comment said it did.** Both kinds are `Disposition::Silent` in +/// [`io_cli::triage`] and route to io-cli's own rewind summary; `src/events.rs` +/// has no arm for either and correctly renders nothing. What the observed forms +/// buy is that the events reach `[[hook]]` observers, the `io exec --json` +/// stream and the durable trace — none of which they had ever reached. The +/// drain below is what carries anything a *future* arm renders, and what makes +/// this the one place such an arm would need no further wiring. Found by the +/// adversarial review. +/// +/// **What is drained is rendered and never fed to [`App::event`].** That method +/// folds into the status line, the fleet and the token totals, which belong to a +/// *run*; an undo is not a run, and 0.20.0 recorded what happens when foreign +/// events go through it — inflated session totals, a replaced provider, and a +/// grafted tree. `Events::event` is the renderer alone, which is exactly what is +/// wanted here. +fn observing( + app: &mut App, + screen: &mut Screen>, + act: impl FnOnce(&dyn io_harness::Observer) -> T, +) -> Result { + let (bridge, mut events) = io_cli::bridge::channel(); + let out = act(&bridge); + // Synchronous and already finished, so the queue is complete: `try_recv` + // drains it without waiting, and waiting is what would hang the interface. + while let Ok(event) = events.try_recv() { + let lines = app.events.event(&event, std::time::Duration::ZERO); + if !lines.is_empty() { + screen.commit(&lines).map_err(|error| error.to_string())?; + } + } + Ok(out) +} + +/// The last run of this session, if it has had one. +/// +/// **Its doc comment was taken by an insertion in this release** — `undo_whole_turn` +/// and `observing` were added directly beneath it, so this sentence became their +/// summary line and this function was left with none. Found by the adversarial +/// review, and worth noting because nothing in the suite can see a rustdoc +/// summary attached to the wrong item. fn last_run(session: &Session, store: &Store) -> Option { session .transcript(store) @@ -5729,6 +6211,57 @@ enum Pick { at: io_cli::providers::At, first: bool, }, + /// A confirmation over one session's removal. Row 0 is `store::LEAVE_IT` and + /// every other row acts — the shape `/mcp` remove established and the one + /// criterion F5 asserts by *index*, because a confirmation whose default + /// keystroke destroys something is the defect it exists to prevent. + StoreRemove { + /// The session the confirmation named, carried rather than re-read: the + /// figures the operator agreed to were read against this id. + id: i64, + }, + /// A confirmation over a date sweep, carrying the boundary the operator was + /// shown. The counts are not here because they cannot be — see + /// io-harness#216. + StoreSweep { + /// The timestamp the sweep compares `sessions.created_at` against. + date: String, + }, + /// A confirmation over a compaction. Carries nothing: the operation takes no + /// argument, and the figures it reports are read either side of the call. + StoreCompact, + /// A confirmation over putting one file back, from the run that wrote it. + UndoFile { + /// The run whose snapshot the file comes from. + run_id: i64, + /// The path, as the operator named it. + path: String, + }, + /// A confirmation over reverse-applying one step's diff. + UndoStep { + /// The run the step belongs to. + run_id: i64, + /// The step, one-based as `/expand` shows them. + step: u32, + }, + /// A confirmation over undoing the whole turn — the typed form of what the + /// rewind chord does with two keystrokes. Carries nothing: `rewind::last_turn` + /// reads the head itself, and a run id carried from before the confirmation + /// could name a turn another `io` has since moved off the head. + UndoRun, + /// A confirmation over one export, carrying the bytes it will write. + /// + /// The content is built before the confirmation and carried rather than + /// rebuilt on acceptance, so the file written is the one the operator was + /// shown the size of — and so a store that stops answering between the two + /// keystrokes cannot turn an agreed export into a truncated file. + Export { + /// Where it goes, relative to the workspace root. + path: String, + /// Exactly what goes in it. For a trace this is io-harness's own string, + /// untouched. + content: String, + }, /// One directory of the workspace, in the order `list_dir` sorted it, so a /// chosen index reads straight back through `complete::pick`. The rows are /// last components rather than paths — see `complete::rows` for why — which diff --git a/src/rewind.rs b/src/rewind.rs index 416cbce..f9a5523 100644 --- a/src/rewind.rs +++ b/src/rewind.rs @@ -39,7 +39,7 @@ use crate::glyphs::Glyphs; use crate::theme::Tone; use io_harness::tools::Workspace; -use io_harness::{rewind_run, Error, Rewind, Session, Store}; +use io_harness::{rewind_run_observed, Error, Observer, Rewind, Session, Store}; /// What the armed prompt says before anything is undone. /// @@ -180,7 +180,11 @@ pub fn preview(session: &Session, store: &Store) -> Option { /// tries by hand: a two-turn undo still leaves a plausible-looking head, so a /// build that got this wrong would pass every test written against a longer /// conversation. -pub fn last_turn(session: &mut Session, store: &Store) -> Result, Error> { +pub fn last_turn( + session: &mut Session, + store: &Store, + observer: &dyn Observer, +) -> Result, Error> { let Some(head) = session.head() else { return Ok(None); }; @@ -197,7 +201,7 @@ pub fn last_turn(session: &mut Session, store: &Store) -> Result, // snapshots is a path whose write already passed the operator's gate, so // putting it back cannot reach anywhere the run could not. let workspace = Workspace::new(session.root()); - let rewound = rewind_run(&workspace, store, turn.run_id)?; + let rewound = rewind_run_observed(&workspace, store, turn.run_id, observer)?; let mut restored = Vec::new(); let mut declined = Vec::new(); diff --git a/src/stats.rs b/src/stats.rs index 26d5e3f..2f275c5 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -236,7 +236,13 @@ fn tallies(tallies: Vec, empty: &str) -> Vec { /// /// Powers of 1024 and named as such: this is a file on a disk, and a reader /// comparing it against what `ls` said should not have to convert. -fn bytes(count: u64) -> String { +/// +/// **Public since 0.27.0 so [`crate::store`] can reuse it rather than write a +/// second one.** Two spellings of the same quantity on two pages of one +/// interface is the shape 0.25.0 recorded when one fact acquired two holders — +/// and a store page reporting `8.2 MB` beside a stats page reporting `8.2 MiB` +/// is that defect in its most readable form. +pub fn bytes(count: u64) -> String { const UNITS: [(&str, u64); 4] = [ ("GiB", 1024 * 1024 * 1024), ("MiB", 1024 * 1024), diff --git a/src/store.rs b/src/store.rs new file mode 100644 index 0000000..2c173a2 --- /dev/null +++ b/src/store.rs @@ -0,0 +1,787 @@ +//! What the run store is holding, and the four things an operator can do about it. +//! +//! io-harness has kept every session, run, step, event, provider call, snapshot +//! and restore point in one SQLite file since this product had a home to put it +//! in — `~/.io-cli/runs.db` since 0.15.0, across twenty-six releases. Until this +//! one there was no retention policy, no rotation, no reclamation, and **no way +//! to look at it at all**. A file that only ever grows is a cost this product +//! created; this module is where it is finally answerable. +//! +//! Everything here is io-harness's arithmetic. [`Store::store_size`], +//! [`Store::session_size`], [`Store::delete_session`], [`Store::sweep_sessions`] +//! and [`Store::compact`] are all public and all uncalled anywhere else in this +//! crate. Nothing in this module computes a byte count, and nothing re-derives a +//! figure the store already returns — the one number io-cli must never invent is +//! the one an operator is about to authorise a deletion against. +//! +//! It follows the shape of [`crate::recall`] and [`crate::sessions`]: take a +//! `&Store`, return owned data, let the caller draw it. Nothing here touches a +//! terminal and nothing here reads a clock. +//! +//! # The distinction the whole module exists to make +//! +//! **A deletion does not shrink the file.** SQLite frees pages *into* the +//! database rather than out of it, so [`Store::delete_session`] moves bytes from +//! [`StoreSize::file_bytes`] into [`StoreSize::free_bytes`] and the file on disk +//! stays exactly the size it was. [`Store::compact`] — a `VACUUM` — is the only +//! reclamation available, because every store this crate has ever created was +//! created without `auto_vacuum` and `PRAGMA incremental_vacuum` therefore does +//! nothing on any existing file. +//! +//! An interface that reported a deletion as having reclaimed space would be +//! lying in precisely the way this release exists to stop, which is why +//! [`Freed`] carries both figures and why `tests/store.rs` asserts `file_bytes` +//! is **unchanged** by a removal rather than asserting it merely did not grow. +//! +//! # The four things that make a naive reading wrong +//! +//! **1. `None` and `Some(zeros)` are different answers.** [`Store::session_size`] +//! answers `None` for a session the store does not have and `Some` with zeros for +//! one that exists and holds nothing. io-harness keeps them apart on purpose — +//! `state/sessions.rs:305-309` says why: an operator sweeping a list of ids needs +//! to know which of them were already gone. [`Sized`] preserves the distinction; +//! mapping `None` to a zeroed [`SessionSize`] would tell that operator every id +//! was empty rather than that half of them no longer exist. +//! +//! **2. The sweep refuses, and the refusal is the interesting half.** A session +//! holding a run that can still be resumed is **not deleted**, and its id comes +//! back in [`Pruned::refused`]. io-harness's reasoning (`state/sessions.rs:370-378`) +//! is that a date is a policy applied to sessions nobody looked at, and a +//! crash-resumable tree that vanished because it was old is the worst outcome the +//! call could have. So a report that shows only what went tells the operator they +//! swept sessions that are still sitting there. +//! +//! **3. There is no public reader for `sessions.created_at`, which is the column +//! the sweep filters on.** `Store::sweep_sessions` selects +//! `WHERE created_at < ?1` (`state/sessions.rs:429`) and nothing in io-harness +//! returns that value — only `root` and `head_turn_id` are readable off the +//! `sessions` row. So the set a date selects **cannot be counted before the sweep +//! runs**, and the nearest substitute is worse than none: a session's earliest +//! `Turn::created_at` is always *later* than the session's own, because +//! `Session::open` writes the session row when the process starts and a turn is +//! written when the operator types. A preview built on turns therefore +//! under-states the deletion, which is the direction that costs somebody data. +//! +//! Filed as io-harness#216. Shipped around by [`Swept`]: the operator authorises +//! the **rule**, which is what the operation is, and every figure they are shown +//! afterwards is one io-harness returned. See `US-IO-CLI-0.27.0-I02`. +//! +//! **4. Memory survives a session deletion, and its recall rows do not.** +//! io-harness's entries belong to the workspace rather than to the session +//! (`state/sessions.rs:368-373`), so removing a session unlearns nothing — but +//! the session's **restore points go**, and their count is in +//! [`Pruned::restore_points`]. That is the sentence an operator needs before they +//! agree, because it is the one that bites later: they delete an old session, +//! and reach for its rewind a week afterwards. +//! +//! # What is deliberately not here +//! +//! No automatic retention, rotation, sweeping or compaction. No startup sweep, no +//! size threshold, no configuration key that turns any of it on. The roadmap's +//! own words are *none of which happens on its own*, and io-harness makes the +//! argument itself: a call that rewrites the whole database and needs the file's +//! size again in free space while it runs is one an operator makes knowingly. +//! +//! And **nothing here is reachable by a model.** io-harness's workspace tool set +//! contains nothing that can call any of these, this release adds no tool, no MCP +//! server and no skill, and `tests/dependencies.rs` asserts that each of the three +//! destructive calls is named in exactly one module — this one. + +use io_harness::{Error, Pruned, SessionSize, Store, StoreSize}; + +/// The size of one session, or the fact that there is no such session. +/// +/// Two variants and not an `Option` at the call site, because the +/// two answers need different sentences and a bare `Option` invites the caller to +/// `unwrap_or_default()` — which is exactly the collapse trap 1 describes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Sized { + /// The session is in the store. Zeros here mean it holds nothing, which is a + /// fact rather than a gap. + Holds(SessionSize), + /// There is no session of that id. Not an error, and not an empty session. + Absent, +} + +impl Sized { + /// The figures, when there are any. + pub fn size(&self) -> Option<&SessionSize> { + match self { + Sized::Holds(size) => Some(size), + Sized::Absent => None, + } + } +} + +/// One row of the store page: a session and what it costs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Session { + /// The session id, which is what every operation here addresses. + pub id: i64, + /// The workspace root the session was opened over, as the store holds it. + pub root: String, + /// What it costs, or that it is gone. Read per session rather than derived + /// from a total, so a row and the operation that acts on it agree. + pub sized: Sized, +} + +/// Everything a store panel needs, and nothing it has to compute. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct View { + /// The file's own page arithmetic: `page_size × page_count` for + /// [`StoreSize::file_bytes`] and `page_size × freelist_count` for + /// [`StoreSize::free_bytes`], plus the session and run counts and the + /// per-table breakdown. Read from [`Store::store_size`] and never assembled + /// here. + pub size: StoreSize, + /// One row per session the store holds, in the order [`crate::sessions`] + /// returns them. + pub rows: Vec, + /// The session scan hit [`crate::sessions::MAX_RUNS_SCANNED`], so [`View::rows`] + /// is a prefix rather than the whole store. The totals in [`View::size`] are + /// unaffected — they come from the file, not from the scan — which is exactly + /// why a cut list beside a complete total has to say so. + pub cut: bool, +} + +impl View { + /// The part of the file that is already free inside it. + /// + /// Named rather than left to a caller's subtraction because the interesting + /// quantity is the one nobody thinks to ask for: bytes a deletion has already + /// released that are still occupying disk, and that only [`compact`] returns. + pub fn reclaimable(&self) -> u64 { + self.size.free_bytes + } +} + +/// Read what the store is holding. +/// +/// Every row is a read. This function cannot write to the store. +/// +/// The session list comes from [`crate::sessions::recent`], which is the list +/// this product already shows for `/resume` — so a session an operator sees here +/// is one they can recognise there, and the scan bound is the same one. Each +/// row's size is read individually, because a total divided among rows would be +/// io-cli inventing an attribution io-harness did not make. +pub fn view(store: &Store) -> Result { + let size = store.store_size()?; + let (recent, cut) = crate::sessions::recent(store)?; + let mut rows = Vec::with_capacity(recent.len()); + for session in recent { + rows.push(Session { + id: session.id, + root: session.root, + sized: sized(store, session.id)?, + }); + } + Ok(View { size, rows, cut }) +} + +/// What one session costs, keeping "absent" apart from "empty". +/// +/// See trap 1. The mapping is the whole function and it is the point of it. +pub fn sized(store: &Store, session_id: i64) -> Result { + Ok(match store.session_size(session_id)? { + Some(size) => Sized::Holds(size), + None => Sized::Absent, + }) +} + +/// What removing one session did, and what it did **not** do to the file. +/// +/// Both halves together, from two reads of [`Store::store_size`] taken either +/// side of the removal, because the fact worth reporting is a relationship +/// between them: rows went, `free_bytes` rose, and the file on disk did not move. +/// A caller handed only [`Pruned`] would have no way to say the third thing, and +/// the third thing is the one that surprises people. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Removed { + /// io-harness's own account of what was deleted. + pub pruned: Pruned, + /// The file before. + pub before: StoreSize, + /// The file after. [`StoreSize::file_bytes`] equals `before`'s; only + /// [`StoreSize::free_bytes`] has moved. + pub after: StoreSize, +} + +impl Removed { + /// Whether the file on disk changed size. Always false, and asserted rather + /// than assumed — if it is ever true, either SQLite's behaviour or this + /// module's understanding of it has changed and the report must not go on + /// claiming otherwise. + pub fn file_moved(&self) -> bool { + self.before.file_bytes != self.after.file_bytes + } + + /// How much more of the file is free than was free before. + pub fn freed_into_file(&self) -> u64 { + self.after.free_bytes.saturating_sub(self.before.free_bytes) + } +} + +/// Remove one session and everything keyed to it. +/// +/// **Final. There is no undelete, and this release does not build one.** What +/// survives is io-harness's decision and is worth knowing before agreeing: the +/// agent's memory entries belong to the workspace and are untouched, while the +/// session's *recall* rows go with it because they name runs that no longer +/// exist, and so do its **restore points** — the count of which is in +/// [`Pruned::restore_points`]. An operator who removes an old session has removed +/// its rewind. +/// +/// Deleting a session the store does not have succeeds and reports nothing, which +/// is io-harness's own behaviour and is not softened here. +pub fn remove(store: &Store, session_id: i64, live: i64) -> Result { + // Before anything is read, let alone deleted. See [`Removal`]. + if session_id == live { + return Ok(Removal::Live); + } + let before = store.store_size()?; + let pruned = store.delete_session(session_id)?; + let after = store.store_size()?; + Ok(Removal::Done(Box::new(Removed { + pruned, + before, + after, + }))) +} + +/// A timestamp the sweep may safely be given. +/// +/// **This exists because the comparison is lexical and the blast radius is the +/// whole store.** `Store::sweep_sessions` selects `WHERE created_at < ?1` against +/// a `%Y-%m-%dT%H:%M:%fZ` **text** column, so any string sorting above `'2'` +/// selects every session io-harness has ever written. Two entirely plausible +/// inputs do that: +/// +/// - `/store sweep 2026-8-1` — an unpadded month. `'2026-08-28T…' < '2026-8-1'` +/// is true at index five, `'0'` against `'8'`, so *today's* sessions go too. +/// - `/store sweep yesterday` — every ISO stamp starts with `'2'` and every +/// letter sorts above it. The entire store, across every workspace. +/// +/// Neither is a typo an operator would catch by reading the confirmation back, +/// because the confirmation echoes the string they typed and it looks like what +/// they meant. So the shape is checked here, before anything is offered. +/// +/// Two spellings are accepted and one of them is normalised: a bare +/// `YYYY-MM-DD`, which becomes midnight of that day, and a full +/// `YYYY-MM-DDTHH:MM:SS(.mmm)Z` as the store writes them. Everything else is +/// refused by name. +/// +/// **No date library, and no clock.** This validates a *shape* — it does not ask +/// whether the thirty-first of February exists, because io-harness's comparison +/// does not either, and a `2026-02-31` boundary sorts exactly where an operator +/// would expect it to. What it refuses is the class of input that means +/// something entirely different from what was typed. +pub fn boundary(input: &str) -> Result { + let digits = |s: &str| s.chars().all(|c| c.is_ascii_digit()); + let date = input.get(..10).unwrap_or_default(); + let ok_date = date.len() == 10 + && digits(&date[0..4]) + && &date[4..5] == "-" + && digits(&date[5..7]) + && &date[7..8] == "-" + && digits(&date[8..10]); + if !ok_date { + return Err(format!( + "`{input}` is not a date. The store compares timestamps as text, so a \ + word or an unpadded month would sweep far more than you meant — write \ + it as 2026-08-01, or in full as 2026-08-01T00:00:00.000Z" + )); + } + if input.len() == 10 { + // Midnight of that day, which is what "before the first of August" means + // to the person who typed it. + return Ok(format!("{date}T00:00:00.000Z")); + } + let rest = &input[10..]; + let shaped = rest.len() >= 10 + && rest.starts_with('T') + && rest.ends_with('Z') + && digits(&rest[1..3]) + && &rest[3..4] == ":" + && digits(&rest[4..6]) + && &rest[6..7] == ":" + && digits(&rest[7..9]); + if !shaped { + return Err(format!( + "`{input}` is not a timestamp the store would have written. Use \ + 2026-08-01, or the full form 2026-08-01T00:00:00.000Z" + )); + } + Ok(input.to_string()) +} + +/// Whether one session may be removed at all. +/// +/// **An operator can see their own session on the `/store` page — with its root, +/// which is the very thing that makes it recognisable — and there is nothing in +/// io-harness to stop them deleting it.** `delete_session` refuses nothing; the +/// resumable-run refusal lives only in `sweep_sessions`. No foreign keys are +/// enforced, so the removal succeeds, the process carries on writing turn rows +/// into a session that no longer exists, and every later `/undo` and `/export` +/// reports an empty session — a false report, from a session that is running. +/// +/// So the live session is refused here, by the one caller that knows which it +/// is. This is io-cli's own rule and not io-harness's, and it is stated as such. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Removal { + /// It went. + Done(Box), + /// It is the conversation this process is having, and was not touched. + Live, +} + +/// What a date sweep did. +/// +/// The counts live here and **not** in a preview, because they cannot be known +/// before the sweep runs — see trap 3 and io-harness#216. This is the whole of +/// the ship-around: the operator authorises the rule, and this is the answer. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Swept { + /// The boundary that was applied, echoed back so the report says what it is + /// the report *of*. + pub before_date: String, + /// io-harness's own account: what went, and every id it refused. + pub pruned: Pruned, + /// The file before the sweep. + pub before: StoreSize, + /// The file after it. As with [`Removed`], `file_bytes` has not moved. + pub after: StoreSize, +} + +impl Swept { + /// The sessions that were refused for still being resumable. + /// + /// A borrow rather than a count, because the report names them: an operator + /// told "one was refused" has to go looking, and the id is the thing they + /// would go looking for. + pub fn refused(&self) -> &[i64] { + &self.pruned.refused + } +} + +/// Remove every session created strictly before `before_date`. +/// +/// `before_date` is a timestamp string compared against `sessions.created_at`, +/// which is a `strftime('%Y-%m-%dT%H:%M:%fZ')` text column — a string comparison +/// is what the storage actually does, so that is what io-harness takes rather +/// than a duration measured against a clock the store does not have. The +/// comparison is **strictly** before: a session created at exactly `before_date` +/// survives. +/// +/// A session holding a resumable run is refused rather than deleted and comes +/// back in [`Swept::refused`]. That is not an error and must not be reported as +/// one. +pub fn sweep(store: &Store, before_date: &str) -> Result { + let before = store.store_size()?; + let pruned = store.sweep_sessions(before_date)?; + let after = store.store_size()?; + Ok(Swept { + before_date: before_date.to_string(), + pruned, + before, + after, + }) +} + +/// What a compaction actually returned to the filesystem. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Freed { + /// The bytes the file shrank by, **measured** by io-harness as the difference + /// between the file's size before and after — never inferred from the + /// freelist. Zero is an honest answer for a store with nothing free, and the + /// caller says so rather than showing an empty success. + pub bytes: u64, + /// The file before. + pub before: StoreSize, + /// The file after. + pub after: StoreSize, +} + +impl Freed { + /// Whether anything was reclaimed at all. + pub fn is_nothing(&self) -> bool { + self.bytes == 0 + } +} + +/// Rewrite the database without its free pages, and say how much that returned. +/// +/// **This is the only reclamation available.** Every store this crate has created +/// was created without `auto_vacuum`, so `PRAGMA incremental_vacuum` does nothing +/// on any existing file, and a deletion on its own never shrinks anything. +/// +/// **It is expensive and the caller must say so first.** io-harness's own +/// documentation is explicit: it rewrites the whole database, it needs free disk +/// space of roughly the file's own size while it runs, it cannot run inside a +/// transaction, and on a large store it is not quick. That is why it is a call an +/// operator makes knowingly and never something a deletion does on their behalf. +pub fn compact(store: &Store) -> Result { + let before = store.store_size()?; + let bytes = store.compact()?; + let after = store.store_size()?; + Ok(Freed { + bytes, + before, + after, + }) +} + +// --------------------------------------------------------------------------- +// The surfaces +// --------------------------------------------------------------------------- + +/// The row every confirmation starts on: the one that does nothing. +/// +/// Index 0, always, in every confirmation this module builds. A `Picker` opens +/// on its first row, so the keystroke an operator gives by reflex is the one +/// that declines — which is the whole of criterion F5 and the reason +/// `tests/store.rs` asserts the *index* rather than the presence of the row. +pub const LEAVE_IT: &str = "leave it"; + +/// Whether a chosen row in one of these confirmations acts. +/// +/// One line, and it is in the library rather than in the driver on purpose. +/// `src/main.rs` is `[[bin]] name = "io"` and nothing under `tests/` can link it, +/// so a `index != 0` written at the three call sites would be the one decision in +/// this module that could never be tested or sabotaged — which is 0.22.0's +/// recorded lesson (*any decision worth testing has to live in the library*) and +/// 0.23.0's (*all three HIGH defects were in `src/main.rs`*). +/// +/// It is also what makes criterion F5 assertable as a property rather than as a +/// label comparison: row 0 declines, whatever it is called. +pub fn acts(index: usize) -> bool { + index != 0 +} + +/// The `/store` page, committed into the scrollback. +/// +/// Into the scrollback and never into a pane, for the reason `/status`, `/cost` +/// and `/stats` all give: the viewport is eight rows and cannot grow, and the +/// terminal's own search, selection and copy already work on everything above +/// it. This is a page a reader wants to keep beside the sessions it accounts for. +pub fn committed( + store: &Store, + theme: &crate::theme::Theme, + width: u16, +) -> Result>, String> { + let view = view(store).map_err(|e| e.to_string())?; + let mut rows: Vec = vec![ + crate::page::Row::heading("the file".to_string()), + crate::page::Row::fact("on disk", crate::stats::bytes(view.size.file_bytes)), + // Named as free *inside* the file rather than as "reclaimable", because + // the second word invites the reading that a deletion returned it. It did + // not: this is space a `VACUUM` could return, and nothing else can. + crate::page::Row::fact("free inside it", crate::stats::bytes(view.reclaimable())), + crate::page::Row::fact("sessions", view.size.sessions.to_string()), + crate::page::Row::fact("runs", view.size.runs.to_string()), + ]; + + if view.reclaimable() > 0 { + rows.push(crate::page::Row::caveat(format!( + "{} is already free inside the file and still occupies the disk; \ + `/store compact` is the only thing that returns it", + crate::stats::bytes(view.reclaimable()) + ))); + } + + rows.push(crate::page::Row::Blank); + rows.push(crate::page::Row::heading("by table".to_string())); + if view.size.tables.is_empty() { + rows.push(crate::page::Row::note("no tables reported".to_string())); + } + for (table, bytes) in &view.size.tables { + rows.push(crate::page::Row::fact( + table.clone(), + crate::stats::bytes(*bytes), + )); + } + + rows.push(crate::page::Row::Blank); + rows.push(crate::page::Row::heading("by session".to_string())); + if view.rows.is_empty() { + rows.push(crate::page::Row::note("no sessions recorded".to_string())); + } + for session in &view.rows { + rows.push(crate::page::Row::fact( + format!("{} · {}", session.id, session.root), + session_figures(&session.sized), + )); + } + if view.cut { + rows.push(crate::page::Row::caveat(format!( + "only the {} most recent sessions are listed; the totals above are \ + the whole file", + view.rows.len() + ))); + } + // **The total and the list count different things, and the difference is + // real rather than a rounding.** `store_size().sessions` counts every row in + // `sessions`; the list comes from `crate::sessions::recent`, which walks runs + // back to sessions and therefore cannot see a session that was opened and + // never used — every `io` somebody started and quit out of. Those sessions + // are invisible here and are exactly what a sweep will remove, so the page + // says how many there are rather than letting the two numbers disagree in + // silence. Found by the adversarial review, not by a test. + let listed = view.rows.len() as u64; + if !view.cut && view.size.sessions > listed { + rows.push(crate::page::Row::caveat(format!( + "{} session{} held no run and {} not listed above — an `io` that was \ + opened and never used. A sweep removes {} too", + view.size.sessions - listed, + if view.size.sessions - listed == 1 { + "" + } else { + "s" + }, + if view.size.sessions - listed == 1 { + "is" + } else { + "are" + }, + if view.size.sessions - listed == 1 { + "it" + } else { + "them" + }, + ))); + } + + rows.push(crate::page::Row::Blank); + rows.push(crate::page::Row::note( + "`/store rm ` removes one session · `/store sweep ` removes \ + every session created before it · `/store compact` returns the free \ + pages to the disk" + .to_string(), + )); + + Ok(crate::page::commit( + "What the store holds", + &rows, + theme, + width, + )) +} + +/// One session's figures, or the sentence for a session that is not there. +/// +/// The two answers are different sentences rather than one with zeros in it — +/// see trap 1. An operator reading `0 turns` about a session that no longer +/// exists would go looking for an empty session. +fn session_figures(sized: &Sized) -> String { + match sized { + Sized::Holds(size) => format!( + "{} · {} turn{} · {} run{}", + crate::stats::bytes(size.bytes), + size.turns, + if size.turns == 1 { "" } else { "s" }, + size.runs, + if size.runs == 1 { "" } else { "s" }, + ), + Sized::Absent => "no such session".to_string(), + } +} + +/// The confirmation for removing one session: what it holds, and what goes with +/// it. +/// +/// The title carries the figures, because a confirmation is a confirmation of +/// something specific rather than of a keystroke. The restore-point sentence is +/// here rather than in the report for the same reason: it is the fact an +/// operator needs *before* they agree, not the one they discover afterwards. +pub fn confirm_remove(id: i64, sized: &Sized, live: i64) -> (String, Vec) { + if id == live { + return ( + format!("Session {id} is the conversation you are in"), + vec![crate::picker::Row::with_detail( + LEAVE_IT, + "io will not remove the session it is running; quit and remove it \ + from another one", + )], + ); + } + let title = match sized { + Sized::Holds(size) => format!( + "Remove session {id}? {} · {} turn{} · {} run{}", + crate::stats::bytes(size.bytes), + size.turns, + if size.turns == 1 { "" } else { "s" }, + size.runs, + if size.runs == 1 { "" } else { "s" }, + ), + Sized::Absent => format!("There is no session {id}"), + }; + let mut rows = vec![crate::picker::Row::with_detail( + LEAVE_IT, + "nothing is removed", + )]; + if sized.size().is_some() { + rows.push(crate::picker::Row::with_detail( + format!("remove session {id}"), + "final; its restore points go with it, its memory does not", + )); + } + (title, rows) +} + +/// The confirmation for a date sweep: the rule, and the refusal policy. +/// +/// **Not a count.** See trap 3 and io-harness#216: the set a date selects cannot +/// be read before the sweep runs, and the one substitute available under-states +/// it. So the operator authorises the rule — which is what the operation is — +/// and [`swept_report`] carries the figures afterwards. +pub fn confirm_sweep(before_date: &str) -> (String, Vec) { + ( + format!("Sweep every session created before {before_date}?"), + vec![ + crate::picker::Row::with_detail(LEAVE_IT, "nothing is removed"), + crate::picker::Row::with_detail( + "sweep them", + "a session still holding a resumable run is refused and named; \ + the figures are reported once it has run", + ), + ], + ) +} + +/// The confirmation for a compaction: what it costs while it runs. +/// +/// The cost sentence is io-harness's own and is stated before the call rather +/// than discovered by a full disk: a `VACUUM` rewrites the whole database and +/// needs roughly the file's own size free while it does. +pub fn confirm_compact(size: &StoreSize) -> (String, Vec) { + ( + format!( + "Compact the store? {} free inside a {} file", + crate::stats::bytes(size.free_bytes), + crate::stats::bytes(size.file_bytes), + ), + vec![ + crate::picker::Row::with_detail(LEAVE_IT, "the file is left as it is"), + crate::picker::Row::with_detail( + "compact it", + "rewrites the whole database and needs about its own size free \ + on disk while it runs", + ), + ], + ) +} + +/// What to say after a removal. +/// +/// Three sentences, and the third is the one this module exists for: rows went, +/// the free space inside the file rose, and **the file on disk did not move**. +pub fn removed_report(removed: &Removed) -> Vec { + let pruned = &removed.pruned; + let mut lines = vec![format!( + "removed {} session{}: {} turn{}, {} run{}, {} row{}, {}", + pruned.sessions, + if pruned.sessions == 1 { "" } else { "s" }, + pruned.turns, + if pruned.turns == 1 { "" } else { "s" }, + pruned.runs, + if pruned.runs == 1 { "" } else { "s" }, + pruned.rows, + if pruned.rows == 1 { "" } else { "s" }, + crate::stats::bytes(pruned.bytes), + )]; + if pruned.restore_points > 0 { + lines.push(format!( + "{} restore point{} went with it; the workspace's memory did not", + pruned.restore_points, + if pruned.restore_points == 1 { "" } else { "s" }, + )); + } + lines.push(freeing_sentence( + removed.freed_into_file(), + removed.after.file_bytes, + removed.file_moved(), + )); + lines +} + +/// What to say after a sweep — including, always, what it refused. +pub fn swept_report(swept: &Swept) -> Vec { + let pruned = &swept.pruned; + let mut lines = vec![format!( + "swept {} session{} created before {}: {} turn{}, {} run{}, {}", + pruned.sessions, + if pruned.sessions == 1 { "" } else { "s" }, + swept.before_date, + pruned.turns, + if pruned.turns == 1 { "" } else { "s" }, + pruned.runs, + if pruned.runs == 1 { "" } else { "s" }, + crate::stats::bytes(pruned.bytes), + )]; + // Never conditional on there being any. A sweep that refused nothing says so, + // because "refused none" and "we did not check" are the same silence + // otherwise — and the refusal is the half an operator has to act on. + if swept.refused().is_empty() { + lines.push("nothing was refused".to_string()); + } else { + lines.push(format!( + "refused {}: {} still holding a resumable run", + swept + .refused() + .iter() + .map(|id| id.to_string()) + .collect::>() + .join(", "), + if swept.refused().len() == 1 { + "it is" + } else { + "they are" + }, + )); + } + lines.push(freeing_sentence( + swept + .after + .free_bytes + .saturating_sub(swept.before.free_bytes), + swept.after.file_bytes, + swept.before.file_bytes != swept.after.file_bytes, + )); + lines +} + +/// What to say after a compaction. +pub fn freed_report(freed: &Freed) -> Vec { + if freed.is_nothing() { + return vec![format!( + "nothing to reclaim; the file is still {}", + crate::stats::bytes(freed.after.file_bytes) + )]; + } + vec![format!( + "returned {} to the disk; the file is now {}", + crate::stats::bytes(freed.bytes), + crate::stats::bytes(freed.after.file_bytes), + )] +} + +/// The sentence that keeps a deletion honest. +/// +/// `moved` is asserted rather than assumed. It is always false — SQLite frees +/// pages into the file — and if it is ever true then either SQLite's behaviour or +/// this module's understanding of it has changed, and the report must say the +/// surprising thing rather than go on printing the reassuring one. +fn freeing_sentence(freed_into_file: u64, file_bytes: u64, moved: bool) -> String { + if moved { + return format!( + "the file is now {} — it changed size, which a deletion is not \ + supposed to do", + crate::stats::bytes(file_bytes) + ); + } + format!( + "{} more is free inside the file; the file on disk is still {} — \ + `/store compact` is what returns it", + crate::stats::bytes(freed_into_file), + crate::stats::bytes(file_bytes), + ) +} diff --git a/src/triage.rs b/src/triage.rs index 8b775f0..13345b3 100644 --- a/src/triage.rs +++ b/src/triage.rs @@ -183,11 +183,21 @@ pub const TRIAGE: &[(&str, Disposition, &str)] = &[ Disposition::Status, "the web field, with its verdict", ), - ( - "speculated", - Disposition::Silent, - "`io exec --json` and the durable trace; nothing about the step's own events moves", - ), + // **0.27.0 — the one silence in this table with no route to any surface an + // operator uses.** It read "`io exec --json` and the durable trace", and both + // of those are places a person goes deliberately, afterwards, having already + // suspected something. A route is supposed to name where the fact reaches + // somebody who was not looking for it, and this one never did. + // + // What it says is worth a line, too: reads started before the model had + // finished asking, and how many were thrown away. Started-and-discarded is + // work that was paid for, and it is the only figure in this product an + // operator can act on by turning speculation off. + // + // The other eight silences reviewed in the same pass keep their routes, + // which are better arguments than drawing them would be — see + // `US-IO-CLI-0.27.0-I04`. + ("speculated", Disposition::Line, "the speculation line"), ( "plugin_dropped", Disposition::Silent, diff --git a/src/undo.rs b/src/undo.rs new file mode 100644 index 0000000..8890b17 --- /dev/null +++ b/src/undo.rs @@ -0,0 +1,207 @@ +//! Putting work back at the granularity the operator actually meant. +//! +//! [`crate::rewind`] has undone a whole run since 0.4.0, and for three years of +//! releases that was the only instrument. It is the wrong size for most +//! mistakes: *this one file went wrong* and *that step should not have happened* +//! are the two things an operator reaches for, and an undo whose cost is larger +//! than the mistake it fixes is an undo nobody uses. +//! +//! Both granularities are public in io-harness and neither had ever been called +//! from this crate — [`io_harness::rewind`] for one path, and +//! [`io_harness::rewind_step_observed`] for one step's recorded diff. +//! +//! # The whole-run case changed too, and that is what makes two events reachable +//! +//! `src/rewind.rs` called the plain `rewind_run`, and **`EventKind::Rewound` is +//! emitted only inside `rewind_run_observed`** (`run.rs:804`). So in this +//! product's entire history that event has never fired once. The same is true of +//! `EventKind::Reverted`, which only `rewind_step_observed` emits (`run.rs:1011`). +//! +//! **What that buys is not a line on screen, and an earlier draft of this file +//! claimed it was.** Both kinds are `Disposition::Silent` in `crate::triage` and +//! route to io-cli's own rewind summary — written from the value the call +//! returned, which `crate::rewind` explains at length — so `src/events.rs` has +//! no arm for either and correctly draws nothing. What the observed forms buy is +//! that the events now **exist**: they reach `[[hook]]` observers, the +//! `io exec --json` stream and the durable trace, none of which they had ever +//! reached. Recorded because the adversarial review found the overclaim. +//! +//! # Three traps, and the middle one will bite an operator +//! +//! **1. Reverting a step is order-sensitive, and io-harness will not guess.** +//! Reverse-applying a hunk while a *later* step's change still sits on top of it +//! finds context that has moved, and the honest answer is +//! [`io_harness::Reverted::Stale`] with the file untouched — +//! never a fuzzy match that quietly corrupts it. Newest step first is the order +//! that works, and [`step_advice`] is what says so on screen when a stale answer +//! comes back, because "nothing happened" without that sentence reads as a bug. +//! +//! **2. A restore does not know about an edit made afterwards.** io-harness puts +//! the file back from the snapshot taken before the run first wrote it, and does +//! not compare that against what is on disk now. An edit the operator made by +//! hand after the turn is overwritten without a word, and io-cli cannot detect it +//! — the snapshot is not readable from here. `src/rewind.rs` already discloses +//! this before its second keystroke; [`confirm_file`] does the same. +//! +//! **3. The four answers are four answers.** [`io_harness::Rewind`] distinguishes +//! `Restored`, `Removed`, `NotKept(why)` and `NotRecorded`, and they mean +//! different things: the first two changed the workspace, the last two changed +//! nothing at all. Collapsing `NotKept` and `NotRecorded` into one sentence would +//! tell an operator "nothing to undo" about a file the run very much wrote, whose +//! previous contents were simply over the snapshot cap. +//! +//! # What the policy is, and whose it is +//! +//! io-harness's. [`io_harness::rewind`] restores through `Workspace::write_file`, +//! so the same path policy a write obeys applies; a **removal** asks +//! `Workspace::check_path` and refuses anything that is not an outright +//! `Effect::Allow`, because a deletion is not content a human can inspect +//! afterwards. Nothing here re-implements either check, and nothing here softens +//! a refusal. + +use io_harness::tools::Workspace; +use io_harness::{Error, Observer, Reverted, Rewind, Store}; + +/// Which granularity an undo was asked for. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Grain { + /// One named path, from this run's snapshot. + File(String), + /// One step's recorded diff, reverse-applied. + Step(u32), + /// Everything the run did. + Run, +} + +// `Grain` carries no `label()`. The confirmations build their own sentences — +// `confirm_file` and `confirm_step` — and the run arm never renders a label at +// all, so a method that formatted one would be reachable only from a test. + +/// Put one file back, from the snapshot taken before this run first wrote it. +/// +/// The answer is io-harness's own [`Rewind`], returned whole rather than reduced +/// to a boolean: see trap 3. `Restored` and `Removed` changed the workspace; +/// `NotKept` and `NotRecorded` changed nothing and say why. +pub fn one_file( + workspace: &Workspace, + store: &Store, + run_id: i64, + path: &str, +) -> Result { + io_harness::rewind(workspace, store, run_id, path) +} + +/// Undo one step's recorded diff, announcing it. +/// +/// **The observed form, so `EventKind::Reverted` fires.** One entry per path the +/// step wrote, in the order it wrote them; a step that wrote nothing comes back +/// empty and is not an error — asking to undo a step that only read files is a +/// reasonable question with a boring answer. +pub fn one_step( + workspace: &Workspace, + store: &Store, + run_id: i64, + step: u32, + observer: &dyn Observer, +) -> Result, Error> { + io_harness::rewind_step_observed(workspace, store, run_id, step, observer) +} + +// **There is deliberately no `whole_run` here.** The whole-turn undo is +// `crate::rewind::last_turn`, which does what this module cannot: it moves the +// conversation head back, by compare-and-swap, and re-opens the session. A thin +// wrapper over `rewind_run_observed` beside it would be a second entry point to +// one act, and the first draft of this module shipped exactly that — public, +// tested, and called by nothing. This codebase has shipped tested-but-unreachable +// surfaces in 0.20.0, 0.21.0 and 0.26.0; the adversarial review caught this one +// before it became the fourth. + +/// What one file's answer says, in the operator's words. +/// +/// Four sentences for four answers. The two that changed nothing say so first, +/// because that is the fact an operator has to notice. +pub fn said(path: &str, answer: &Rewind) -> String { + match answer { + Rewind::Restored(_) => format!("{path} is back as it was before the run"), + Rewind::Removed => format!("{path} is gone — the run had created it"), + Rewind::NotKept(why) => { + format!("{path} is unchanged: its previous contents were not kept ({why})") + } + Rewind::NotRecorded => { + format!("{path} is unchanged: this run recorded no restore point for it") + } + } +} + +/// What one step's answer says, per path. +pub fn step_said(path: &str, answer: &Reverted) -> String { + match answer { + Reverted::Applied(_) => format!("{path} is back to what it was before that step"), + Reverted::Stale(why) => format!("{path} is unchanged: {why}"), + // **`NoHunk` is the third variant and it is not exotic.** io-harness names + // it for a row written before hunks were kept, and for a file whose + // previous contents were not kept at all — over the snapshot cap, or not + // text. That is the same class `Rewind::NotKept` gets a written sentence + // for one function up, and it deserves one here rather than a Rust debug + // string with quotes in it. The first version of this function let it + // fall to a `{other:?}` wildcard, and the adversarial review found it. + Reverted::NoHunk(why) => format!( + "{path} is unchanged: no diff was stored for that step ({why}) — \ + `/undo {path}` puts the whole file back instead" + ), + // `Reverted` may gain a fourth variant in a later io-harness. Reporting + // the conservative thing — nothing changed — is the arm that cannot + // mislead, because the alternative is claiming a restore this build did + // not understand. + _ => format!("{path} is unchanged: this build does not know that answer"), + } +} + +/// The sentence to add when a step's revert came back stale. +/// +/// Trap 1, said out loud. A stale answer with no explanation reads as a bug — +/// the operator asked for an undo, was told nothing happened, and has no way to +/// know that undoing the *newer* step first is what makes this one apply. +pub fn step_advice(answers: &[(String, Reverted)]) -> Option { + answers + .iter() + .any(|(_, answer)| matches!(answer, Reverted::Stale(_))) + .then(|| { + "a later step is still standing on top of this one — reverting is \ + order-sensitive, so undo the newest step first and this one applies" + .to_string() + }) +} + +/// The confirmation for putting one file back. +/// +/// Row 0 declines, like every confirmation this release adds. The disclosure is +/// trap 2 and it is in the row that acts, because it is what the operator is +/// agreeing to rather than a caveat about what they are declining. +pub fn confirm_file(path: &str) -> (String, Vec) { + ( + format!("Put {path} back as it was before this run?"), + vec![ + crate::picker::Row::with_detail(crate::store::LEAVE_IT, "the file is left as it is"), + crate::picker::Row::with_detail( + format!("put {path} back"), + "any edit you made by hand after the turn is overwritten", + ), + ], + ) +} + +/// The confirmation for undoing one step. +pub fn confirm_step(step: u32) -> (String, Vec) { + ( + format!("Undo what step {step} wrote?"), + vec![ + crate::picker::Row::with_detail(crate::store::LEAVE_IT, "nothing is changed"), + crate::picker::Row::with_detail( + format!("undo step {step}"), + "reverse-applies that step's diff; a newer step on the same lines \ + makes it stale and nothing changes", + ), + ], + ) +} diff --git a/tests/commands.rs b/tests/commands.rs index 2fe1683..83e82ad 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -84,6 +84,11 @@ fn the_commands_are_the_commands() { "/profile", "/effort", "/contain", + // 0.27.0 — the undo that is the size of the mistake, taking the + // `Turn` slot `/contain` left when it was re-filed into `Session`. + // `Turn` was ten of ten and the bound was not widened, which is the + // fourth time this product has made that correction. + "/undo", // 0.12.0 — the planning phase stopped being something // `[app.io-cli.containment]` switched on by accident, so it needs a // switch of its own. @@ -98,6 +103,16 @@ fn the_commands_are_the_commands() { // this list, which is the rule every alias here follows. "/cost", "/stats", + // 0.27.0 — the third page about work already done, and the first that + // can also change it. Under **inspect** because that is what its bare + // form does, with every destructive verb behind a confirmation whose + // row 0 does nothing; it and `/export` take that group to ten, which + // is the bound. + "/store", + // 0.27.0 — the other end of the same question: `/store` is what is + // being kept, this is how the work gets out. It takes `Inspect` to + // ten, which is the bound. + "/export", // 0.24.0 — beside `/stats` because `/stats` is the only other row // that says the word: that page counts how the gates went, and until // this release nothing in the product could say what a gate was. It @@ -309,9 +324,10 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio // release growing a command and losing another. assert_eq!( COMMANDS.len(), - 33, - "0.26.0 adds exactly one command, `/effort`, on top of the thirty-two \ - 0.25.0 shipped; a thirty-fourth here means one arrived unrecorded", + 36, + "0.27.0 adds exactly three commands — `/store`, `/export` and `/undo` — on \ + top of the thirty-three 0.26.0 shipped; a thirty-seventh here means one \ + arrived unrecorded", ); } @@ -328,7 +344,12 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio #[test] fn f1_effort_parses_three_levels_the_absence_and_the_question() { use io_cli::commands::Reasoning; - let parse = |text: &str| commands::parse(text, &defaults(), &DARK); + // `parse` takes the word without its slash — the driver strips it before the + // call, which is why every other test in this file types `continue` rather + // than `/continue`. Written as a helper so the cases below read as the + // operator types them. + let parse = + |text: &str| commands::parse(text.strip_prefix('/').unwrap_or(text), &defaults(), &DARK); assert_eq!( parse("effort low"), @@ -980,18 +1001,29 @@ fn the_writers_are_configure_commands_and_skills_is_the_inspection() { "`/skills` opens a list and writes nothing, which is what Inspect means", ); - // The room the move made is real, not an accounting trick: Inspect is under - // the bound with a command to spare. `f13_no_group_is_longer_than_ten` is - // still the gate; this only says the release did not spend every seat it - // freed. + // **0.27.0 spends the last seat this move freed, and that is a decision + // rather than an oversight.** The 0.19.0 form of this assertion was + // `inspect < 10` — the release that made the room saying it had not used all + // of it. `/store` and `/export` take the group to exactly ten, which is the + // bound `f13_no_group_is_longer_than_ten` enforces, so the sentence this gate + // makes changes from *there is room left* to *the room is now gone*: the next + // command that would belong in `Inspect` re-files one that is in the wrong + // group rather than widening the bound, which is what `Turn` did for `/undo` + // in this same release and what 0.19.0, 0.22.0 and 0.26.0 each did before it. + // + // The equality is deliberate. `<= 10` would go on passing at nine and would + // stop saying anything the moment somebody removed a command; an exact ten + // fails in both directions and makes the next change a decision somebody + // records here. let inspect = GROUPS .iter() .find(|(group, _)| *group == Group::Inspect) .map(|(_, names)| names.len()) .expect("Inspect is a group with commands in it"); - assert!( - inspect < 10, - "Inspect holds {inspect}; the point of the move was to leave room", + assert_eq!( + inspect, 10, + "Inspect holds {inspect}; 0.27.0 filled it to the bound with `/store` and \ + `/export`, so the next command here re-files rather than widens", ); let rows = palette(&Templates::none(), &[]); @@ -1444,3 +1476,103 @@ fn gates_is_a_configure_command_that_resolves_to_its_own_action() { "`/gates`'s row is advertised and inert", ); } + +/// **The three new commands' argued forms, which had no gate at all.** +/// +/// The adversarial review found that nothing in `tests/` typed `/store`, +/// `/undo` or `/export` with an argument — so the release's most load-bearing +/// decision, that an unparseable verb must **never** fall through to something +/// destructive, was asserted nowhere. +/// +/// Both refusal families exist for a stated reason and both are checked here: +/// +/// - `Action::UndoNoStep` — *an operator who typed a step and got the entire run +/// undone would have lost work they never asked to lose.* +/// - `Keep::{NoId, NoDate, Unknown}` — *somebody who typed a delete and got a +/// report would believe the delete had happened.* +#[test] +fn f10_an_unparseable_verb_never_falls_through_to_something_destructive() { + use io_cli::commands::{Keep, Taken}; + use io_cli::undo::Grain; + + // `parse` takes the word without its slash — the driver strips it before the + // call, which is why every other test in this file types `continue` rather + // than `/continue`. Written as a helper so the cases below read as the + // operator types them. + let parse = + |text: &str| commands::parse(text.strip_prefix('/').unwrap_or(text), &defaults(), &DARK); + + // `/undo` — the one that must never become a whole-run undo. + assert_eq!(parse("/undo"), Action::Undo(Grain::Run)); + assert_eq!( + parse("/undo src/a.rs"), + Action::Undo(Grain::File("src/a.rs".into())), + ); + assert_eq!(parse("/undo step 4"), Action::Undo(Grain::Step(4))); + for wrong in [ + "/undo step", + "/undo step -1", + "/undo step 3.5", + "/undo step nine", + "/undo step 99999999999999999999", + "/undo step 0x10", + ] { + assert_eq!( + parse(wrong), + Action::UndoNoStep, + "`{wrong}` must be refused by name and must never become Grain::Run", + ); + } + + // `/store` — the verbs, and the three refusals. + assert_eq!(parse("/store"), Action::Store(None)); + assert_eq!(parse("/store rm 7"), Action::Store(Some(Keep::Remove(7)))); + assert_eq!(parse("/store compact"), Action::Store(Some(Keep::Compact))); + assert_eq!( + parse("/store sweep 2026-08-01"), + Action::Store(Some(Keep::Sweep("2026-08-01".into()))), + ); + for wrong in [ + "/store rm", + "/store rm all", + "/store rm -1x", + "/store delete", + ] { + assert!( + matches!(parse(wrong), Action::Store(Some(Keep::NoId))), + "`{wrong}` must ask for an id rather than act: {:?}", + parse(wrong), + ); + } + assert!(matches!( + parse("/store sweep"), + Action::Store(Some(Keep::NoDate)) + )); + // The typo that matters: a near-miss verb must not reach the page, because a + // report where a deletion was asked for reads as a deletion that happened. + for typo in ["/store swep 2026-08-01", "/store purge", "/store vacume"] { + assert!( + matches!(parse(typo), Action::Store(Some(Keep::Unknown(_)))), + "`{typo}` must be named as unknown rather than showing the page: {:?}", + parse(typo), + ); + } + + // `/export` — `trace` is a word and everything else is a path. + assert_eq!(parse("/export"), Action::Export(Taken::Conversation(None))); + assert_eq!( + parse("/export notes.md"), + Action::Export(Taken::Conversation(Some("notes.md".into()))), + ); + assert_eq!(parse("/export trace"), Action::Export(Taken::Trace(None))); + assert_eq!( + parse("/export trace run.txt"), + Action::Export(Taken::Trace(Some("run.txt".into()))), + ); + // The documented ambiguity, and its documented escape. + assert_eq!( + parse("/export ./trace"), + Action::Export(Taken::Conversation(Some("./trace".into()))), + "a file literally called `trace` is reachable as `./trace`", + ); +} diff --git a/tests/contract.rs b/tests/contract.rs index f92a1c7..f271519 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -1896,3 +1896,31 @@ fn f11_asking_for_the_default_explicitly_changes_nothing() { let built = io_cli::contract::configured("a goal", root(), &agrees); assert!(built.detached_spawns); } + +/// **N3 — 0.27.0 adds no configuration key, so an operator who runs none of it +/// gets the contract 0.26.0 built.** +/// +/// Every surface this release adds is a *command*: `/store`, `/export`, `/undo`. +/// None of them is configured, none is read at startup, and none reaches +/// `contract::session` at all. So the strongest statement of N3 is that the key +/// catalogue did not move — a new key is the only way this release could have +/// changed what a turn is built from, and this is the one place that would show. +/// +/// The number is written out rather than derived, which is the same choice +/// `tests/commands.rs` makes about the command inventory and for the same +/// reason: growing the settings surface should be a decision somebody records +/// here, not a line somebody adds elsewhere. +/// +/// Sabotage: add a key to `CATALOGUE` — under which only this fails, and it +/// fails by saying a release that promised to add no configuration added one. +#[test] +fn n3_this_release_adds_no_configuration_key() { + let before_0_27_0 = 37; + assert_eq!( + io_cli::configure::CATALOGUE.len(), + before_0_27_0, + "0.27.0 adds three commands and no keys; a different number here means a \ + release that promised an operator nothing would change gave them \ + something to configure", + ); +} diff --git a/tests/dependencies.rs b/tests/dependencies.rs index a3b7d17..5e22583 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -950,3 +950,73 @@ fn n4_the_syntax_set_is_loaded_lazily_and_only_by_the_diff_renderer() { "the driver reached for the highlighter, which puts it on the startup path", ); } + +/// **N1 — the three destructive store operations are named in exactly one +/// module each, and no model can reach any of them.** +/// +/// The roadmap's words for `/store` are *none of which any model can call*, and +/// the way that stays true is by adding nothing: io-harness's workspace tool set +/// contains nothing that reaches `delete_session`, `sweep_sessions` or +/// `compact`, and this release registers no tool, no MCP server and no skill +/// that could. +/// +/// **"Nothing was added" is not assertable, so this asserts the reachable form +/// of it**: each call is named in one file, that file is `src/store.rs`, and +/// `src/store.rs` is reached from command dispatch rather than from anything on +/// a run's path. A second module naming one of these would be the first step +/// towards a caller that is not a keystroke, and it fails here by name. +/// +/// The needles are assembled at run time for the reason `tests/store.rs` records +/// at length: a gate that reads source cannot spell the thing it forbids, or its +/// own array is the first match. io-cli has now hit that in 0.16.0, 0.19.0, +/// 0.25.0, 0.26.0 and twice in 0.27.0. +#[test] +fn n1_the_store_operations_live_in_one_module_and_no_model_can_reach_them() { + let calls = [ + format!("{}_{}", "delete", "session"), + format!("{}_{}", "sweep", "sessions"), + format!("{}{}", "compact", "()"), + ]; + + for call in &calls { + let named: Vec = sources() + .into_iter() + .filter(|(_, text)| code_of(text).contains(call.as_str())) + .map(|(path, _)| { + path.file_name() + .expect("a file name") + .to_string_lossy() + .into_owned() + }) + .collect(); + + assert_eq!( + named, + vec!["store.rs".to_string()], + "`{call}` must be named in exactly one module, and it must be the one \ + whose whole subject is the store; found it in {named:?}", + ); + } +} + +/// **N1 — this release registers no tool, no MCP server and no skill.** +/// +/// The other half of *no model can call it*. A tool is how a model reaches +/// anything at all, so the honest check is that the set of ways this crate hands +/// the model a capability did not grow: `with_tools` is io-harness's own field +/// and this crate never sets it, and the two surfaces that do add capability — +/// `[[mcp]]` servers and skills — are read from the operator's configuration +/// rather than written by this release. +#[test] +fn n1_this_release_hands_the_model_no_new_capability() { + let forbidden = format!("{}_{}", "with", "tools"); + for (path, text) in sources() { + assert!( + !code_of(&text).contains(forbidden.as_str()), + "{} calls `{forbidden}`: io-cli does not choose the model's tool set, \ + and a release that started to would be handing it reach this crate \ + has never granted", + path.display(), + ); + } +} diff --git a/tests/export.rs b/tests/export.rs new file mode 100644 index 0000000..f554c92 --- /dev/null +++ b/tests/export.rs @@ -0,0 +1,445 @@ +//! Taking the work out of the terminal. +//! +//! **The single assertion this file exists for is a byte comparison.** A run's +//! canonical trace is valuable only because it is canonical: io-harness excludes +//! wall-clock stamps, measured durations, an argv's ephemeral tempdir and +//! `AUTOINCREMENT` ids from it so that two runs of one case can be compared, and +//! its own documentation says each exclusion is a promise rather than a +//! convenience. A trace io-cli pretty-printed would compare against nothing. +//! It is pipe-delimited text and not JSON, which this release's plan had wrong +//! until a sabotage that killed nothing exposed it — see `US-IO-CLI-0.27.0-I03`. +//! +//! So `f8_the_trace_is_written_byte_for_byte` compares **strings**, not parsed +//! values. Parsing both sides and comparing them is the test that looks more +//! rigorous and is the one that passes on the defect: a reformatted document +//! parses to the same values, which is exactly the thing that must fail. +//! +//! The markdown half is asserted on content and order rather than on formatting, +//! because nothing reads it back — it is for a person and for a diff, it carries +//! no schema version, and this product parses it nowhere. + +use io_cli::export::{ + confirm, conversation, conversation_path, occupied, report, trace, trace_path, write, Refused, +}; +use io_harness::tools::{Workspace, Wrote}; +use io_harness::Store; + +/// The session, as a `Session`, for the reader that walks its history. +fn reopened(store: &Store, id: i64) -> io_harness::Session { + io_harness::Session::reopen(store, id).expect("the session reopens") +} + +/// A store in a real file, plus a workspace to write into. +/// +/// One `TempDir` for both, because an export writes into the workspace the +/// session is running over and the point of the last test here is that it cannot +/// write anywhere else. +fn fixture() -> (tempfile::TempDir, Store, Workspace) { + let dir = tempfile::tempdir().expect("a temporary directory"); + let store = Store::open(dir.path().join("runs.db")).expect("a store opens on disk"); + let workspace = Workspace::new(dir.path()); + (dir, store, workspace) +} + +/// One session with a finished turn per prompt, and the run ids in order. +/// +/// **Every run records two steps, and that is not decoration.** The first +/// version of this fixture recorded none, and `Store::canonical_trace` is built +/// entirely from `steps` and `context_events` (`state/trace.rs:1019-1036`) — so +/// it returned the empty string, and the byte-for-byte gate below was comparing +/// `""` against `""`. It passed, and it would have gone on passing over any +/// defect whatsoever. +/// +/// It was found because the named sabotage — pretty-print the trace — killed +/// **nothing**, which is the whole reason a sabotage is executed rather than +/// described. A gate that cannot fail is worse than a missing one, because it +/// reads on the page as coverage. +fn seed(store: &Store, prompts: &[(&str, Option<&str>)]) -> (i64, Vec) { + let session = store.create_session("/tmp/work").expect("a session opens"); + let mut runs = Vec::new(); + let mut parent: Option = None; + for (prompt, reply) in prompts { + let run = store.start_run(prompt, "/tmp/work").expect("a run starts"); + for step in 1..=2u32 { + store + .record( + run, + &io_harness::StepRecord { + step, + decision: format!("write_file ok on step {step}"), + result: format!("wrote src/thing-{step}.rs"), + prompt: prompt.to_string(), + tool_call: format!("write_file:{{\"path\":\"src/thing-{step}.rs\"}}"), + tokens: 128 * u64::from(step), + }, + ) + .expect("a step records"); + } + let turn = store + .record_turn(session, parent, run, prompt) + .expect("a turn records"); + // **The head is moved as a real turn would move it.** `conversation` + // walks `Session::history` — the path from the head back to the root — + // rather than `Store::session_turns`, which io-harness documents as the + // whole tree. A fixture that left the head unset would export nothing, + // and one that parented every turn at the root would export a fan. + store + .set_session_head(session, Some(turn)) + .expect("the head moves"); + parent = Some(turn); + // A `None` reply is a turn that did not finish, and it is written as one. + // The turn is still closed, because an unfinished *reply* and an + // unfinished *turn* are different states and only the first is on trial. + store + .finish_turn(turn, *reply, "completed") + .expect("a turn finishes"); + store.finish_run(run, "success").expect("the run finishes"); + runs.push(run); + } + (session, runs) +} + +// --------------------------------------------------------------------------- +// F8 — the conversation, in order +// --------------------------------------------------------------------------- + +/// **F8 — every prompt and every reply appears, in the store's own order.** +/// +/// Order is asserted by position in the rendered string rather than by counting +/// occurrences, because a document that carried all six strings shuffled would +/// satisfy a count and would be useless to read. +#[test] +fn f8_every_prompt_and_reply_appears_in_the_stores_order() { + let (_dir, store, _ws) = fixture(); + let (session, _runs) = seed( + &store, + &[ + ("the first question", Some("the first answer")), + ("the second question", Some("the second answer")), + ("the third question", Some("the third answer")), + ], + ); + + let markdown = conversation(&store, &reopened(&store, session)) + .expect("the conversation is readable") + .expect("there is one"); + + let mut at = 0; + for needle in [ + "the first question", + "the first answer", + "the second question", + "the second answer", + "the third question", + "the third answer", + ] { + let found = markdown[at..] + .find(needle) + .unwrap_or_else(|| panic!("`{needle}` is missing or out of order:\n{markdown}")); + at += found + needle.len(); + } +} + +/// **F8 — a turn with no reply is written as a turn with no reply.** +/// +/// `Turn::reply` is `Option` and `None` means the turn did not finish. +/// An empty section would make an interrupted conversation read as one the agent +/// had nothing to say to. +#[test] +fn f8_a_turn_that_never_answered_says_so_rather_than_showing_an_empty_reply() { + let (_dir, store, _ws) = fixture(); + let (session, _runs) = seed( + &store, + &[("answered", Some("here you are")), ("never answered", None)], + ); + + let markdown = conversation(&store, &reopened(&store, session)) + .expect("the conversation is readable") + .expect("there is one"); + + assert!(markdown.contains("here you are")); + assert!( + markdown.contains("did not finish"), + "an unanswered turn is named, not blank:\n{markdown}", + ); +} + +/// A session with no turns is nothing to export, and says so. +/// +/// A file asserting the session was empty is worse than no file: the session may +/// simply not have started. +#[test] +fn a_session_with_no_turns_is_refused_rather_than_written_empty() { + let (_dir, store, _ws) = fixture(); + let session = store.create_session("/tmp/work").expect("a session opens"); + + assert_eq!( + conversation(&store, &reopened(&store, session)).expect("readable"), + None, + "there is no conversation to write", + ); + assert!(Refused::Nothing.said().contains("nothing to export")); +} + +// --------------------------------------------------------------------------- +// F8 — the trace, byte for byte +// --------------------------------------------------------------------------- + +/// **F8 — the trace file is byte-identical to `Store::canonical_trace`.** +/// +/// A string comparison, deliberately. See the module note: parsing both sides +/// and comparing values is the test that passes on the defect. +#[test] +fn f8_the_trace_is_written_byte_for_byte() { + let (_dir, store, workspace) = fixture(); + let (_session, runs) = seed(&store, &[("do the thing", Some("done"))]); + let run = runs[0]; + + let ours = trace(&store, run).expect("a trace is readable"); + let harness = store.canonical_trace(run).expect("a trace is readable"); + // **The gate that stops this one going vacuous again.** A run with no steps + // has an empty canonical trace, and every assertion below is satisfied by + // two empty strings. Assert there is something to compare before comparing + // it — 0.26.0's lesson that a gate can go vacuous without going red. + assert!( + !harness.is_empty(), + "the fixture must produce a trace, or every assertion here is `\"\" == \"\"`", + ); + assert!(harness.contains("step 1"), "and a real one: {harness}"); + assert_eq!(ours, harness, "the trace is passed through untouched"); + + let path = trace_path(run); + let written = write(&workspace, &path, &ours).expect("the trace is written"); + assert_eq!(written.wrote, Wrote::Created); + + let back = std::fs::read_to_string(workspace.root().join(&path)).expect("the file is there"); + assert_eq!( + back, harness, + "the file on disk is io-harness's own string, byte for byte", + ); +} + +/// The trace is not reformatted, and this is the assertion that says so +/// independently of the equality above. +/// +/// A pretty-printer is the specific defect: it would change the whitespace and +/// nothing else, so a test that only compared parsed values would not see it. +#[test] +fn the_trace_is_not_reformatted() { + let (_dir, store, _ws) = fixture(); + let (_session, runs) = seed(&store, &[("do the thing", Some("done"))]); + + let ours = trace(&store, runs[0]).expect("a trace is readable"); + let harness = store.canonical_trace(runs[0]).expect("a trace is readable"); + + assert!(!harness.is_empty(), "there is something to reformat"); + assert_eq!( + ours.len(), + harness.len(), + "a reformatting changes the length even when it changes no value", + ); + assert_eq!( + ours.matches('\n').count(), + harness.matches('\n').count(), + "and it changes the line count, which is what a pretty-printer does", + ); +} + +// --------------------------------------------------------------------------- +// F8 — the write goes through the workspace +// --------------------------------------------------------------------------- + +/// **F8 — a path outside the workspace is refused rather than written.** +/// +/// The refusal is io-harness's, through `Workspace::write_file`, and is asserted +/// by the file's absence as well as by the error — an error that had already +/// written the file would be worse than no error at all. +#[test] +fn f8_a_path_outside_the_workspace_is_refused() { + let (_dir, store, workspace) = fixture(); + let (session, _runs) = seed(&store, &[("a question", Some("an answer"))]); + let markdown = conversation(&store, &reopened(&store, session)) + .expect("readable") + .expect("there is one"); + + let escape = "../escaped.md"; + let refused = write(&workspace, escape, &markdown); + + assert!(refused.is_err(), "a path above the root is refused"); + assert!( + !workspace.root().join("..").join("escaped.md").exists(), + "and nothing is written outside the workspace", + ); +} + +/// An existing file is refused rather than overwritten. +/// +/// An export is a snapshot; the next one is a different snapshot. A command that +/// silently replaced the first would destroy what the operator was about to +/// compare against. +#[test] +fn an_existing_file_is_refused_rather_than_overwritten() { + let (_dir, store, workspace) = fixture(); + let (session, _runs) = seed(&store, &[("a question", Some("an answer"))]); + let path = conversation_path(session); + let markdown = conversation(&store, &reopened(&store, session)) + .expect("readable") + .expect("there is one"); + + assert!( + !occupied(&workspace, &path).expect("askable"), + "nothing yet" + ); + write(&workspace, &path, &markdown).expect("the first export is written"); + assert!( + occupied(&workspace, &path).expect("askable"), + "and now something is there", + ); + + let said = Refused::Exists(path.clone()).said(); + assert!(said.contains(&path), "the refusal names the file: {said}"); + assert!(said.contains("snapshot"), "and says why: {said}"); +} + +/// The proposed paths are distinct per session and per run, so two exports do +/// not collide before anyone has been asked anything. +#[test] +fn the_proposed_paths_name_what_they_hold() { + assert_ne!(conversation_path(1), conversation_path(2)); + assert_ne!(trace_path(1), trace_path(2)); + assert!(conversation_path(7).ends_with(".md")); + assert!(trace_path(7).ends_with(".txt")); +} + +/// The confirmation declines at row 0, like every other confirmation this +/// release adds, and names the path it is about. +#[test] +fn the_confirmation_declines_at_row_zero_and_names_the_path() { + let (title, rows) = confirm("io-session-3.md", "conversation"); + + assert!(title.contains("io-session-3.md"), "{title}"); + assert_eq!(rows[0].label, io_cli::store::LEAVE_IT); + assert!(!io_cli::store::acts(0)); + assert!(io_cli::store::acts(1)); + assert!(rows[1].label.contains("io-session-3.md")); +} + +/// The report says what was written and how much of it. +#[test] +fn the_report_names_the_file_and_its_size() { + let (_dir, store, workspace) = fixture(); + let (session, _runs) = seed(&store, &[("a question", Some("an answer"))]); + let markdown = conversation(&store, &reopened(&store, session)) + .expect("readable") + .expect("there is one"); + let written = + write(&workspace, &conversation_path(session), &markdown).expect("the export is written"); + + let said = report(&written); + assert!(said.contains(&conversation_path(session)), "{said}"); + assert!(said.contains(&written.bytes.to_string()), "{said}"); + assert_eq!(written.bytes, markdown.len()); +} + +/// **`write` refuses an existing file itself, not only through its caller.** +/// +/// The confirmation an operator agrees to is shown after an earlier `occupied` +/// call, and between the two keystrokes another `io`, an editor autosave or a +/// build can create the file. Without a check inside `write` the export would +/// silently replace it and report a creation — and `Written::wrote` promises +/// `Wrote::Created`. Found by the adversarial review, which also found that the +/// test named for this refusal never actually called `write` twice. +#[test] +fn f8_write_refuses_an_existing_file_rather_than_overwriting_it() { + let (_dir, store, workspace) = fixture(); + let (session, _runs) = seed(&store, &[("a question", Some("an answer"))]); + let path = conversation_path(session); + let markdown = conversation(&store, &reopened(&store, session)) + .expect("readable") + .expect("there is one"); + + write(&workspace, &path, &markdown).expect("the first export is written"); + let again = write(&workspace, &path, "something else entirely"); + + assert!(again.is_err(), "the second write is refused"); + assert_eq!( + std::fs::read_to_string(workspace.root().join(&path)).expect("the file"), + markdown, + "and the first export is untouched", + ); +} + +/// **A dangling symlink is occupied, so the write cannot follow it out of the +/// workspace.** +/// +/// `Workspace::resolve` is purely lexical and `Path::exists()` follows the link, +/// so a broken symlink answered "nothing is there" — and `write_file` then +/// followed it and created the file wherever it pointed. `check_path`'s own +/// symlink branch cannot help: it is guarded by a `canonicalize` that fails on a +/// broken link. Asking about the link itself is what closes it. Found by the +/// adversarial review. +#[cfg(unix)] +#[test] +fn f8_a_dangling_symlink_is_not_mistaken_for_an_empty_path() { + let (_dir, _store, workspace) = fixture(); + let link = workspace.root().join("io-session-9.md"); + std::os::unix::fs::symlink("../../escaped.md", &link).expect("the link is made"); + + assert!( + !link.exists(), + "the link is dangling, which is the whole point", + ); + assert!( + occupied(&workspace, "io-session-9.md").expect("askable"), + "a dangling symlink is something, and a write would follow it out of the \ + workspace", + ); + assert!( + write(&workspace, "io-session-9.md", "anything").is_err(), + "so the write is refused", + ); +} + +/// **The export is the conversation, not the whole turn tree.** +/// +/// `Store::session_turns` is documented as "the whole tree, not one path through +/// it", and a rewind moves the head back **without deleting the turn**. So a +/// session that was undone once and carried on holds turns that are not part of +/// the conversation at all — and exporting them flat, unmarked, in the one +/// artifact whose purpose is being read by somebody who was not there, would +/// misrepresent both what the agent was asked and what it answered. +/// +/// Found by the adversarial review. `Session::history` is the path. +#[test] +fn f8_an_undone_turn_is_not_in_the_export() { + let (_dir, store, _ws) = fixture(); + let (session, _runs) = seed( + &store, + &[ + ("the first question", Some("the first answer")), + ("the abandoned question", Some("the abandoned answer")), + ], + ); + + // Move the head back to the first turn, exactly as a rewind does — the turn + // row stays, off the head path. + let turns = store.session_turns(session).expect("readable"); + let first = turns.first().expect("a first turn").id; + store + .set_session_head(session, Some(first)) + .expect("the head moves back"); + + let markdown = conversation(&store, &reopened(&store, session)) + .expect("readable") + .expect("there is a conversation"); + + assert!( + markdown.contains("the first question"), + "the conversation that stands is exported:\n{markdown}", + ); + assert!( + !markdown.contains("the abandoned question"), + "a turn that was undone is not part of the conversation and must not be \ + in the document:\n{markdown}", + ); +} diff --git a/tests/glyphs.rs b/tests/glyphs.rs index 4eeeed1..fc47e52 100644 --- a/tests/glyphs.rs +++ b/tests/glyphs.rs @@ -582,6 +582,15 @@ fn every_event_this_release_renders_draws_in_ascii() { backend: Some("macos-sandbox-exec".into()), }, EventKind::Stalled, + // 0.27.0 — the one silence that gained a line, and it carries the muted + // leader and a separator, both of which differ between the two glyph + // sets. `discarded` is non-zero because the arm declines a step that + // speculated perfectly, and a declined event renders nothing to sweep. + EventKind::Speculated { + started: 3, + used: 1, + discarded: 2, + }, ]; // **The list above is checked against the renderer, not trusted.** It is diff --git a/tests/live.rs b/tests/live.rs index e02a5a9..3fb7843 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -924,7 +924,7 @@ async fn live_f13_work_survives_the_session() { "armed: {}", io_cli::rewind::armed_line(&about, &DARK.glyphs) ); - let undone = io_cli::rewind::last_turn(&mut session, &store) + let undone = io_cli::rewind::last_turn(&mut session, &store, &io_harness::Ignore) .expect("the rewind runs") .expect("there was a turn to undo"); for (tone, line) in io_cli::rewind::undone_lines(&undone, &DARK.glyphs) { @@ -3395,3 +3395,237 @@ async fn live_f5_an_unreachable_primary_falls_through_to_the_provider_underneath answered — it is what io-harness turns into `EventKind::FellBackTo`", ); } + +/// **O3 / F6 / F7 — an undo of a file a real agent actually wrote.** +/// +/// The fixture tests build their restore points through io-harness's own run +/// loop with a scripted provider, which is the right instrument for asserting +/// the four answers. What no fixture can settle is whether a *real* model's +/// `write_file` produces a snapshot this crate can rewind — that depends on the +/// tool the model chooses, and a model may edit, patch or rewrite. +/// +/// So this drives a real turn, asserts the file changed, then puts it back and +/// asserts the bytes on disk are the ones that were there before the agent ran. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f6_f7_a_real_agents_write_is_put_back() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + let before = "the original line\n"; + std::fs::write(root.join("subject.txt"), before).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( + "Replace the entire contents of subject.txt with exactly the word: replaced. \ + Then say what you did in one sentence.", + &provider, + &store, + &policy, + &io_harness::ApproveAll, + &observer, + &inbox, + ) + .await + .expect("the turn runs"); + println!("outcome: {:?}", result.outcome); + + let after = std::fs::read_to_string(root.join("subject.txt")).expect("the file is there"); + println!("after the turn: {after:?}"); + assert_ne!(after, before, "the agent has to have changed something"); + + let head = session.head().expect("the turn is on the head"); + let run_id = store + .session_turn(head) + .expect("readable") + .expect("there") + .run_id; + + // The undo, through the same call `/undo ` makes. + let workspace = io_harness::tools::Workspace::new(root); + let answer = + io_cli::undo::one_file(&workspace, &store, run_id, "subject.txt").expect("the undo runs"); + println!( + "the undo said: {}", + io_cli::undo::said("subject.txt", &answer) + ); + + assert!( + matches!(answer, io_harness::Rewind::Restored(_)), + "a real agent's write leaves a restore point this crate can use: {answer:?}", + ); + assert_eq!( + std::fs::read_to_string(root.join("subject.txt")).expect("still there"), + before, + "the bytes on disk are the ones that were there before the agent ran", + ); + + // And the whole-turn form emits the event that had never fired before + // 0.27.0 — through `rewind::last_turn`, which is the production path. + let watcher = Collector { + events: Arc::new(Mutex::new(Vec::new())), + }; + let undone = io_cli::rewind::last_turn(&mut session, &store, &watcher) + .expect("the whole-turn undo runs"); + println!("undone: {undone:?}"); + let saw = watcher + .events + .lock() + .expect("not poisoned") + .iter() + .any(|event| matches!(event.kind, io_harness::EventKind::Rewound { .. })); + assert!(saw, "EventKind::Rewound reached the observer on a real run"); +} + +/// **O3 / F8 — an export of a conversation that actually happened.** +/// +/// A fixture's trace is assembled from rows a test wrote. This asserts the +/// export against a run a real model drove: the markdown carries the prompt and +/// the reply that were really exchanged, and the trace file is byte-identical to +/// what io-harness produced for that run. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f8_a_real_conversation_exports() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + + 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 prompt = "In one short sentence, say what a canonical trace is for."; + session + .turn_steered( + prompt, &provider, &store, &policy, &DenyAll, &observer, &inbox, + ) + .await + .expect("the turn runs"); + + let markdown = io_cli::export::conversation(&store, &session) + .expect("readable") + .expect("a conversation that happened"); + println!("--- exported markdown ---\n{markdown}"); + assert!( + markdown.contains(prompt), + "the operator's own words are in the export", + ); + + let head = session.head().expect("a turn"); + let run_id = store + .session_turn(head) + .expect("readable") + .expect("there") + .run_id; + + let trace = io_cli::export::trace(&store, run_id).expect("a trace"); + assert!( + !trace.is_empty(), + "a run that really happened has a trace; an empty one would make the \ + byte comparison below vacuous, which is the defect this release found \ + in its own fixture", + ); + + let workspace = io_harness::tools::Workspace::new(root); + let written = io_cli::export::write(&workspace, &io_cli::export::trace_path(run_id), &trace) + .expect("the trace is written"); + let back = std::fs::read_to_string(root.join(&written.path)).expect("the file"); + assert_eq!( + back, + store.canonical_trace(run_id).expect("readable"), + "the file on disk is io-harness's own string, byte for byte", + ); +} + +/// **O3 — whichever of the two conditional events a real run emits.** +/// +/// `Speculated` fires only when a provider reports finished calls and something +/// was actually started; `CacheMarked` only when a marked prefix advances. +/// Neither can be provoked, so this **reports** rather than asserts: it prints +/// what a real run emitted so the release record can say what was observed +/// rather than what was hoped for. +/// +/// The one thing it does assert is that if `Speculated` arrives with something +/// discarded, the renderer draws a line for it — which is the arm 0.27.0 added. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f9_whichever_conditional_events_a_real_run_emits() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + for name in ["one.txt", "two.txt", "three.txt"] { + std::fs::write(root.join(name), format!("contents of {name}\n")).expect("a 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), + }; + + session + .turn_steered( + "Read one.txt, two.txt and three.txt, then tell me in one sentence what \ + they have in common.", + &provider, + &store, + &policy, + &DenyAll, + &observer, + &inbox, + ) + .await + .expect("the turn runs"); + + let events = collected.lock().expect("not poisoned").clone(); + let mut drew = 0usize; + for event in &events { + match &event.kind { + io_harness::EventKind::Speculated { + started, + used, + discarded, + } => { + println!("live: Speculated started={started} used={used} discarded={discarded}"); + if *discarded > 0 { + let mut renderer = io_cli::events::Events::new(io_cli::theme::DARK); + let lines = renderer.event(event, std::time::Duration::ZERO); + assert!( + !lines.is_empty(), + "a real discarded read must draw the line 0.27.0 added", + ); + drew += 1; + } + } + io_harness::EventKind::CacheMarked { + through_step, + prefix_bytes, + } => println!("live: CacheMarked through_step={through_step} bytes={prefix_bytes}"), + _ => {} + } + } + println!( + "live: drew {drew} speculation line(s) from {} events", + events.len() + ); +} diff --git a/tests/narrow.rs b/tests/narrow.rs index 15ef6ed..8854be8 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -2248,3 +2248,155 @@ fn f11_status_folds_at_eighty_columns_rather_than_losing_the_end_of_a_row() { } } } + +/// **N2 — every surface 0.27.0 adds fits eighty columns in both glyph sets.** +/// +/// The store page, the four confirmations, and the reports each operation +/// writes. Drawn through the real render buffer for the picker, and measured as +/// built lines for the page and the reports, which are committed into the +/// terminal's own scrollback rather than drawn into the viewport. +/// +/// **Width-independent where it can be.** 0.26.0's lesson is that a gate which +/// picks a width is asserting arithmetic it will get wrong — its own +/// narrow-fixture test passed at eighty because the fixture could not fit the +/// field either way. So the confirmations are checked at six widths and the +/// property is the same at all of them: no row overflows, and the declining row +/// survives, because a confirmation that dropped its "leave it" at a narrow +/// terminal would leave an operator with only the destructive choice on screen. +#[test] +fn n2_every_store_confirmation_fits_and_keeps_its_declining_row() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let store = io_harness::Store::open(dir.path().join("runs.db")).expect("a store"); + let session = store + .create_session("/a/deliberately/long/workspace/path/that/will/not/fit/in/eighty/columns") + .expect("a session"); + let sized = io_cli::store::sized(&store, session).expect("a size"); + let size = store.store_size().expect("a size"); + + for theme in themes() { + let set = if theme.glyphs.separator == io_cli::glyphs::ASCII.separator { + "ascii" + } else { + "unicode" + }; + for (what, (title, rows)) in [ + ("remove", io_cli::store::confirm_remove(session, &sized, 0)), + ( + "sweep", + io_cli::store::confirm_sweep("2026-08-01T00:00:00.000Z"), + ), + ("compact", io_cli::store::confirm_compact(&size)), + ( + "export", + io_cli::export::confirm("io-session-1.md", "conversation"), + ), + ( + "undo file", + io_cli::undo::confirm_file("src/a/deeply/nested/path/that/is/long.rs"), + ), + ("undo step", io_cli::undo::confirm_step(12)), + ] { + // Six widths rather than one. The property holds at all of them and + // names no number, which is what 0.26.0's correction asks for. + for width in [40u16, 60, 72, 80, 100, 160] { + let mut picker = io_cli::picker::Picker::new(title.clone(), rows.clone()); + let (mut screen, _recorder) = support::screen(width, HEIGHT); + screen + .draw(|frame| picker.render(frame, frame.area(), &theme)) + .expect("frame"); + let drawn = screen.viewport_text().to_string(); + for line in drawn.lines() { + assert!( + line.chars().count() <= width as usize, + "{what} overflowed {width} columns ({set}): {line:?}", + ); + } + } + assert_eq!( + rows[0].label, + io_cli::store::LEAVE_IT, + "{what} ({set}): the declining row is row 0 at every width", + ); + } + } +} + +/// **N2 — the store page and every report fit eighty columns in both sets.** +#[test] +fn n2_the_store_page_and_the_reports_fit_eighty_columns() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let store = io_harness::Store::open(dir.path().join("runs.db")).expect("a store"); + let session = store + .create_session("/a/deliberately/long/workspace/path/that/will/not/fit/in/eighty/columns") + .expect("a session"); + let run = store.start_run("a goal", "/tmp/w").expect("a run"); + let turn = store + .record_turn(session, None, run, "a goal") + .expect("a turn"); + store + .finish_turn(turn, Some("an answer"), "completed") + .expect("the turn finishes"); + store.finish_run(run, "success").expect("the run finishes"); + + for theme in themes() { + let set = if theme.glyphs.separator == io_cli::glyphs::ASCII.separator { + "ascii" + } else { + "unicode" + }; + let page = io_cli::store::committed(&store, &theme, WIDTH).expect("the page renders"); + within_eighty(set, &text_of(&page).join("\n")); + + // The reports are committed as plain sentences, so they are measured as + // strings rather than drawn — `App::record` wraps them the way every + // other committed line is wrapped. + let removed = match io_cli::store::remove(&store, session, 0).expect("the removal runs") { + io_cli::store::Removal::Done(removed) => *removed, + io_cli::store::Removal::Live => unreachable!("0 is not a session id"), + }; + for line in io_cli::store::removed_report(&removed) { + assert!( + !line.contains('\n'), + "{set}: a report line carries its own newline: {line:?}", + ); + } + let swept = io_cli::store::sweep(&store, "2000-01-01T00:00:00.000Z").expect("the sweep"); + for line in io_cli::store::swept_report(&swept) { + assert!(!line.contains('\n'), "{set}: {line:?}"); + } + let freed = io_cli::store::compact(&store).expect("the compaction"); + for line in io_cli::store::freed_report(&freed) { + assert!(!line.contains('\n'), "{set}: {line:?}"); + } + } +} + +/// **N2 — the speculation line fits eighty columns in both glyph sets.** +/// +/// The one event 0.27.0 draws. It carries the muted leader and a separator, both +/// of which differ between the sets. +#[test] +fn n2_the_speculation_line_fits_eighty_columns() { + for theme in themes() { + let set = if theme.glyphs.separator == io_cli::glyphs::ASCII.separator { + "ascii" + } else { + "unicode" + }; + let mut events = io_cli::events::Events::new(theme); + let lines = events.event( + &io_harness::RunEvent::new( + 1, + 1, + io_harness::EventKind::Speculated { + started: 999, + used: 111, + discarded: 888, + }, + ), + std::time::Duration::ZERO, + ); + assert!(!lines.is_empty(), "{set}: the line is drawn"); + within_eighty(set, &text_of(&lines).join("\n")); + } +} diff --git a/tests/rewind.rs b/tests/rewind.rs index ef647da..1d14399 100644 --- a/tests/rewind.rs +++ b/tests/rewind.rs @@ -192,7 +192,7 @@ async fn an_edited_file_comes_back_and_a_created_file_goes_and_both_are_reported assert_eq!(fixture.bytes("notes.md"), AFTER.as_bytes()); assert_eq!(fixture.bytes("summary.md"), INVENTED.as_bytes()); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("the rewind cannot fail") .expect("a turn was taken, so there is one to undo"); @@ -280,7 +280,7 @@ async fn memory_the_run_changed_is_put_back_and_memory_it_invented_is_removed() "the turn's wrong value must actually be in the store before it is undone", ); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("the rewind cannot fail") .expect("a turn was taken"); @@ -338,7 +338,7 @@ async fn a_file_whose_previous_contents_were_not_kept_is_declined_and_left_untou let by_hand = b"what the operator typed instead\n"; std::fs::write(fixture.dir.path().join("logo.bin"), by_hand).expect("the file is writable"); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("the rewind cannot fail") .expect("a turn was taken"); @@ -394,7 +394,7 @@ async fn rewinding_the_second_of_two_turns_leaves_the_head_on_the_first() { "the fixture needs two distinct turns", ); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("the rewind cannot fail") .expect("a turn was taken"); @@ -435,7 +435,7 @@ async fn rewinding_the_only_turn_leaves_the_session_with_no_head_at_all() { "the fixture needs one turn", ); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("the rewind cannot fail") .expect("a turn was taken"); @@ -515,7 +515,7 @@ async fn an_undo_is_refused_when_another_io_has_already_moved_the_head_on() { .set_session_head(fixture.session.id(), Some(theirs)) .expect("the other process advances the head"); - let refused = rewind::last_turn(&mut fixture.session, &fixture.store) + let refused = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect_err("an undo whose head has moved underneath it must not succeed"); // The assertion is on the **refusal**, and deliberately so: the defect this @@ -578,7 +578,7 @@ async fn an_undo_over_a_store_no_other_io_touched_still_moves_the_head_back() { .turn_writing("now do it", &[("plan.md", AFTER)]) .await; - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("nothing raced, so the conditional write must match its row") .expect("a turn was taken"); @@ -594,7 +594,7 @@ fn a_session_that_has_taken_no_turns_undoes_nothing_and_does_not_fail() { rewind::preview(&fixture.session, &fixture.store).is_none(), "there is nothing to arm a confirmation prompt about", ); - let undone = rewind::last_turn(&mut fixture.session, &fixture.store) + let undone = rewind::last_turn(&mut fixture.session, &fixture.store, &io_harness::Ignore) .expect("an empty session is not an error"); assert!( undone.is_none(), diff --git a/tests/store.rs b/tests/store.rs new file mode 100644 index 0000000..80849c6 --- /dev/null +++ b/tests/store.rs @@ -0,0 +1,720 @@ +//! What the run store holds, and the four things `/store` can do about it. +//! +//! **The one fact every test here defends: a deletion does not shrink the file.** +//! SQLite frees pages *into* the database rather than out of it, so a removal +//! moves bytes from `StoreSize::file_bytes` into `StoreSize::free_bytes` and the +//! file on disk stays exactly the size it was. An interface that reported a +//! deletion as having reclaimed space would be lying, and it would be a lie an +//! operator only discovers by running out of disk. So `f1` asserts `file_bytes` +//! is **unchanged** — an equality, not an inequality — and `f2` asserts that +//! `compact` is the only call that moves it. +//! +//! **Every store here is a real file in a `TempDir`.** `Store::memory()` cannot +//! be used for any of this: the whole subject is page arithmetic on a file, and +//! a `VACUUM` of a connection that never touched a disk would be testing the half +//! of the claim that was never in doubt. That is also criterion O4 — a gate that +//! opened the developer's own `~/.io-cli/runs.db` and swept it would be data loss +//! shipped as a test, so the last test in this file asserts that no test here +//! names a real home. +//! +//! **What cannot be asserted from this crate, and why it is written down rather +//! than quietly skipped.** F4 describes a session created at exactly the sweep's +//! boundary surviving, because io-harness's comparison is strictly before. That +//! arm is real and it is io-harness's own doctest (`state/sessions.rs:420`); it +//! is **not falsifiable here**, because `sessions.created_at` is written by the +//! store and there is no public reader for it — the same gap as io-harness#216, +//! from the other side. A test that pretended otherwise would be asserting a +//! timestamp this crate invented. The reachable halves are asserted instead: a +//! date before everything sweeps nothing, a date after everything sweeps +//! everything not refused, and the refusal is named. This is 0.23.0's recorded +//! lesson — *check where a sabotage lives before writing the criterion* — showing +//! up on the other side of the same wall. + +use io_cli::store::{ + acts, committed, compact, confirm_compact, confirm_remove, confirm_sweep, freed_report, + removed_report, sized, sweep, swept_report, view, Sized, LEAVE_IT, +}; +use io_harness::Store; + +/// A store in a real file, and the directory that has to outlive it. +/// +/// The `TempDir` comes back with the store because dropping it deletes the +/// database underneath — and because criterion O4 is that no gate in this file +/// can reach anything else. +fn on_disk() -> (tempfile::TempDir, Store) { + let dir = tempfile::tempdir().expect("a temporary directory"); + let store = Store::open(dir.path().join("runs.db")).expect("a store opens on disk"); + (dir, store) +} + +/// One session with `turns` finished turns on it. +/// +/// The run is finished as well as the turn. A run nothing ever finished is +/// `running`, which is a *resumable* run — which is exactly what +/// [`seed_resumable`] exists to produce deliberately, and what this must not +/// produce by accident, or every sweep test would be testing the refusal. +fn seed(store: &Store, root: &str, turns: usize) -> i64 { + let session = store.create_session(root).expect("a session opens"); + for turn in 0..turns { + let prompt = format!("a prompt long enough to occupy some bytes, number {turn}"); + let run = store.start_run(&prompt, root).expect("a run starts"); + let id = store + .record_turn(session, None, run, &prompt) + .expect("a turn records"); + store + .finish_turn(id, Some("an answer of some length"), "completed") + .expect("a turn finishes"); + store + .finish_run(run, "success") + .expect("the run finishes too"); + } + session +} + +/// One session whose run is left `running`, and is therefore still resumable. +/// +/// This is what `Store::sweep_sessions` refuses. io-harness's argument +/// (`state/sessions.rs:370-378`) is that a date is a policy applied to sessions +/// nobody looked at, and a crash-resumable tree that vanished because it was old +/// is the worst outcome the call could have. +fn seed_resumable(store: &Store, root: &str) -> i64 { + let session = store.create_session(root).expect("a session opens"); + let run = store.start_run("unfinished", root).expect("a run starts"); + store + .record_turn(session, None, run, "unfinished") + .expect("a turn records"); + session +} + +/// Remove a session that is not the live one, and unwrap the removal. +/// +/// The live-session id is `0`, which no store ever allocates, so every test that +/// is not *about* the guard is unaffected by it. `f6`'s own test names a real id. +fn removed(store: &Store, session: i64) -> io_cli::store::Removed { + match io_cli::store::remove(store, session, 0).expect("the removal runs") { + io_cli::store::Removal::Done(removed) => *removed, + io_cli::store::Removal::Live => panic!("session {session} is not the live one"), + } +} + +/// A timestamp after every session any of these tests can create. +const AFTER_EVERYTHING: &str = "2999-01-01T00:00:00.000Z"; + +/// A timestamp before every session any of these tests can create. +const BEFORE_EVERYTHING: &str = "2000-01-01T00:00:00.000Z"; + +// --------------------------------------------------------------------------- +// F1 — the file's own arithmetic, and what a deletion does not do to it +// --------------------------------------------------------------------------- + +/// **F1 — the page reads io-harness's figures and computes none of its own.** +/// +/// Asserted as an equality against `Store::store_size` rather than by checking +/// the numbers look plausible, because the defect this guards against is io-cli +/// deriving a total from the rows it happens to have listed — which would be +/// right on a small store and wrong on the one an operator came here about. +#[test] +fn f1_the_view_reports_the_stores_own_figures() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/one", 3); + + let view = view(&store).expect("the store is readable"); + let direct = store.store_size().expect("a size is readable"); + + assert_eq!( + view.size, direct, + "the view must not compute its own totals" + ); + assert_eq!(view.reclaimable(), direct.free_bytes); +} + +/// **F1 — a removal frees pages INTO the file and never shrinks it.** +/// +/// The three assertions are the three halves of the sentence the module exists +/// to make true, and the middle one is an **equality**: `file_bytes` is the same +/// number afterwards, not merely no larger. +#[test] +fn f1_a_removal_frees_pages_into_the_file_and_the_file_does_not_move() { + let (_dir, store) = on_disk(); + let doomed = seed(&store, "/tmp/doomed", 12); + seed(&store, "/tmp/kept", 2); + + let removed = removed(&store, doomed); + + assert_eq!(removed.pruned.sessions, 1); + assert!(removed.pruned.turns >= 12, "every turn is accounted for"); + assert!(removed.pruned.bytes > 0, "rows carried bytes"); + assert_eq!( + removed.before.file_bytes, removed.after.file_bytes, + "a deletion must not change the size of the file on disk", + ); + assert!( + !removed.file_moved(), + "file_moved() is the same claim and must agree", + ); + assert!( + removed.after.free_bytes >= removed.before.free_bytes, + "free space inside the file never goes down on a removal", + ); + // **`>=` and not `>`, and that is a fact about SQLite rather than a weak + // assertion.** The adversarial review proposed strengthening this to a strict + // `>`; it was tried and it is **false**. A deletion frees *rows*, and pages + // reach the freelist only when a page empties — a twelve-turn session does + // not fill one, so `free_bytes` is unchanged and a compaction afterwards + // returns zero. Both were run before this comment was written. + // + // The claim this criterion actually makes is the equality above: the file on + // disk did not move. That the freed pages go *into* the file is asserted by + // `f2_only_compact_shrinks_the_file_and_reports_the_measured_difference`, at + // a size where whole pages are freed and the compaction has something to + // return. Asserting it here as well would be asserting arithmetic about + // SQLite's page layout, which is 0.26.0's recorded mistake in a new place. +} + +/// **F1 — the report says the file did not move, and names the compaction.** +/// +/// A report that only said what was deleted would leave the operator believing +/// they had reclaimed the bytes it named. +#[test] +fn f1_the_report_says_the_file_is_unchanged_and_what_would_change_it() { + let (_dir, store) = on_disk(); + let doomed = seed(&store, "/tmp/doomed", 6); + let removed = removed(&store, doomed); + + let report = removed_report(&removed).join("\n"); + assert!(report.contains("removed 1 session"), "{report}"); + assert!( + report.contains("still"), + "the report must say the file is still its old size: {report}", + ); + assert!( + report.contains("/store compact"), + "and must name the one thing that changes that: {report}", + ); +} + +// --------------------------------------------------------------------------- +// F2 — only a compaction shrinks the file, and it reports a measured figure +// --------------------------------------------------------------------------- + +/// **F2 — compaction is the only thing that shrinks the file, and the figure it +/// reports is measured.** +/// +/// The returned number is asserted against the difference between the two +/// `store_size` readings rather than against the freelist, because those are +/// different numbers and io-harness deliberately returns the true one. +#[test] +fn f2_only_compact_shrinks_the_file_and_reports_the_measured_difference() { + let (_dir, store) = on_disk(); + let doomed = seed(&store, "/tmp/doomed", 40); + seed(&store, "/tmp/kept", 1); + removed(&store, doomed); + + let freed = compact(&store).expect("the store compacts"); + + assert!( + freed.after.file_bytes < freed.before.file_bytes, + "a compaction is the one call that shrinks the file", + ); + assert_eq!( + freed.bytes, + freed.before.file_bytes - freed.after.file_bytes, + "the figure reported is the measured shrink, not the freelist", + ); + assert!(!freed.is_nothing()); + + let report = freed_report(&freed).join("\n"); + assert!(report.contains("returned"), "{report}"); +} + +/// **F2 — a store with nothing free reclaims nothing, and says so.** +/// +/// Zero is an honest answer and must not render as an empty success. This is +/// also the arm that stops the report being written as "returned {bytes}" with no +/// branch, which would print `returned 0 B` and read as a failure. +#[test] +fn f2_compacting_a_store_with_nothing_free_returns_zero_and_says_so() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/kept", 2); + // Compact once to reach a state with nothing free, then again to observe it. + compact(&store).expect("the store compacts"); + + let freed = compact(&store).expect("the store compacts again"); + + assert!(freed.is_nothing(), "nothing was free to reclaim"); + let report = freed_report(&freed).join("\n"); + assert!( + report.contains("nothing to reclaim"), + "zero must be a sentence of its own: {report}", + ); +} + +// --------------------------------------------------------------------------- +// F3 — absent is not empty +// --------------------------------------------------------------------------- + +/// **F3 — a session that is not there and a session that holds nothing are +/// different answers.** +/// +/// io-harness keeps them apart on purpose (`state/sessions.rs:305-309`): an +/// operator sweeping a list of ids needs to know which of them were already gone. +/// Collapsing `None` into a zeroed `SessionSize` tells them every id was empty. +#[test] +fn f3_absent_is_not_the_same_answer_as_empty() { + let (_dir, store) = on_disk(); + let empty = store.create_session("/tmp/empty").expect("a session opens"); + + let held = sized(&store, empty).expect("a size is readable"); + let missing = sized(&store, 9_999).expect("a missing session is not an error"); + + assert!(matches!(held, Sized::Holds(_)), "an empty session exists"); + assert_eq!(held.size().expect("figures").turns, 0, "and holds nothing"); + assert_eq!(missing, Sized::Absent, "a missing session is absent"); + assert!(missing.size().is_none()); +} + +/// **F3 — the two answers produce different confirmations.** +/// +/// The confirmation for a session that is not there offers no acting row at all, +/// because there is nothing to act on. A confirmation that offered "remove it" +/// for a session the store does not have would be a button that cannot work. +#[test] +fn f3_the_confirmation_for_a_missing_session_offers_nothing_to_do() { + let (title, rows) = confirm_remove(9_999, &Sized::Absent, 0); + + assert!(title.contains("no session 9999"), "{title}"); + assert_eq!(rows.len(), 1, "only the declining row"); + assert_eq!(rows[0].label, LEAVE_IT); +} + +/// **F3 — removing a session the store does not have succeeds and reports +/// nothing removed.** +/// +/// io-harness's own behaviour, not softened here: it is not an error, and it must +/// not be reported as one. +#[test] +fn f3_removing_a_session_that_is_not_there_reports_nothing() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/kept", 1); + + let removed = removed(&store, 9_999); + + assert_eq!(removed.pruned.sessions, 0); + assert_eq!(removed.pruned.turns, 0); + assert_eq!(removed.before.file_bytes, removed.after.file_bytes); +} + +// --------------------------------------------------------------------------- +// F4 — the sweep, its refusal, and the rule it asks for +// --------------------------------------------------------------------------- + +/// **F4 — a date before everything sweeps nothing.** +/// +/// The half of "strictly before" that *is* reachable from this crate. Without +/// it, a sweep that ignored its argument entirely would pass every other test in +/// this file. +#[test] +fn f4_a_date_before_everything_sweeps_nothing() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/one", 2); + seed(&store, "/tmp/two", 2); + + let swept = sweep(&store, BEFORE_EVERYTHING).expect("the sweep runs"); + + assert_eq!(swept.pruned.sessions, 0, "nothing was created before 2000"); + assert!(swept.refused().is_empty()); +} + +/// **F4 — a resumable session is refused, and named.** +/// +/// The refusal is the interesting half: a report that showed only what went would +/// tell the operator they had swept sessions that are still sitting there. +#[test] +fn f4_a_resumable_session_is_refused_and_named() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/finished", 3); + let alive = seed_resumable(&store, "/tmp/alive"); + + let swept = sweep(&store, AFTER_EVERYTHING).expect("the sweep runs"); + + assert_eq!( + swept.refused(), + &[alive], + "the session holding a resumable run is refused, not deleted", + ); + assert!( + swept.pruned.sessions >= 1, + "and the finished one is still swept", + ); + assert!( + matches!(sized(&store, alive), Ok(Sized::Holds(_))), + "a refused session is still in the store", + ); + + let report = swept_report(&swept).join("\n"); + assert!( + report.contains(&alive.to_string()), + "the refused id must be named: {report}", + ); + assert!(report.contains("resumable"), "{report}"); +} + +/// **F4 — a sweep that refused nothing says so.** +/// +/// "Refused none" and "we did not check" are the same silence otherwise, and the +/// refusal is the half an operator has to act on. +#[test] +fn f4_a_sweep_that_refused_nothing_says_nothing_was_refused() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/finished", 2); + + let swept = sweep(&store, AFTER_EVERYTHING).expect("the sweep runs"); + let report = swept_report(&swept).join("\n"); + + assert!(swept.refused().is_empty()); + assert!(report.contains("nothing was refused"), "{report}"); +} + +/// **F4 — the confirmation names the rule and never a count.** +/// +/// The counts cannot be known before the sweep runs — there is no public reader +/// for `sessions.created_at`, which is the column the sweep filters on +/// (io-harness#216, `US-IO-CLI-0.27.0-I02`). A confirmation carrying a number +/// here would be carrying one io-cli invented, and the one available substitute +/// under-states the deletion. +#[test] +fn f4_the_sweep_confirmation_names_the_rule_and_carries_no_count() { + let (title, rows) = confirm_sweep("2026-08-01"); + + assert!( + title.contains("2026-08-01"), + "the rule names the date: {title}" + ); + assert!( + !title + .chars() + .any(|c| c.is_ascii_digit() && !title.contains("2026-08-01")), + "no figure other than the date itself", + ); + assert_eq!(rows[0].label, LEAVE_IT); + let detail = rows[1].detail.clone().unwrap_or_default(); + assert!( + detail.contains("refused"), + "the refusal policy is stated up front: {detail}", + ); + assert!( + detail.contains("reported"), + "and the operator is told the figures come afterwards: {detail}", + ); +} + +// --------------------------------------------------------------------------- +// F5 — the confirmation, and the row that does nothing +// --------------------------------------------------------------------------- + +/// **F5 — row 0 declines in every confirmation this module builds.** +/// +/// Asserted by **index**, across all three, because a `Picker` opens on its first +/// row: the keystroke an operator gives by reflex has to be the one that changes +/// nothing. A label comparison alone would not catch an acting row that drifted +/// to the top. +#[test] +fn f5_row_zero_declines_in_every_confirmation() { + let (_dir, store) = on_disk(); + let session = seed(&store, "/tmp/one", 1); + let size = store.store_size().expect("a size is readable"); + let held = sized(&store, session).expect("a size is readable"); + + for (what, rows) in [ + ("remove", confirm_remove(session, &held, 0).1), + ("sweep", confirm_sweep("2026-08-01").1), + ("compact", confirm_compact(&size).1), + ] { + assert_eq!(rows[0].label, LEAVE_IT, "{what}: row 0 must decline"); + assert!( + !acts(0), + "{what}: and the driver's predicate must agree that it does", + ); + assert!(rows.len() >= 2, "{what}: there is something to choose"); + for index in 1..rows.len() { + assert!(acts(index), "{what}: every other row acts"); + } + } +} + +/// **F5 — declining changes nothing.** +/// +/// The whole `StoreSize` is compared, not one field, so an operation that ran and +/// then reported a refusal is caught as well as one that reported honestly and +/// ran anyway. +#[test] +fn f5_declining_leaves_the_store_byte_identical() { + let (_dir, store) = on_disk(); + let session = seed(&store, "/tmp/one", 4); + let before = store.store_size().expect("a size is readable"); + + // What the driver does with row 0: nothing at all. The predicate is the whole + // decision and it lives in the library precisely so this can be asserted — + // `src/main.rs` is a binary nothing under `tests/` can link. + if acts(0) { + removed(&store, session); + } + + let after = store.store_size().expect("a size is readable"); + assert_eq!(before, after, "declining must change nothing at all"); + assert!(matches!(sized(&store, session), Ok(Sized::Holds(_)))); +} + +/// **F5 — the removal confirmation states what it costs and what goes with it.** +/// +/// The restore-point sentence is in the confirmation rather than in the report, +/// because it is the fact an operator needs *before* they agree: they delete an +/// old session, and reach for its rewind a week later. +#[test] +fn f5_the_removal_confirmation_states_the_cost_and_the_restore_points() { + let (_dir, store) = on_disk(); + let session = seed(&store, "/tmp/one", 5); + let held = sized(&store, session).expect("a size is readable"); + + let (title, rows) = confirm_remove(session, &held, 0); + + assert!(title.contains(&session.to_string()), "{title}"); + assert!( + title.contains("turn"), + "the figures are in the title: {title}" + ); + let detail = rows[1].detail.clone().unwrap_or_default(); + assert!(detail.contains("restore points"), "{detail}"); + assert!( + detail.contains("memory does not"), + "and what survives, which is the half that surprises: {detail}", + ); +} + +/// **F5 — the compaction confirmation states what it costs while it runs.** +/// +/// io-harness's own warning, stated before the call rather than discovered by a +/// full disk. +#[test] +fn f5_the_compaction_confirmation_states_what_it_needs_while_it_runs() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/one", 2); + let size = store.store_size().expect("a size is readable"); + + let (_title, rows) = confirm_compact(&size); + let detail = rows[1].detail.clone().unwrap_or_default(); + + assert!(detail.contains("rewrites"), "{detail}"); + assert!(detail.contains("free on disk"), "{detail}"); +} + +// --------------------------------------------------------------------------- +// The page, and O4 +// --------------------------------------------------------------------------- + +/// The page names the file, what is free inside it, and the verbs. +/// +/// Also the one place the "free inside the file" wording is asserted: calling it +/// *reclaimable* on the page would invite the reading that a deletion returned it. +#[test] +fn the_page_names_the_file_the_free_space_and_the_three_verbs() { + let (_dir, store) = on_disk(); + let doomed = seed(&store, "/tmp/doomed", 8); + seed(&store, "/tmp/kept", 1); + removed(&store, doomed); + + let theme = io_cli::theme::THEMES[0]; + let lines = committed(&store, &theme, 100).expect("the page renders"); + let text = lines + .iter() + .flat_map(|line| line.spans.iter().map(|span| span.content.as_ref())) + .collect::(); + + assert!(text.contains("on disk"), "{text}"); + assert!(text.contains("free inside it"), "{text}"); + assert!(text.contains("/store compact"), "{text}"); + assert!(text.contains("/store rm"), "{text}"); + assert!(text.contains("/store sweep"), "{text}"); + assert!( + !text.contains("reclaimed"), + "nothing on this page may say a deletion reclaimed anything: {text}", + ); +} + +/// **O4 — no gate in this file can reach a real store.** +/// +/// A test that compacted or swept the developer's own `~/.io-cli/runs.db` would +/// be data loss shipped as a test. Read as text, because that is the only +/// instrument that sees what a test *could* open rather than what it did. +#[test] +fn o4_no_store_gate_names_a_real_home() { + let source = include_str!("store.rs"); + let code: String = source + .lines() + .filter(|line| !line.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + + // **The needles are assembled rather than written out, and that is the whole + // reason this loop looks odd.** A gate that reads its own file cannot spell + // the thing it forbids: the array would be the first match. io-cli has now + // hit this in 0.16.0, 0.19.0, 0.25.0 and 0.26.0 — twice in one wave in + // 0.25.0, where two agents each wrote a forbidden name into a comment in + // order to say the file did not use it. Stripping comments, which the sweep + // above does, is only half the fix; the other half is here. + for (head, tail) in [ + (".io", "-cli"), + ("home", "_dir"), + ("IO_CONFIG", "_HOME"), + ("USER", "PROFILE"), + ] { + let forbidden = format!("{head}{tail}"); + assert!( + !code.contains(&forbidden), + "a store gate must never be able to reach a real home, and this file names `{forbidden}`", + ); + } + assert!( + code.contains("tempfile::tempdir"), + "every store here is a temporary one", + ); + // Assembled for the same reason as the loop above: spelled out, this needle + // is its own first match. + let in_memory = format!("{}::{}", "Store", "memory"); + assert!( + !code.contains(&in_memory), + "and a real file, because the subject is page arithmetic on one", + ); +} + +// --------------------------------------------------------------------------- +// The date the sweep is given, and the session it is not allowed to remove +// --------------------------------------------------------------------------- + +/// **F4 — a sweep boundary that is not a timestamp is refused before anything +/// is offered.** +/// +/// The comparison io-harness makes is **lexical**, against a +/// `%Y-%m-%dT%H:%M:%fZ` text column, so any string sorting above `'2'` selects +/// every session in the store. Two entirely plausible inputs do that, and +/// neither looks wrong when the confirmation echoes it back: +/// +/// - `2026-8-1`, an unpadded month, which sorts above `2026-08-28T…` +/// - `yesterday`, or any word, because every letter sorts above `'2'` +/// +/// **Both adversarial reviewers found this independently**, which is the +/// strongest signal that gate gives. +#[test] +fn f4_a_boundary_that_is_not_a_timestamp_is_refused() { + use io_cli::store::boundary; + + // The two that would have swept the whole store. + for wrong in [ + "2026-8-1", + "yesterday", + "last-week", + "now", + "9", + "", + "2026/08/01", + ] { + let refused = boundary(wrong).expect_err(&format!("`{wrong}` must be refused")); + assert!( + refused.contains(wrong) || wrong.is_empty(), + "the refusal names what was typed: {refused}", + ); + } + + // A bare date is normalised to midnight, which is what the operator meant. + assert_eq!( + boundary("2026-08-01").expect("a bare date is accepted"), + "2026-08-01T00:00:00.000Z", + ); + // A full stamp is taken as written. + assert_eq!( + boundary("2026-08-01T12:30:00.000Z").expect("a full stamp is accepted"), + "2026-08-01T12:30:00.000Z", + ); + + // **The property that matters: an accepted boundary never sorts above a real + // timestamp unless it is meant to.** A store's stamps look like this one. + let real = "2026-08-28T09:15:22.417Z"; + assert!( + boundary("2026-08-01").expect("accepted").as_str() < real, + "a boundary before today must sort before today's sessions", + ); + assert!( + boundary("2026-09-01").expect("accepted").as_str() > real, + "and one after it must sort after", + ); +} + +/// **The session io is running is refused, and the confirmation offers no way to +/// do it.** +/// +/// `/store` lists every session **with its root**, which is exactly what makes +/// the operator's own workspace the recognisable one. io-harness refuses nothing +/// — `delete_session` has no guard, the resumable-run refusal lives only in +/// `sweep_sessions`, and no foreign keys are enforced — so the removal would +/// succeed, the process would carry on writing turns into a session that no +/// longer exists, and every later `/undo` and `/export` would report an empty +/// session. A false report, from a session that is still running. +/// +/// Found by both adversarial reviewers. +#[test] +fn the_live_session_cannot_be_removed() { + let (_dir, store) = on_disk(); + let live = seed(&store, "/tmp/live", 3); + let other = seed(&store, "/tmp/other", 2); + + assert_eq!( + io_cli::store::remove(&store, live, live).expect("not an error"), + io_cli::store::Removal::Live, + "the conversation this process is having is not removable", + ); + assert!( + matches!(sized(&store, live), Ok(Sized::Holds(_))), + "and it is still in the store", + ); + + // The confirmation does not offer it either, so the refusal is visible before + // the keystroke rather than after it. + let held = sized(&store, live).expect("a size"); + let (title, rows) = confirm_remove(live, &held, live); + assert!(title.contains("the conversation you are in"), "{title}"); + assert_eq!(rows.len(), 1, "there is nothing to choose but leaving it"); + assert_eq!(rows[0].label, LEAVE_IT); + + // Any other session is unaffected. + assert!(matches!( + io_cli::store::remove(&store, other, live).expect("not an error"), + io_cli::store::Removal::Done(_) + )); +} + +/// **A sweep says how many restore points went, as a removal does.** +/// +/// `removed_report` has said it since it was written and the module preamble +/// calls it the sentence that bites later. A sweep removes many sessions' +/// restore points at once and said nothing, before or after. Found by the +/// adversarial review. +#[test] +fn f4_the_sweep_report_names_the_restore_points_it_took() { + let (_dir, store) = on_disk(); + seed(&store, "/tmp/one", 3); + + let swept = sweep(&store, AFTER_EVERYTHING).expect("the sweep runs"); + let report = swept_report(&swept).join("\n"); + + assert!(report.contains("swept"), "{report}"); + if swept.pruned.restore_points > 0 { + assert!( + report.contains("restore point"), + "a sweep that took restore points has to say so: {report}", + ); + assert!( + report.contains("memory did not"), + "and say what survived: {report}", + ); + } +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 1fbbf2f..0b59545 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -430,6 +430,30 @@ impl Scripted { batches: Mutex::new(VecDeque::from(vec![batch])), } } + + /// One batch per **step**, so a turn takes as many steps as there are slices. + /// + /// [`Scripted::writing`] puts every file in one batch, which is one step. A + /// test about undoing *one step* of a run needs the steps to be distinct, and + /// the only way to get that is to make the provider answer more than once — + /// io-harness's step boundary is a completion, not a tool call. + /// + /// Steps are numbered from one by the run loop, so `in_steps(&[a, b])` writes + /// `a` at step 1 and `b` at step 2. + pub fn in_steps(steps: &[&[(&str, &str)]]) -> Self { + let batches = steps + .iter() + .map(|files| { + files + .iter() + .map(|(path, content)| write_call(path, content)) + .collect() + }) + .collect::>>(); + Self { + batches: Mutex::new(VecDeque::from(batches)), + } + } } impl Provider for Scripted { diff --git a/tests/triage.rs b/tests/triage.rs index 92f6cce..2b658e3 100644 --- a/tests/triage.rs +++ b/tests/triage.rs @@ -254,11 +254,6 @@ fn a_status_or_silent_kind_commits_nothing() { host: "example.com:443".into(), permitted: true, }, - EventKind::Speculated { - started: 2, - used: 1, - discarded: 1, - }, EventKind::PluginDropped { plugin: "acme".into(), why: "manifest unreadable".into(), @@ -372,3 +367,105 @@ fn the_dispositions_are_the_three_the_contract_names() { "if almost everything is a line, nothing was triaged: {status} status, {silent} silent", ); } + +/// **0.27.0 — the one silence that had no route, and now has a line.** +/// +/// `speculated` routed to `io exec --json` and the durable trace, and both of +/// those are places somebody goes deliberately, afterwards, already suspecting +/// something. A route is meant to name where the fact reaches an operator who +/// was *not* looking for it. The other eight silences reviewed in the same pass +/// keep theirs, because they are better arguments than drawing would be — see +/// `US-IO-CLI-0.27.0-I04`. +#[test] +fn f9_a_discarded_read_is_drawn_and_a_perfect_one_is_not() { + let mut events = Events::new(DARK); + + assert_eq!( + triage::disposition("speculated"), + Some(Disposition::Line), + "speculated is a line since 0.27.0", + ); + + let lines = events.event( + &event(EventKind::Speculated { + started: 3, + used: 1, + discarded: 2, + }), + Duration::ZERO, + ); + let text = lines + .iter() + .flat_map(|line| line.spans.iter().map(|span| span.content.as_ref())) + .collect::(); + assert!(text.contains("read ahead"), "{text}"); + assert!(text.contains('3'), "started is on the line: {text}"); + assert!(text.contains('2'), "and so is what was thrown away: {text}"); + + // **A step that speculated perfectly has nothing to report.** io-harness + // emits this whenever `started > 0`, so without the guard every transcript + // would carry a line per step saying nothing went wrong — and a line that is + // always there is a line nobody reads. + let quiet = events.event( + &event(EventKind::Speculated { + started: 4, + used: 4, + discarded: 0, + }), + Duration::ZERO, + ); + assert!( + quiet.is_empty(), + "nothing was discarded, so there is nothing to say: {quiet:?}", + ); + assert_eq!( + events.unknown(), + 0, + "declining an event is not the same as not knowing the kind", + ); +} + +/// **Every `Silent` route names a surface this product actually ships.** +/// +/// A route is the whole justification for a silence: the fact is not on screen +/// *because it reaches the operator another way*. Until this release that claim +/// was prose nobody checked, and `speculated` is what a route looks like when it +/// is wrong — it named two places a person only goes deliberately, afterwards. +/// +/// So every route must name a command this build registers, or one of the two +/// machine surfaces. A route naming a command that does not exist is a silence +/// with nothing behind it. +#[test] +fn f9_every_silent_route_names_a_surface_that_exists() { + let commands: Vec<&str> = io_cli::commands::COMMANDS + .iter() + .map(|(name, _)| *name) + .collect(); + + for (name, disposition, route) in triage::TRIAGE { + if *disposition != Disposition::Silent { + continue; + } + assert!( + !route.is_empty(), + "{name} is silent and says nothing about why" + ); + + // A route may name a command, one of the two machine surfaces, or a + // surface of this interface that is not a slash command — the overlay, a + // committed summary, the answer itself. The first is the one that can go + // stale, so it is the one that is checked: any `/word` a route names must + // be a command this build registers. + for word in route.split_whitespace() { + let candidate = word.trim_matches(|c: char| !c.is_ascii_alphanumeric() && c != '/'); + if let Some(slash) = candidate.strip_prefix('/').map(|rest| format!("/{rest}")) { + if slash.len() > 1 { + assert!( + commands.contains(&slash.as_str()), + "{name} routes to {slash}, which this build does not register", + ); + } + } + } + } +} diff --git a/tests/undo.rs b/tests/undo.rs new file mode 100644 index 0000000..bbe1fdc --- /dev/null +++ b/tests/undo.rs @@ -0,0 +1,454 @@ +//! Putting work back at three granularities, and the two events that proves. +//! +//! **The assertion that matters most here is that an event *arrives*.** +//! `EventKind::Rewound` is emitted only inside `rewind_run_observed` +//! (`run.rs:804`) and `EventKind::Reverted` only inside `rewind_step_observed` +//! (`run.rs:1011`). `src/rewind.rs` called the plain `rewind_run` from 0.4.0 +//! until this release, so neither has ever fired in this product's history — and +//! `src/events.rs` has had an arm for one of them the whole time, unreachable. +//! +//! So the gates below collect what an observer was handed. Asserting the +//! rendered sentence would pass just as happily on the unobserved call, which is +//! the defect: the words come from io-cli either way, and only the event says +//! which function was called. +//! +//! **The workspace assertions are on bytes on disk, never on a message.** An +//! undo that reports success and leaves the file alone is the whole failure mode. + +mod support; + +use std::sync::Mutex; + +use io_cli::undo::{ + confirm_file, confirm_step, one_file, one_step, said, step_advice, step_said, Grain, +}; +use io_harness::tools::Workspace; +use io_harness::{ + ApproveAll, EventKind, Flow, Observer, Policy, Reverted, Rewind, RunEvent, Session, Store, +}; +use support::Scripted; + +/// An observer that keeps every event it is handed. +/// +/// The only instrument that can tell `rewind_run` from `rewind_run_observed`. +#[derive(Default)] +struct Collected { + events: Mutex>, +} + +impl Observer for Collected { + fn event(&self, event: &RunEvent) -> Flow { + self.events + .lock() + .expect("the lock is not poisoned") + .push(event.kind.clone()); + Flow::Continue + } +} + +impl Collected { + fn kinds(&self) -> Vec { + self.events + .lock() + .expect("the lock is not poisoned") + .clone() + } + + fn saw_rewound(&self) -> bool { + self.kinds() + .iter() + .any(|kind| matches!(kind, EventKind::Rewound { .. })) + } + + fn saw_reverted(&self) -> Option { + self.kinds().iter().find_map(|kind| match kind { + EventKind::Reverted { undid_step, .. } => Some(*undid_step), + _ => None, + }) + } +} + +/// A temporary workspace, a store, and a session over the two. +/// +/// **Every run here is a real turn.** A restore point is written by io-harness's +/// own run loop through `Store::record_snapshot`, which is `pub(crate)` — so a +/// test cannot seed one, and a fixture that inserted rows by hand would be +/// testing a shape production does not produce. This is the same fixture +/// `tests/rewind.rs` has used since 0.4.0, for the same reason. +struct Fixture { + dir: tempfile::TempDir, + store: Store, + session: Session, +} + +impl Fixture { + fn new() -> Self { + 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"); + Self { + dir, + store, + session, + } + } + + /// One turn whose agent writes every one of `files`, in one step. + async fn turn_writing(&mut self, prompt: &str, files: &[(&str, &str)]) -> i64 { + self.drive(prompt, Scripted::writing(files)).await + } + + /// One turn that takes a step per slice, so the steps are distinct. + async fn turn_in_steps(&mut self, prompt: &str, steps: &[&[(&str, &str)]]) -> i64 { + self.drive(prompt, Scripted::in_steps(steps)).await + } + + async fn drive(&mut self, prompt: &str, provider: Scripted) -> i64 { + self.session + .turn( + prompt, + &provider, + &self.store, + &Policy::permissive(), + &ApproveAll, + ) + .await + .expect("a scripted turn cannot fail"); + let head = self.session.head().expect("the turn is on the head"); + self.store + .session_turn(head) + .expect("the turn is readable") + .expect("the turn is there") + .run_id + } + + fn workspace(&self) -> Workspace { + Workspace::new(self.dir.path()) + } + + fn read(&self, path: &str) -> Option { + std::fs::read_to_string(self.dir.path().join(path)).ok() + } + + fn exists(&self, path: &str) -> bool { + self.dir.path().join(path).exists() + } +} + +// --------------------------------------------------------------------------- +// F6 — one file, and four answers that stay four answers +// --------------------------------------------------------------------------- + +/// **F6 — a modified file comes back with its pre-run bytes.** +/// +/// Asserted on the workspace, never on the returned sentence. +#[tokio::test] +async fn f6_a_modified_file_comes_back_as_it_was() { + let mut fixture = Fixture::new(); + std::fs::write(fixture.dir.path().join("thing.rs"), "the original\n") + .expect("the file starts out as the operator left it"); + let run = fixture + .turn_writing("edit it", &[("thing.rs", "what the agent wrote\n")]) + .await; + assert_eq!( + fixture.read("thing.rs").as_deref(), + Some("what the agent wrote\n") + ); + + let answer = + one_file(&fixture.workspace(), &fixture.store, run, "thing.rs").expect("the undo runs"); + + assert!(matches!(answer, Rewind::Restored(_)), "{answer:?}"); + assert_eq!( + fixture.read("thing.rs").as_deref(), + Some("the original\n"), + "the bytes on disk are the pre-run bytes", + ); +} + +/// **F6 — a file the run created is removed, not emptied.** +#[tokio::test] +async fn f6_a_file_the_run_created_is_removed() { + let mut fixture = Fixture::new(); + let run = fixture + .turn_writing("make it", &[("new.rs", "brand new\n")]) + .await; + assert!(fixture.exists("new.rs")); + + let answer = + one_file(&fixture.workspace(), &fixture.store, run, "new.rs").expect("the undo runs"); + + assert_eq!(answer, Rewind::Removed); + assert!( + !fixture.exists("new.rs"), + "a created file is gone, not left empty", + ); +} + +/// **F6 — a path the run never touched answers `NotRecorded` and changes +/// nothing.** +#[tokio::test] +async fn f6_a_path_the_run_never_touched_is_not_recorded() { + let mut fixture = Fixture::new(); + let run = fixture + .turn_writing("touch one", &[("written.rs", "by the agent\n")]) + .await; + std::fs::write(fixture.dir.path().join("untouched.rs"), "mine\n") + .expect("the operator's own file"); + + let answer = + one_file(&fixture.workspace(), &fixture.store, run, "untouched.rs").expect("the undo runs"); + + assert_eq!(answer, Rewind::NotRecorded); + assert_eq!( + fixture.read("untouched.rs").as_deref(), + Some("mine\n"), + "an unrecorded path is left exactly alone", + ); +} + +/// **F6 — the four answers are four sentences.** +/// +/// `NotKept` and `NotRecorded` in particular: collapsing them tells an operator +/// "nothing to undo" about a file the run very much wrote, whose previous +/// contents were merely over the snapshot cap. +#[test] +fn f6_the_four_answers_say_four_different_things() { + let restored = said("a.rs", &Rewind::Restored(io_harness::tools::Wrote::Changed)); + let removed = said("a.rs", &Rewind::Removed); + let not_kept = said("a.rs", &Rewind::NotKept("over the cap".to_string())); + let not_recorded = said("a.rs", &Rewind::NotRecorded); + + let all = [&restored, &removed, ¬_kept, ¬_recorded]; + for (i, one) in all.iter().enumerate() { + for (j, other) in all.iter().enumerate() { + if i != j { + assert_ne!(one, other, "every answer is its own sentence"); + } + } + } + assert!(not_kept.contains("not kept"), "{not_kept}"); + assert!( + not_kept.contains("over the cap"), + "the reason survives: {not_kept}" + ); + assert!(not_recorded.contains("no restore point"), "{not_recorded}"); + assert!( + removed.contains("created"), + "a removal says why it was a removal: {removed}", + ); +} + +// --------------------------------------------------------------------------- +// F7 — one step, the rest of the run untouched, and the events that prove it +// --------------------------------------------------------------------------- + +/// **F7 — undoing one step leaves the other step's file exactly as the run left +/// it, and `EventKind::Reverted` arrives.** +/// +/// The event is the assertion that proves the `_observed` form is what was +/// called. A rendered sentence would be identical either way. +#[tokio::test] +async fn f7_one_step_is_undone_and_the_rest_of_the_run_stands() { + let mut fixture = Fixture::new(); + std::fs::write(fixture.dir.path().join("two.rs"), "two before\n").expect("a starting file"); + std::fs::write(fixture.dir.path().join("three.rs"), "three before\n").expect("a starting file"); + let run = fixture + .turn_in_steps( + "two steps", + &[ + &[("two.rs", "two after\n")], + &[("three.rs", "three after\n")], + ], + ) + .await; + + // Step 2 is the second batch, because the run loop numbers from one. + let watcher = Collected::default(); + let answers = + one_step(&fixture.workspace(), &fixture.store, run, 1, &watcher).expect("the undo runs"); + + assert!( + !answers.is_empty(), + "step 1 wrote a file, so it has an answer" + ); + assert_eq!( + fixture.read("two.rs").as_deref(), + Some("two before\n"), + "the undone step's file is back", + ); + assert_eq!( + fixture.read("three.rs").as_deref(), + Some("three after\n"), + "and the other step's is exactly as the run left it", + ); + assert_eq!( + watcher.saw_reverted(), + Some(1), + "EventKind::Reverted arrived, naming the step — which only the observed \ + form emits, and which nothing in this product had ever emitted", + ); +} + +/// **F7 — undoing the whole run emits `EventKind::Rewound`.** +/// +/// The event this product has never fired. `src/rewind.rs:200` called the plain +/// `rewind_run` from 0.4.0 until this release, so the event had never fired. +/// +/// **What that buys is not a line on screen.** `rewound` is `Disposition::Silent` +/// and routes to io-cli's own rewind summary; `src/events.rs` has no arm for it +/// and correctly draws nothing. What the observed form buys is that the event +/// exists at all — it reaches `[[hook]]` observers, `io exec --json` and the +/// durable trace, none of which it had ever reached. An earlier draft of this +/// file claimed there was an unreachable renderer arm; there is not, and the +/// adversarial review caught the overclaim. +/// +/// It goes through `rewind::last_turn`, which is the production path: that is +/// what moves the conversation head back, and a thin wrapper beside it would be +/// a second entry point to one act. +#[tokio::test] +async fn f7_the_whole_run_emits_rewound() { + let mut fixture = Fixture::new(); + std::fs::write(fixture.dir.path().join("one.rs"), "before\n").expect("a starting file"); + fixture + .turn_writing("a whole run", &[("one.rs", "after\n")]) + .await; + + let watcher = Collected::default(); + let undone = io_cli::rewind::last_turn(&mut fixture.session, &fixture.store, &watcher) + .expect("the undo runs") + .expect("there was a turn to undo"); + + assert!(!undone.restored.is_empty(), "there was a file to put back"); + assert_eq!( + fixture.read("one.rs").as_deref(), + Some("before\n"), + "and it is back", + ); + assert!( + watcher.saw_rewound(), + "EventKind::Rewound arrived — the assertion that the observed form was \ + called, which no rendered sentence could make", + ); +} + +/// A step that wrote nothing is not an error, and says so. +#[tokio::test] +async fn a_step_that_wrote_nothing_changes_nothing_and_is_not_an_error() { + let mut fixture = Fixture::new(); + let run = fixture.turn_writing("read only", &[]).await; + + let watcher = Collected::default(); + let answers = + one_step(&fixture.workspace(), &fixture.store, run, 1, &watcher).expect("not an error"); + + assert!( + answers.is_empty(), + "nothing was written, so nothing came back" + ); +} + +/// **The order-sensitivity sentence is said only when something came back +/// stale.** +/// +/// Advice about a problem the operator does not have is noise, and noise in a +/// destructive surface is what teaches people to stop reading it. +#[test] +fn the_stale_advice_is_given_only_when_something_is_stale() { + let clean = vec![( + "a.rs".to_string(), + Reverted::Applied(io_harness::tools::Wrote::Changed), + )]; + assert_eq!(step_advice(&clean), None, "nothing stale, nothing said"); + + let stale = vec![("a.rs".to_string(), Reverted::Stale("moved".to_string()))]; + let advice = step_advice(&stale).expect("a stale answer earns the sentence"); + assert!(advice.contains("newest step first"), "{advice}"); + assert!( + advice.contains("order-sensitive"), + "and says why, or it reads as a bug: {advice}", + ); + + let said = step_said("a.rs", &Reverted::Stale("context moved".to_string())); + assert!( + said.contains("unchanged"), + "a stale step changed nothing: {said}" + ); + assert!( + said.contains("context moved"), + "the reason survives: {said}" + ); +} + +// --------------------------------------------------------------------------- +// F10 — the group re-file +// --------------------------------------------------------------------------- + +/// **F10 — `/contain` is in `Session`, `/undo` is in `Turn`, `/store` and +/// `/export` are in `Inspect`, and no group is longer than ten.** +#[test] +fn f10_the_groups_are_where_this_release_put_them() { + use io_cli::commands::{group_of, Group, COMMANDS, GROUPS}; + + assert_eq!(group_of("/contain"), Some(Group::Session)); + assert_eq!(group_of("/undo"), Some(Group::Turn)); + assert_eq!(group_of("/store"), Some(Group::Inspect)); + assert_eq!(group_of("/export"), Some(Group::Inspect)); + + for (group, names) in GROUPS { + assert!( + names.len() <= 10, + "{group:?} holds {} commands; ten is the bound", + names.len(), + ); + } + assert_eq!( + COMMANDS.len(), + 36, + "0.27.0 adds exactly three commands to the thirty-three 0.26.0 shipped", + ); +} + +/// The confirmations decline at row 0, like every other one this release adds. +#[test] +fn the_undo_confirmations_decline_at_row_zero() { + for (what, rows) in [ + ("file", confirm_file("src/a.rs").1), + ("step", confirm_step(4).1), + ] { + assert_eq!(rows[0].label, io_cli::store::LEAVE_IT, "{what}"); + assert!(!io_cli::store::acts(0), "{what}"); + assert!(io_cli::store::acts(1), "{what}"); + } +} + +/// The file confirmation discloses the one thing an operator can lose. +/// +/// io-harness restores from the snapshot taken before the run first wrote the +/// path and does not compare it against what is on disk now, so an edit made by +/// hand after the turn is overwritten without a word. io-cli cannot detect that; +/// it can say so before the keystroke, which is what this asserts. +#[test] +fn the_file_confirmation_discloses_what_a_restore_overwrites() { + let (_title, rows) = confirm_file("src/a.rs"); + let detail = rows[1].detail.clone().unwrap_or_default(); + + assert!(detail.contains("by hand"), "{detail}"); + assert!(detail.contains("overwritten"), "{detail}"); +} + +/// `Grain` is a description of what was asked for and nothing more. +/// +/// It carries no `label()`: the confirmations build their own sentences and the +/// run arm never renders one, so a formatter here would be reachable only from +/// this file — the tested-but-unreachable shape this codebase has shipped three +/// times. The adversarial review caught it before it became a fourth. +#[test] +fn the_grains_carry_only_what_was_asked_for() { + assert_eq!( + Grain::File("src/a.rs".into()), + Grain::File("src/a.rs".into()) + ); + assert_ne!(Grain::Step(1), Grain::Step(2)); + assert_ne!(Grain::Run, Grain::Step(1)); +}