diff --git a/batten.toml b/batten.toml index 61b7a7a66..021c498c2 100644 --- a/batten.toml +++ b/batten.toml @@ -954,8 +954,11 @@ bytes the tracker returned, never a re-typed copy; do NOT re-type one by hand."" # `{authority:ready}` IS THE SIXTH FIELD AND IT IS APPENDED, never inserted # (CLOUD-1100). Every reader of this receipt is positional — `claim-check` takes # field 4 and `finding-sink-check` field 5 — so a field added at the end moves -# none of them, and `mise run issue-read-check`, which still writes five, keeps -# minting a receipt every consumer can read. It renders the Ready-block verdict +# none of them, and a five-field receipt already on disk stays readable by every +# consumer. (This used to say `mise run issue-read-check` "still writes five" — +# that task is retired and writes nothing; what survives is the positional +# compatibility, which is what the clause was about.) It renders the Ready-block +# verdict # from the COMPILED authority (`crates/batten/src/ready.rs`) rather than by # spawning a program, which is what makes it free here: the boundary already # holds the payload, so the verdict costs a library call on a read the agent was @@ -1970,8 +1973,19 @@ gated here.""" # caveat: `save_issue` takes no if-match precondition, so nothing can make the # write conditional. Existence alone was the gate that let CLOUD-504 through. # -# The receipt is minted by `mise run issue-read-check`, whose write side this row -# does not touch — the engine reads the file that task already writes. +# The receipt is minted by the engine's own `record_mints`, from the `[[mint]]` +# row above, and that is now the ONLY producer. This clause used to name +# `mise run issue-read-check` as the writer whose file the engine merely reads; +# that task no longer exists anywhere in the tree, so the sentence sent a reader +# hitting this refusal to a program they cannot run (CLOUD-1147). +# +# THE MINT RECOVERS AN INTERCEPTED RESULT. A host may refuse to hand over a large +# result, write the bytes to a file and substitute a notice naming it. The notice +# is prose, so the ordinary decode fails and nothing minted — measured three +# times, each leaving a row permanently un-updatable behind a refusal whose stated +# remedy is the operation that fails. `record_mints` now follows the path the host +# named in its own notice, so the receipt attests the bytes the server actually +# returned rather than a field subset. [[rule]] id = "an-update-owes-a-recent-read" kind = "receipt" diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index f06b39abc..f13a3518e 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -3396,6 +3396,52 @@ pub fn payload_in(result: &serde_json::Value) -> Option { } } +/// The file a host named when it substituted a notice for an over-limit result. +/// +/// # Why this exists at all +/// +/// A host may refuse to hand over a large tool result and write it to a file +/// instead, substituting a plain-text notice that names the path. Measured +/// 2026-09-01 (CLOUD-1147): a `get_issue` returning 71,501 characters arrived at +/// the hook as a STRING, so [`payload_in`]'s JSON parse failed and every mint +/// over that call was skipped — silently, because a mint's failure is silent by +/// design. Three rows had become permanently un-updatable that way, each with a +/// refusal whose stated remedy ("re-read the row") is the operation that fails. +/// +/// The bytes were never gone. They were in a file the notice named, and nothing +/// looked. This is the projection that looks. +/// +/// # Why a shape rather than a host's exact sentence +/// +/// The anchor is `saved to ` terminated by a period at end of line, which +/// is the shape of the notice rather than one host's wording. A host that phrases +/// it differently recovers nothing and the caller falls back to the ordinary +/// no-mint path, which is the direction a miss must fail in. +/// +/// **Pure, and it does not read the file.** Whether to open what this names is +/// the caller's decision, and it carries the bound: only a path a HOST put in a +/// result it substituted, never one a caller supplied. +#[must_use] +pub fn spilled_path(text: &str) -> Option<&str> { + let after = text.split_once("saved to ")?.1; + // The path ends at the period that closes the sentence, and a path may + // contain periods — so the terminator is a period at END OF LINE rather than + // the first period, and a notice on one line ends at the string's end. + let line = after.split('\n').next()?; + let path = line.strip_suffix('.').unwrap_or(line).trim(); + // ABSOLUTE ONLY. A relative path would resolve against whatever directory the + // hook happens to run in, which is not a thing the notice can have meant. + // + // ASKED OF `Path`, NOT SPELLED AS A LEADING SLASH, and that is a measured + // correction rather than a tidy-up. This read `path.starts_with('/')`, which + // is the shape of an absolute path on one platform: `D:\a\_temp\result.txt` + // fails it, so on Windows the recovery never fired and the whole class stayed + // starved there. The engine builds on Windows and its own suite runs there — + // the case below went red on that job and green everywhere else, which is + // exactly the half a Unix-only run cannot see. + (std::path::Path::new(path).is_absolute() && path.len() > 1).then_some(path) +} + /// One agent-sourced fact a consumer declares: its name, and the command whose /// output answers it. /// diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index fb3adc496..acebaa452 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -6879,6 +6879,45 @@ fn write_records(overrides: &Overrides, envelope: &hook::Envelope) { /// hook that cannot record a fact must not become the reason work stops. The gate /// that reads the receipt simply denies again with the same remedy, which is the /// safe direction and one the agent can see. +/// The payload a host wrote to a file when it refused to hand over a large result. +/// +/// CLOUD-1147. A host may substitute a plain-text notice for an over-limit tool +/// result and write the real bytes to a file it names. `payload_in` then fails to +/// parse — the notice is prose, not JSON — and every mint over that call is +/// skipped silently, which is what left three rows permanently un-updatable: the +/// `issue-read` receipt never minted, and `an-update-owes-a-recent-read` refused +/// with a remedy ("re-read the row") that is the very operation that fails. +/// +/// Measured 2026-09-01, on the live host: the envelope's `result` is a STRING +/// (not `null`, so the early return above is not what stops the mint) whose text +/// names an absolute path, and that file holds the complete payload the server +/// returned. +/// +/// # This is not CLOUD-691's forgery +/// +/// The receipt records what was SEEN. Reading the file recovers exactly the bytes +/// the server sent, so a receipt minted from it attests nothing that was not +/// returned — which is why this is a recovery rather than the field-subset +/// compromise CLOUD-1147 contemplated while it believed the bytes were gone. +/// +/// # The bounds, and each is load-bearing +/// +/// Only a path a HOST placed in a result it substituted, taken from the notice's +/// own shape — never one a caller supplied. Read ONCE, and only when the ordinary +/// decode already failed, so no clean result pays for this. Everything after is +/// unchanged: the recovered value goes through `payload_in` like any other, and a +/// mint's `requires` still decides, so a spilled file lacking the declared fields +/// mints nothing exactly as today. +/// +/// Every failure is silent and returns `None`, matching the mint boundary's own +/// documented posture: the gate that reads the receipt simply denies again. +fn recover_spilled(result: &serde_json::Value) -> Option { + let text = result.as_str()?; + let path = facts::spilled_path(text)?; + let bytes = std::fs::read_to_string(path).ok()?; + facts::payload_in(&serde_json::from_str(&bytes).ok()?) +} + fn record_mints(overrides: &Overrides, envelope: &hook::Envelope) { // Before the config load, the cheap question first: a post-tool event for a // tool no row names — which is nearly all of them, now that batten is @@ -6891,7 +6930,9 @@ fn record_mints(overrides: &Overrides, envelope: &hook::Envelope) { // blocks, so reading fields off `envelope.result` directly matches nothing in // production while passing every fixture, which hands the engine a bare // object. `facts::payload_in` is the one authority on that unwrap. - let Some(result) = facts::payload_in(&envelope.result) else { + let Some(result) = + facts::payload_in(&envelope.result).or_else(|| recover_spilled(&envelope.result)) + else { return; }; let Ok((policy, _)) = load_policy(overrides, hook::Harness::ExitCode) else { diff --git a/crates/batten/tests/it/board_receipts.rs b/crates/batten/tests/it/board_receipts.rs index 7095734bb..137edb1f6 100644 --- a/crates/batten/tests/it/board_receipts.rs +++ b/crates/batten/tests/it/board_receipts.rs @@ -1623,3 +1623,152 @@ fn a_zero_hit_search_still_mints_and_a_read_payload_never_does() { "a payload with no page key is not a search result" ); } + +// --------------------------------------------------------------------------- +// THE INTERCEPTED READ (CLOUD-1147). +// +// A host may refuse to hand over a large tool result, write the bytes to a file +// and substitute a plain-text notice naming it. The notice is prose, so +// `payload_in` cannot parse it and every mint over that call was skipped — +// silently, because a mint's failure is silent by design. Three rows became +// permanently un-updatable that way, each refused with a remedy ("re-read the +// row") that is the operation that fails, and fails BECAUSE the row is large. +// +// Measured 2026-09-01 on the live host: the envelope's `result` is a STRING +// naming an absolute path, and that file holds the complete payload. The bytes +// were never gone; nothing looked. +// +// The negatives below are the load-bearing half. A recovery that fired on a +// notice naming nothing, or on a file that is not a payload, would forge a +// receipt for a read that did not happen — worse than the starvation it fixes, +// and CLOUD-691's recorded class. +// --------------------------------------------------------------------------- + +/// The host's notice, in the shape it actually arrives: a sentence naming the +/// file, then the guidance lines that follow it. +fn interception_notice(path: &Path) -> String { + serde_json::to_string(&format!( + "Error: result (71,501 characters across 1 line) exceeds maximum allowed \ + tokens. Output has been saved to {}.\nFormat: Plain text\nUse offset and \ + limit parameters to read specific portions of the file.", + path.display() + )) + .expect("a notice is encodable") +} + +#[test] +fn an_intercepted_read_recovers_the_spilled_payload_and_mints() { + let repo = repo("mint-recovers-an-intercepted-read"); + let update = r#"{"id":"CLOUD-1","description":"groomed"}"#; + assert_eq!( + verdict(&repo, "mcp__Linear__save_issue", update), + Some(2), + "the gate denies before anything has read the row" + ); + + // The spill lives OUTSIDE the repository, as the host's own does. + let spill = common::scratch("mint-spill").join("result.txt"); + std::fs::create_dir_all(spill.parent().expect("a parent")).expect("spill dir"); + std::fs::write(&spill, READ_RESULT).expect("the host wrote the real bytes"); + + completed( + &repo, + "mcp__Linear__get_issue", + &interception_notice(&spill), + ); + + assert!( + receipt(&repo, "issue-read.CLOUD-1").is_some(), + "the payload the host spilled is the payload the server returned, so the \ + receipt it mints attests a read that really happened" + ); + assert_eq!( + verdict(&repo, "mcp__Linear__save_issue", update), + Some(0), + "and the row is updatable again — the whole point of CLOUD-1147" + ); +} + +/// ANTI-VACUITY, and the case that separates a recovery from a forgery: a notice +/// naming a file that is not there mints nothing. +/// +/// Without this, the case above is satisfied by a change that mints on any +/// unparseable result, which is precisely the forgery CLOUD-691 records. +#[test] +fn a_notice_naming_a_file_that_is_not_there_mints_nothing() { + let repo = repo("mint-spill-absent"); + let missing = common::scratch("mint-spill-absent-target").join("gone.txt"); + completed( + &repo, + "mcp__Linear__get_issue", + &interception_notice(&missing), + ); + assert!( + receipt(&repo, "issue-read.CLOUD-1").is_none(), + "nothing was read, so nothing may be attested" + ); +} + +/// A spilled file that is not a payload mints nothing either — the recovered +/// value goes through the same decode and the same `requires` as any other, so +/// this is the ordinary no-mint path rather than a second rule. +#[test] +fn a_spilled_file_that_is_not_a_payload_mints_nothing() { + let repo = repo("mint-spill-not-a-payload"); + let spill = common::scratch("mint-spill-garbage").join("result.txt"); + std::fs::create_dir_all(spill.parent().expect("a parent")).expect("spill dir"); + std::fs::write(&spill, "this is not json").expect("write garbage"); + completed( + &repo, + "mcp__Linear__get_issue", + &interception_notice(&spill), + ); + assert!( + receipt(&repo, "issue-read.CLOUD-1").is_none(), + "a file that carries no payload is not a read" + ); +} + +/// THE ABSOLUTENESS BOUND, pinned on every platform rather than on the one whose +/// spelling the predicate happened to carry. +/// +/// A relative path resolves against whatever directory the hook is running in, +/// which is not a thing the notice can have meant — so it recovers nothing even +/// when a file of that name is sitting right there. This case exists because the +/// bound was first written as `starts_with('/')`, which is the Unix spelling of +/// the question rather than the question: `D:\a\_temp\result.txt` failed it, so +/// the recovery was dead on Windows while the positive case above passed green on +/// Linux. Asking `Path::is_absolute` answers it on both, and this is the arm that +/// stays red if the bound is dropped altogether on either. +#[test] +fn a_notice_naming_a_relative_path_recovers_nothing() { + let repo = repo("mint-spill-relative"); + let spill = repo.join("result.txt"); + std::fs::write(&spill, READ_RESULT).expect("a real payload, in the wrong kind of place"); + completed( + &repo, + "mcp__Linear__get_issue", + &interception_notice(Path::new("result.txt")), + ); + assert!( + receipt(&repo, "issue-read.CLOUD-1").is_none(), + "a relative path names no file the notice can have meant, whatever is at it" + ); +} + +/// An ordinary unparseable string is untouched. The recovery is keyed on a host +/// naming a path in its own notice, so a tool that simply answered with prose +/// still mints nothing — this is not a blanket amnesty for failed decodes. +#[test] +fn an_unparseable_result_that_names_no_path_is_unchanged() { + let repo = repo("mint-no-path-in-notice"); + completed( + &repo, + "mcp__Linear__get_issue", + "\"Error: something went wrong and no file was written\"", + ); + assert!( + receipt(&repo, "issue-read.CLOUD-1").is_none(), + "no path, no recovery, no receipt" + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 31fe41025..fd82b548e 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -182,6 +182,7 @@ mod submodule; mod suite_subjects; mod surface; mod symbols; +mod target_consolidation; mod target_prune; mod task_prose; mod task_receipt; diff --git a/crates/batten/tests/it/mediated_verbs.rs b/crates/batten/tests/it/mediated_verbs.rs index d89d6d49a..97698e70d 100644 --- a/crates/batten/tests/it/mediated_verbs.rs +++ b/crates/batten/tests/it/mediated_verbs.rs @@ -638,6 +638,30 @@ fn an_interpreter_writing_through_its_program_text_is_a_known_gap() { assert_allowed(&format!("python3 -c \"open('{AUTHORITY}','w')\"")); } +/// THE SECOND RESIDUE, AND IT IS STRUCTURAL RATHER THAN A SCANNING LIMIT. +/// +/// The sibling above loses because the path sits inside a quoted word. This one +/// loses earlier: the path is inside a HEREDOC BODY, and `hook::segments` drops +/// bodies before any predicate exists — a body is data, not shell (CLOUD-723). +/// So the segment's words are `["python3", "-", "<<'PY'"]` and the operand list +/// contains no path at all. No operand-based check can reach it, however wide. +/// +/// CLOUD-1141's own commit names this shape as deliberately open, and its body +/// says closing it "needs the prospective content as a fact rather than a string +/// to grep, which is its own row". That row is why this is a pin and not a fix. +/// +/// Written down because it had NO case at all while its sibling had one, which +/// is the CLOUD-418 shape exactly: a suite that reads complete over a shape it +/// never exercises. Measured 2026-09-01 on `5a9924b6` — a `python3` heredoc +/// wrote a registered policy module and was not refused. +/// +/// Flips when the mediated surface gains that fact. That is the signal, and it +/// is the same one the sibling carries. +#[test] +fn an_interpreter_writing_through_a_heredoc_body_is_a_known_gap() { + assert_allowed(&format!("python3 - <<'PY'\nopen('{AUTHORITY}','w')\nPY")); +} + /// THE DIRECTION A CARELESS FIX BREAKS, and the one that decides whether this /// gate survives contact with daily use. /// diff --git a/crates/batten/tests/it/test_targets.rs b/crates/batten/tests/it/test_targets.rs index b2573bf09..97b05125f 100644 --- a/crates/batten/tests/it/test_targets.rs +++ b/crates/batten/tests/it/test_targets.rs @@ -225,3 +225,86 @@ fn every_added_target_is_named() { "both top-level additions are reported and the grouped one is not" ); } + +// --------------------------------------------------------------------------- +// The other half of the grouping, over the LIVE tree. +// +// `policy/test-targets.rego` refuses a new TOP-LEVEL target, which is what stops +// the count regrowing. It says nothing about a file that lands in the group and +// is never declared — and cargo says nothing either, because an undeclared `.rs` +// beside a target is not an error. It is simply not compiled. +// +// Measured on `5a9924b6`: `target_consolidation.rs` had no `mod` line for its +// whole life, so its three cases never ran. That file is the one asserting the +// isolation property CLOUD-1210 rests on, and its own doc says the claim "ships +// as a case rather than as a sentence in a commit message" — so the grouping's +// safety argument was a sentence after all. That is CLOUD-418's class exactly: a +// suite that reads complete over a shape it never exercises. +// +// Over the live tree deliberately, for `mediated_verbs.rs`'s reason: the question +// is what THIS repository's group declares, and a fixture would assert about a +// `main.rs` the case wrote itself. + +/// Every `.rs` beside `it/main.rs` is declared as a `mod`, and every `mod` names +/// something that resolves. +/// +/// Both directions, because one alone is satisfiable by a degenerate tree: an +/// empty group declares nothing and is missing nothing. +#[test] +fn every_grouped_test_file_is_declared_and_every_declaration_resolves() { + let group = common::at_root("crates/batten/tests/it"); + let main = fs::read_to_string(group.join("main.rs")).expect("the group harness"); + + let declared: std::collections::BTreeSet = main + .lines() + .filter_map(|line| line.trim().strip_prefix("mod ")?.strip_suffix(';')) + .map(str::to_owned) + .collect(); + + let mut present = std::collections::BTreeSet::new(); + for entry in fs::read_dir(&group).expect("the group directory") { + let path = entry.expect("a group entry").path(); + // A DIRECTORY is a module too: `mod common;` resolves to + // `it/common/mod.rs`. Counting only files would report it missing. + if path.is_dir() { + if path.join("mod.rs").is_file() { + present.insert(name_of(&path)); + } + continue; + } + if path.extension().is_some_and(|ext| ext == "rs") + && path.file_stem().is_some_and(|s| s != "main") + { + present.insert(name_of(&path)); + } + } + + let undeclared: Vec<&String> = present.difference(&declared).collect(); + assert!( + undeclared.is_empty(), + "a file in the group with no `mod` line is never compiled and its cases \ + never run — add it to `crates/batten/tests/it/main.rs`: {undeclared:?}" + ); + + let unresolved: Vec<&String> = declared.difference(&present).collect(); + assert!( + unresolved.is_empty(), + "a `mod` line naming nothing that resolves: {unresolved:?}" + ); + + // ANTI-VACUITY. Both assertions above hold over an empty group, so the sets + // have to be non-trivial for either to mean anything. + assert!( + declared.len() > 100, + "the group is the whole integration suite; a handful of modules means \ + this case is asserting over the wrong directory ({} declared)", + declared.len() + ); +} + +/// The file stem, or the directory name for a `mod.rs` module. +fn name_of(path: &Path) -> String { + path.file_stem() + .map(|stem| stem.to_string_lossy().into_owned()) + .unwrap_or_default() +} diff --git a/mise.toml b/mise.toml index 3a02b2ce8..c98813a11 100644 --- a/mise.toml +++ b/mise.toml @@ -526,7 +526,7 @@ shell = "bash -c" # itself when `INSTA_UPDATE` says to; the binary only ever added a nicer review # UI on top of that. So the accept half is the suite, run with the variable set. # -# `--test snapshots`, AND THAT BOUND IS NOT COSMETIC. This runs `cargo test`, +# `--test it snapshots::`, AND THAT BOUND IS NOT COSMETIC. This runs `cargo test`, # which runs a target's cases as THREADS IN ONE PROCESS, while this workspace's # runner is nextest, which gives each case its own. At least one suite depends on # that isolation: `document_read_count` asserts a per-process acquisition COUNT, @@ -1057,7 +1057,9 @@ run = "cargo nextest run -p batten --no-tests=fail -E 'binary(it) & test(/^confi env = { BATTEN_TEST_SCRATCH_LANE = "narrow" } [tasks."test:hook-profile"] -run = "cargo nextest run -p batten --no-tests=fail -E 'binary(hook_profile)'" +description = "Gate: the hook profile the wiring declares is the one the binary reports (CLOUD-509)" +run = "cargo nextest run -p batten --no-tests=fail -E 'binary(it) & test(/^hook_profile::/)'" +env = { BATTEN_TEST_SCRATCH_LANE = "narrow" } [tasks."test:verdict-vocabulary"] description = "Gate: every verdict vocabulary word is one token under the declared pin (CLOUD-1284 arm 4)"