diff --git a/README.md b/README.md index 02de66ec0..fe41f321a 100644 --- a/README.md +++ b/README.md @@ -168,8 +168,25 @@ the launcher's own share is attributable. | `check` | + config load, trust resolution, one-rule tree | 2.3 ms | 2.7 ms | — | | `hook` | + envelope decode, adjudication, decision write | 2.8 ms | 3.0 ms | ≤ 100 ms | | `passthrough` | a call no rule selects — decode, allow, no config load | — | — | ≤ 100 ms | +| `posttool` | a PostToolUse call — decode, capture the response | — | — | ≤ 100 ms | | `wired` | the hook as `.claude/settings.json` invokes it | 8.0 ms | 8.4 ms | ≤ 100 ms | +`posttool` is the same envelope decode plus the response capture, and its cells +are `—` for `passthrough`'s reason: the series above comes from a quieter +machine, and a figure from a noisier one published in that column reads as a +step change rather than as a different room. Measured against its own +contemporaneous `hook` on one container, 100 runs each, the capture costs +**1.25x at p50 and 1.12x at p95** — 16.3 ms and 21.1 ms against that machine's +own 13.0 ms and 18.9 ms for `hook`, so about **5x inside** the 100 ms budget on +a noisy container and further inside it on the machine the table above reports. + +`perf-compare` reads a different pairing and gets a different number, and both +are honest: it runs this arm against the MERGE BASE's binary, which performs no +capture at all, so its 1.806x prices the feature rather than a drift in the cost +of the same work. That arm carries an exemption in `perf-compare` until `main` +holds a capturing binary and the comparison is like with like; the absolute +budget is what gates it in the meantime. + 100 ms is the [Command Line Interface Guidelines'][clig] floor for a response that reads as instant. It is an absolute ceiling rather than a tight band around the measured value: a shared runner's p95 moves by more than a percentage band diff --git a/completions/batten.bash b/completions/batten.bash index 91111d9e2..c00881a0f 100644 --- a/completions/batten.bash +++ b/completions/batten.bash @@ -820,7 +820,7 @@ _batten() { return 0 ;; batten__subcmd__capture__subcmd__list) - opts="-J -q -v -y -h --stream --json --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" + opts="-J -q -v -y -h --stream --calls --json --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 @@ -876,7 +876,7 @@ _batten() { return 0 ;; batten__subcmd__capture__subcmd__show) - opts="-J -q -v -y -h --lines --grep --json --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" + opts="-J -q -v -y -h --lines --grep --raw --bytes --json --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 @@ -890,6 +890,10 @@ _batten() { COMPREPLY=($(compgen -f "${cur}")) return 0 ;; + --bytes) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; --strictness) COMPREPLY=($(compgen -W "permissive standard strict" -- "${cur}")) return 0 diff --git a/completions/batten.fish b/completions/batten.fish index 125a25faa..f72cfbfb5 100644 --- a/completions/batten.fish +++ b/completions/batten.fish @@ -170,6 +170,7 @@ complete -c batten -n "__fish_batten_using_subcommand capture; and not __fish_se complete -c batten -n "__fish_batten_using_subcommand capture; and not __fish_seen_subcommand_from show list prune help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l lines -d 'A 1-indexed inclusive line range, `FROM:TO`, clamped to the capture' -r complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l grep -d 'Only lines containing this literal substring' -r +complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l bytes -d 'A 0-indexed half-open byte range, `FROM:TO`, either side omittable, clamped to the capture' -r complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' standard\t'The default: a finding is a violation' strict\t'Everything `Standard` fails on, plus anything advisory'" @@ -180,6 +181,7 @@ normal\t'The default' verbose\t'Explain what is being checked' debug\t'Add resolution detail' trace\t'Add everything'" +complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l raw -d 'Write the selected bytes to stdout verbatim, with no decode and no added newline' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -s J -l json -d 'Emit byte-stable JSON instead of pointer lines' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l fail-on-warning -d 'Promote a warn-severity finding to a violation (an override may only turn this on)' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from show" -l silent -d 'Say nothing but a verdict or a usage error' @@ -202,6 +204,7 @@ normal\t'The default' verbose\t'Explain what is being checked' debug\t'Add resolution detail' trace\t'Add everything'" +complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from list" -l calls -d 'List recorded calls instead of stored captures, in a byte-stable order' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from list" -s J -l json -d 'Emit byte-stable JSON instead of pointer lines' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from list" -l fail-on-warning -d 'Promote a warn-severity finding to a violation (an override may only turn this on)' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from list" -l silent -d 'Say nothing but a verdict or a usage error' diff --git a/completions/batten.zsh b/completions/batten.zsh index 837b67616..8a7be55aa 100644 --- a/completions/batten.zsh +++ b/completions/batten.zsh @@ -193,6 +193,7 @@ trace\:"Add everything"))' \ _arguments "${_arguments_options[@]}" : \ '--lines=[A 1-indexed inclusive line range, \`FROM\:TO\`, clamped to the capture]: :_default' \ '--grep=[Only lines containing this literal substring]: :_default' \ +'--bytes=[A 0-indexed half-open byte range, \`FROM\:TO\`, either side omittable, clamped to the capture]: :_default' \ '--strictness=[Raise how strictly gates apply (an override may only tighten policy)]: :((permissive\:"Advisory\: findings are reported without failing the run" standard\:"The default\: a finding is a violation" strict\:"Everything \`Standard\` fails on, plus anything advisory"))' \ @@ -203,6 +204,7 @@ normal\:"The default" verbose\:"Explain what is being checked" debug\:"Add resolution detail" trace\:"Add everything"))' \ +'--raw[Write the selected bytes to stdout verbatim, with no decode and no added newline]' \ '-J[Emit byte-stable JSON instead of pointer lines]' \ '--json[Emit byte-stable JSON instead of pointer lines]' \ '--fail-on-warning[Promote a warn-severity finding to a violation (an override may only turn this on)]' \ @@ -235,6 +237,7 @@ normal\:"The default" verbose\:"Explain what is being checked" debug\:"Add resolution detail" trace\:"Add everything"))' \ +'--calls[List recorded calls instead of stored captures, in a byte-stable order]' \ '-J[Emit byte-stable JSON instead of pointer lines]' \ '--json[Emit byte-stable JSON instead of pointer lines]' \ '--fail-on-warning[Promote a warn-severity finding to a violation (an override may only turn this on)]' \ diff --git a/crates/batten/src/capture.rs b/crates/batten/src/capture.rs index f0919e0f5..8c5efc4e6 100644 --- a/crates/batten/src/capture.rs +++ b/crates/batten/src/capture.rs @@ -55,12 +55,29 @@ pub enum Stream { Stdout, /// The child's standard error. Stderr, + /// A tool response the harness handed over (CLOUD-918). + /// + /// **Appended, never inserted.** `semver` reads a reordered variant as + /// `enum_no_repr_variant_discriminant_changed`, so declaration order is an + /// API fact here exactly as it is for [`crate::hook::Capability`]. + /// + /// Not a child's stream at all, and it rides this enum anyway because + /// everything downstream of the store key is identical: the handle shape, + /// [`Handle::parse`], [`list`], [`prune`] and the `-` + /// filename need no new case. A second addressing scheme for the same + /// question is how two stores come to disagree. + /// + /// **Sealed-only**, and that is enforced by [`LiveStream`] rather than + /// asserted: a response arrives whole, so there is nothing to spool and a + /// live handle would promise a file that is still growing when nothing is + /// writing it. + Response, } impl Stream { - /// Both streams, so anything ranging over them is derived rather than typed + /// Every stream, so anything ranging over them is derived rather than typed /// twice. - pub const ALL: &'static [Stream] = &[Stream::Stdout, Stream::Stderr]; + pub const ALL: &'static [Stream] = &[Stream::Stdout, Stream::Stderr, Stream::Response]; /// The stable token used in the store key and in the hashed preimage. #[must_use] @@ -68,8 +85,334 @@ impl Stream { match self { Stream::Stdout => "stdout", Stream::Stderr => "stderr", + Stream::Response => "response", + } + } +} + +/// A stream that can spool: the only thing a live handle can name. +/// +/// **Makes sealed-only unrepresentable rather than merely untested.** +/// [`live_handle`] and [`Spool::open`] take this instead of a [`Stream`], so +/// there is no code path that mints a live handle for [`Stream::Response`] and +/// no test standing guard over one. The exhaustive `match` in +/// [`LiveStream::new`] also means a seventh stream cannot land without deciding +/// which side it is on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LiveStream(Stream); + +impl LiveStream { + /// A child's standard output, which spools. + /// + /// A const rather than [`LiveStream::new`] plus a handled `None`, because at + /// every production call site the stream is statically one of these two and + /// the crate's lints forbid unwrapping a total answer into a reachable panic. + pub const STDOUT: LiveStream = LiveStream(Stream::Stdout); + /// A child's standard error, which spools. + pub const STDERR: LiveStream = LiveStream(Stream::Stderr); + + /// `None` for a stream that is sealed-only. + /// + /// A child's pipes grow while it runs and can be read mid-flight; a tool + /// response is handed over complete, so a watermark would have nothing to + /// name. + #[must_use] + pub const fn new(stream: Stream) -> Option { + match stream { + Stream::Stdout | Stream::Stderr => Some(LiveStream(stream)), + Stream::Response => None, } } + + /// The stream this names. + #[must_use] + pub const fn stream(self) -> Stream { + self.0 + } + + /// Whether this is the error stream. + /// + /// A boolean rather than leaving callers to `match` the inner [`Stream`]: + /// there are exactly two spooling streams, so a caller choosing between two + /// terminal sinks has a total answer here and no third arm to write for a + /// case [`LiveStream`] already excludes. + #[must_use] + pub const fn is_stderr(self) -> bool { + matches!(self.0, Stream::Stderr) + } +} + +/// How faithfully the bytes a capture holds relate to the bytes the host framed +/// (CLOUD-917). +/// +/// Five values, mutually exclusive, and every declared cell carries exactly one. +/// The column this ranges over is [`crate::hook::CaptureCapabilities`], per host +/// and per response shape — a capture that does not say how faithful it is makes +/// every reader guess, and the guesses differ. +/// +/// # `byte-perfect` names [`Fidelity::LexicalBytes`] and [`Fidelity::SpillFile`] +/// +/// Exactly two of these five may be described that way, and +/// [`Fidelity::is_byte_perfect`] is the only authority on which — no doc comment, +/// output line, record field or test name may say it of any other. +/// `tests/capture_fidelity.rs` scans this module's own docs and this type's +/// rendered output for the term and refuses a mention beside a value that does +/// not answer `true` there. +/// +/// The value that makes the rule necessary is [`Fidelity::DecodedContent`]: +/// re-serializing a decoded JSON value normalizes key order, escaping and +/// whitespace, so what comes back out is a different byte string from what +/// arrived. It is exact for the member it decoded and it is not a reproduction +/// of the document, and those are two claims rather than one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum Fidelity { + /// The original bytes of the response member, exactly as the host framed + /// them. A faithful reproduction of the document that arrived. + LexicalBytes, + /// The decoded content bytes, with the framing recorded separately. + /// + /// Exact for the member it decoded, and **not** a reproduction of the + /// document the host framed: a decode-then-reserialize round trip + /// renormalizes key order, escaping and whitespace, so the bytes that come + /// back out are a different string from the ones that arrived. The framing — + /// block count, per-block type — lives on the provenance row rather than + /// interleaved into the bytes, so a reader of the stream gets content and a + /// reader of the record gets structure. + DecodedContent, + /// The bytes of the file the host spilled the response into. A faithful + /// reproduction: the file is read once and stored as it was read. + SpillFile, + /// A leading prefix, plus whatever is known about the whole. + /// + /// **Not** a reproduction of the document, and it never claims to be: this + /// is the value that says so out loud, which is the whole reason a partial + /// capture does not borrow a completeness claim it cannot support. + Prefix { + /// How many bytes were captured. + captured: u64, + /// The total the host declared, when it declared one. `None` is a + /// truncation signal with no total, never a total of zero. + declared: Option, + }, + /// Nothing; the host does not make the bytes reachable here. + /// + /// The honest value for an unsurveyed host, and never a guess. A host that + /// cannot be captured is *knowable* rather than silent, which is the whole + /// difference between this and having no column at all. + Unavailable, +} + +impl Fidelity { + /// Every fidelity arm, so a census is derived rather than hand-kept. + /// + /// [`Fidelity::Prefix`] carries a payload, so it appears here under one + /// representative value. The census ranges over **arms**: the payload is a + /// fact about one capture and is not part of the vocabulary a host declares. + pub const ALL: &'static [Fidelity] = &[ + Fidelity::LexicalBytes, + Fidelity::DecodedContent, + Fidelity::SpillFile, + Fidelity::Prefix { + captured: 0, + declared: None, + }, + Fidelity::Unavailable, + ]; + + /// The stable token, for byte-stable output (§6). + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Fidelity::LexicalBytes => "lexical-bytes", + Fidelity::DecodedContent => "decoded-content", + Fidelity::SpillFile => "spill-file", + Fidelity::Prefix { .. } => "prefix", + Fidelity::Unavailable => "unavailable", + } + } + + /// Whether this value may be described as byte-perfect: true for + /// [`Fidelity::LexicalBytes`] and [`Fidelity::SpillFile`], false for the + /// other three. + /// + /// **The one authority**, so the claim cannot be made in prose by a site + /// that never consulted it. Only a capture holding the bytes as the host + /// framed them qualifies: the original member + /// ([`Fidelity::LexicalBytes`]) or the spilled file + /// ([`Fidelity::SpillFile`]). A decoded member is exact for what it decoded + /// and is not this; a prefix does not claim completeness at all. + #[must_use] + pub const fn is_byte_perfect(self) -> bool { + matches!(self, Fidelity::LexicalBytes | Fidelity::SpillFile) + } + + /// The rendered one-line description, for a `doctor` row or a listing. + /// + /// Carries the reserved term for — and only for — the two values + /// [`Fidelity::is_byte_perfect`] admits, so deleting the claim from the + /// rendering while leaving it in the type is a red rather than a drift. + #[must_use] + pub const fn note(self) -> &'static str { + match self { + Fidelity::LexicalBytes => "the response member as the host framed it; byte-perfect", + Fidelity::SpillFile => "the file the host spilled the response into; byte-perfect", + Fidelity::DecodedContent => { + "the decoded content, exact for the member and not the framed document" + } + Fidelity::Prefix { .. } => "a leading prefix; completeness is not claimed", + Fidelity::Unavailable => "the host does not make the bytes reachable here", + } + } +} + +/// The state root could not be resolved, so there is no store to write into. +/// +/// Distinct from [`STORE_UNWRITABLE`] on purpose: two different remedies — fix +/// the repository, or fix the directory — so one id would send the reader to the +/// wrong place. That is `doctor.rs`'s pattern for a reason id, and these inherit +/// its `a_reason_id_never_carries_a_path` gate. +pub const STATE_ROOT_UNRESOLVED: &str = "capture-state-root-unresolved"; + +/// The store resolved and a record could not be written into it. +pub const STORE_UNWRITABLE: &str = "capture-store-unwritable"; + +/// The response arrived in a shape no decoder here recognises. +/// +/// **Could-not-look, never zero bytes**, which is `facts::rows_in`'s own +/// precedent: an unread shape is not an empty response, and recording it as one +/// would make "the tool said nothing" and "we could not read what it said" the +/// same record. +pub const RESPONSE_SHAPE_UNREADABLE: &str = "capture-response-shape-unreadable"; + +/// The write-time budget refused the bytes. +pub const BUDGET_EXHAUSTED: &str = "capture-budget-exhausted"; + +/// The file a host spilled a response into was gone when it was opened. +pub const SPILL_VANISHED: &str = "capture-spill-vanished"; + +/// A spilled file's length moved between the open and the end of the read. +/// +/// **Never a [`Fidelity::Prefix`]**, which would imply the host truncated: a +/// racing writer is a fidelity answer about our read, not a claim about what the +/// host meant to send. +pub const SPILL_RACED: &str = "capture-spill-raced"; + +/// Every reason id this module can produce, so a census is derived. +pub const REASONS: &[&str] = &[ + STATE_ROOT_UNRESOLVED, + STORE_UNWRITABLE, + RESPONSE_SHAPE_UNREADABLE, + BUDGET_EXHAUSTED, + SPILL_VANISHED, + SPILL_RACED, +]; + +/// What a response member decoded to, plus the framing kept out of the bytes. +/// +/// The framing — how many blocks, and of what type — lives here rather than +/// interleaved into [`Decoded::bytes`], which is what makes the bytes replayable: +/// a reader of the stream gets content, and a reader of the provenance row gets +/// structure. Interleaving would make `--raw` return something no host ever +/// sent. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Decoded { + /// The decoded content bytes, in the order the host sent them. + pub bytes: Vec, + /// How many blocks were concatenated. One for a bare string or an object. + pub blocks: usize, + /// The fidelity this decode achieved. + pub fidelity: Fidelity, +} + +/// The response member's content bytes, or why they could not be read. +/// +/// **Always [`Fidelity::DecodedContent`] today, and that is a measurement rather +/// than a choice.** [`crate::hook::decode`] hands the engine an already-parsed +/// `serde_json::Value`, so the member's original lexical bytes are gone before +/// anything here sees it. Reaching [`Fidelity::LexicalBytes`] needs two things +/// that do not exist: `serde_json`'s non-default `raw_value` feature, and a +/// decoder that carries the member's raw span beside the parsed value. +/// +/// **Re-serializing is not a substitute and is forbidden.** `to_vec` over the +/// parsed value renormalizes key order, escaping, numbers and whitespace, so the +/// result is a different byte string from what arrived — and describing that as a +/// faithful reproduction of the document is precisely the claim [`Fidelity`]'s +/// reserved word exists to refuse. So this decodes CONTENT and says so. +/// +/// Three shapes are recognised, which are the three the corpus carries: +/// +/// * an array of `{"type":"text","text":…}` blocks — the MCP content-block shape; +/// each block's decoded text, concatenated in host order. +/// * an object with string members — Claude Code's Bash shape; `stdout` then +/// `stderr`, in that fixed order so the bytes are a function of the value +/// rather than of map iteration. +/// * a bare string. +/// +/// Anything else is [`RESPONSE_SHAPE_UNREADABLE`] rather than zero bytes. +/// +/// # Errors +/// +/// Returns [`RESPONSE_SHAPE_UNREADABLE`] when the member is in none of the three +/// shapes above. **Could-not-look, never zero bytes** — recording an unread shape +/// as an empty response would make "the tool said nothing" and "we could not read +/// what it said" one record, which is `facts::rows_in`'s own distinction. +pub fn decode_response(result: &serde_json::Value) -> Result { + match result { + serde_json::Value::String(text) => Ok(Decoded { + bytes: text.as_bytes().to_vec(), + blocks: 1, + fidelity: Fidelity::DecodedContent, + }), + serde_json::Value::Array(items) => { + let mut bytes = Vec::new(); + let mut blocks = 0; + for item in items { + // A block whose `text` is a string contributes it; anything else + // in the array is framing this decoder does not claim to read. + if let Some(text) = item.get("text").and_then(serde_json::Value::as_str) { + bytes.extend_from_slice(text.as_bytes()); + blocks += 1; + } + } + // An EMPTY array is a real response of zero bytes — `[]` is a host + // saying "nothing", which is not the same as a shape we cannot read. + if blocks == 0 && !items.is_empty() { + return Err(RESPONSE_SHAPE_UNREADABLE); + } + Ok(Decoded { + bytes, + blocks, + fidelity: Fidelity::DecodedContent, + }) + } + serde_json::Value::Object(map) => { + // A FIXED ORDER, declared here rather than taken from the map: two + // runs over one value must produce one digest (§6), and iteration + // order is not a promise a caller can rely on. + let mut bytes = Vec::new(); + let mut blocks = 0; + for key in ["stdout", "stderr"] { + if let Some(text) = map.get(key).and_then(serde_json::Value::as_str) { + bytes.extend_from_slice(text.as_bytes()); + blocks += 1; + } + } + // An empty object is zero bytes, for the empty array's reason. An + // object with members but none we read is a shape, not an absence. + if blocks == 0 && !map.is_empty() { + return Err(RESPONSE_SHAPE_UNREADABLE); + } + Ok(Decoded { + bytes, + blocks, + fidelity: Fidelity::DecodedContent, + }) + } + // `Null` never reaches here: the caller gates on the member being + // present, because absent and empty are two records. + _ => Err(RESPONSE_SHAPE_UNREADABLE), + } } /// A pointer to captured bytes: which stream, how many bytes, and their digest. @@ -104,12 +447,68 @@ fn captures_dir(repo_root: &Path) -> Result { Ok(state::repo_state_dir(repo_root)?.join("captures")) } +/// The capture store's mode: owner only (CLOUD-918). +/// +/// Following [`crate::secrets`]'s `KEY_DIR_MODE`, and a **change from previous +/// behaviour**: the store used to be created with a bare `create_dir_all` and +/// inherited the umask. That was defensible while a capture held only the output +/// of a command the operator themself ran; it is not once the same store can hold +/// a tool response, which is the likeliest thing in an envelope to carry a +/// secret. +/// +/// A store written by an earlier binary is **not** retroactively tightened — +/// stated rather than left to be discovered, because the mode is set when a +/// directory is created and nothing here walks an existing one to fix it. +#[cfg(unix)] +const STORE_DIR_MODE: u32 = 0o700; + +/// A stored capture's mode: owner read and write. +#[cfg(unix)] +const STORE_FILE_MODE: u32 = 0o600; + +/// Create the capture store, owner-only where the platform enforces it. +/// +/// `create_dir_all` then `set_permissions`, which is [`crate::secrets`]'s +/// `create_dir_private` idiom and acceptable for its reason: `create_dir_all` +/// takes no mode, and the window between the two holds an EMPTY directory. On +/// Windows the claim is unenforced, the same per-platform arm the state root +/// already carries. +/// +/// **Only a store THIS call creates gets its mode set**, which is what makes +/// [`STORE_DIR_MODE`]'s compatibility note true rather than aspirational. Every +/// [`store`] goes through here, so chmod-ing unconditionally would silently +/// tighten a directory an operator may have widened on purpose — and would +/// contradict the promise that an existing store is not retroactively changed. +/// Caught in review on the commit that introduced it. +fn create_store_dir(dir: &Path) -> Result<()> { + let fresh = !dir.exists(); + std::fs::create_dir_all(dir) + .with_context(|| format!("create the capture store {}", dir.display()))?; + #[cfg(unix)] + if fresh { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(dir, std::fs::Permissions::from_mode(STORE_DIR_MODE)) + .with_context(|| format!("restrict the capture store {}", dir.display()))?; + } + // Read only under `unix`, where the mode exists to be set at all. + #[cfg(not(unix))] + let _ = fresh; + Ok(()) +} + /// Distinguishes one staging file from another within a process. /// /// See [`store`]: the pid alone collides when two threads of one `batten` write /// the same content-addressed record at the same moment. static STAGING_ATTEMPT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +/// How many staging names [`store`] tries before calling it a storage failure. +/// +/// The counter restarts per process, so a stale `.tmp` from an interrupted run +/// can collide after pid reuse. A handful of names is plenty to step past one; +/// needing more means the directory itself is the problem. +const STAGING_ATTEMPTS: u32 = 8; + /// Store `bytes` as `stream`'s capture for the repository at `repo_root`. /// /// Content-addressed and therefore idempotent: storing the same bytes twice @@ -123,10 +522,20 @@ static STAGING_ATTEMPT: std::sync::atomic::AtomicU64 = std::sync::atomic::Atomic /// nobody checked. That is the same posture the receipt store takes, and the /// reason both are internal errors rather than fail-open allowances. pub fn store(repo_root: &Path, stream: Stream, bytes: &[u8]) -> Result { + store_in(&captures_dir(repo_root)?, stream, bytes) +} + +/// [`store`] into a store directory named outright — [`Spool::open_in`]'s seam, +/// for its reason: resolving the state root reads the OS data directory, which a +/// unit test must not write into. +/// +/// # Errors +/// +/// As [`store`]. +pub fn store_in(dir: &Path, stream: Stream, bytes: &[u8]) -> Result { let digest = identity::capture_fingerprint(stream.as_str(), bytes).to_hex(); - let dir = captures_dir(repo_root)?; - std::fs::create_dir_all(&dir) - .with_context(|| format!("create the capture store {}", dir.display()))?; + let dir = dir.to_path_buf(); + create_store_dir(&dir)?; // The digest already carries the stream, so the filename does not need to — // but it is included so a human listing the directory can tell what they are @@ -150,14 +559,61 @@ pub fn store(repo_root: &Path, stream: Stream, bytes: &[u8]) -> Result // bundle whose commands printed the same thing, or two library callers at // once — would stage to one path, and the second `rename` would find nothing // there. Measured, as `No such file or directory` on the publish. - let attempt = STAGING_ATTEMPT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let staging = dir.join(format!( - "{}-{digest}.{}.{attempt}.tmp", - stream.as_str(), - std::process::id() - )); - let mut file = std::fs::File::create(&staging) - .with_context(|| format!("write the capture {}", staging.display()))?; + // MODE AT CREATION, never a chmod afterwards (`secrets.rs`'s `write_private` + // idiom, and its reason): a file created world-readable and tightened after + // the write is world-readable for exactly the window in which the bytes are + // in it. + // + // WHICH FORCES `create_new`, AND `create_new` FORCES THIS RETRY. Setting a + // mode at creation means `OpenOptions`, and an `OpenOptions` that truncated + // an existing file would reintroduce the torn-record window the temp-and- + // rename exists to close. But the staging name is only unique WITHIN a + // process: the counter restarts at zero every run, so a stale `.tmp` left by + // an interrupted earlier run collides after pid reuse, and `create_new` then + // fails a capture that has nothing wrong with it. Minting another attempt is + // the fix — the loop is bounded, because a directory that refuses every name + // is a storage failure rather than a collision. Caught in review; the + // predecessor used `File::create`, which truncated the stale file instead. + let mut file = None; + let mut staging = PathBuf::new(); + let mut last: Option = None; + for _ in 0..STAGING_ATTEMPTS { + let attempt = STAGING_ATTEMPT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + staging = dir.join(format!( + "{}-{digest}.{}.{attempt}.tmp", + stream.as_str(), + std::process::id() + )); + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(STORE_FILE_MODE); + } + match options.open(&staging) { + Ok(opened) => { + file = Some(opened); + break; + } + // The one error a different name can fix. + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => last = Some(err), + Err(err) => { + return Err(anyhow::Error::from(err)) + .with_context(|| format!("write the capture {}", staging.display())); + } + } + } + let Some(mut file) = file else { + let err = last.unwrap_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + "every staging name was taken", + ) + }); + return Err(anyhow::Error::from(err)) + .with_context(|| format!("write the capture {}", staging.display())); + }; file.write_all(bytes) .with_context(|| format!("write the capture {}", staging.display()))?; file.sync_all() @@ -189,9 +645,11 @@ fn live_dir(repo_root: &Path) -> Result { /// bundle, both of which the caller already knows — it spawned the process. That /// is deliberate: printing the handle would put a pid in Batten's output, and §6 /// byte-stability forbids a field that differs between two identical runs. +/// Takes a [`LiveStream`] rather than a [`Stream`]: [`Stream::Response`] is +/// sealed-only, and the type is what says so. #[must_use] -pub fn live_handle(stream: Stream, key: &str) -> String { - format!("{}@{key}", stream.as_str()) +pub fn live_handle(stream: LiveStream, key: &str) -> String { + format!("{}@{key}", stream.stream().as_str()) } /// What a reader saw when it asked a live capture for bytes. @@ -271,7 +729,7 @@ impl Spool { /// /// Returns an error when the state root cannot be resolved or the spool /// cannot be created. - pub fn open(repo_root: &Path, stream: Stream, key: &str) -> Result { + pub fn open(repo_root: &Path, stream: LiveStream, key: &str) -> Result { Self::open_in(&live_dir(repo_root)?, stream, key) } @@ -285,7 +743,7 @@ impl Spool { /// # Errors /// /// Returns an error when the spool cannot be created. - pub fn open_in(dir: &Path, stream: Stream, key: &str) -> Result { + pub fn open_in(dir: &Path, stream: LiveStream, key: &str) -> Result { let dir = dir.to_path_buf(); std::fs::create_dir_all(&dir) .with_context(|| format!("create the live capture directory {}", dir.display()))?; @@ -407,7 +865,7 @@ impl Spool { /// already-sealed handle is [`LiveRead::Absent`] — neither is an error. pub fn read_live( repo_root: &Path, - stream: Stream, + stream: LiveStream, key: &str, from: u64, limit: usize, @@ -422,7 +880,7 @@ pub fn read_live( /// As [`read_live`]. pub fn read_live_in( dir: &Path, - stream: Stream, + stream: LiveStream, key: &str, from: u64, limit: usize, @@ -480,13 +938,13 @@ pub fn read_live_in( /// # Errors /// /// Returns an error when the state root cannot be resolved. -pub fn live_watermark(repo_root: &Path, stream: Stream, key: &str) -> Result> { +pub fn live_watermark(repo_root: &Path, stream: LiveStream, key: &str) -> Result> { Ok(live_watermark_in(&live_dir(repo_root)?, stream, key)) } /// [`live_watermark`] in a directory named outright — [`Spool::open_in`]'s seam. #[must_use] -pub fn live_watermark_in(dir: &Path, stream: Stream, key: &str) -> Option { +pub fn live_watermark_in(dir: &Path, stream: LiveStream, key: &str) -> Option { let path = dir.join(format!("{}.watermark", live_handle(stream, key))); std::fs::read_to_string(&path) .ok() @@ -600,6 +1058,23 @@ pub enum Selection { /// expression. `forbid` took the regex decision narrowly (CLOUD-283) and this /// is one of the places it was deliberately not taken. Grep { needle: String }, + /// A **0-indexed, half-open** byte range, clamped to the capture + /// (CLOUD-918). + /// + /// `from` inclusive, `to` exclusive, both optional; an absent `from` is the + /// start and an absent `to` is the end. Clamped at both ends for + /// [`Selection::Lines`]'s reason, and an inverted range selects nothing + /// rather than panicking. + /// + /// **The asymmetry with [`Selection::Lines`] is deliberate rather than + /// sloppy.** Lines are 1-indexed and inclusive because a human reads a line + /// number off a rendering that starts at 1, and `grep`'s output feeds + /// straight back in. Bytes are 0-indexed and half-open because byte ranges + /// *tile*: `0:N` then `N:M` covers the capture exactly once, with no + /// off-by-one at the seam, and an absent `to` is the only way to say "to the + /// end" without first learning the length. Collapsing the two conventions + /// would make one of them wrong. + Raw { from: Option, to: Option }, } /// A capture's lines, numbered, as selected. @@ -630,6 +1105,74 @@ pub struct Selected { /// The selected lines. Empty for [`Selection::Summary`], and empty is also a /// real answer for a `--grep` that matched nothing. pub selected: Vec, + /// Whether decoding the capture replaced anything (CLOUD-918). + /// + /// **A property of the whole capture, not of the selection**, and the doc has + /// to say so or the field lies: the decode is of the whole capture, so + /// `--lines 1:2` of a log whose byte 4000 is invalid reports `true` even + /// though every selected line came back clean. What it answers is "is this + /// view a faithful rendering of the stored bytes", which is a question about + /// the record. + /// + /// Without it a caller cannot tell a capture that decoded cleanly from one + /// that did not, which is the difference between the line view being a + /// convenience and being a trap. `--raw` is the operation that gets the bytes + /// themselves. + pub lossy: bool, +} + +/// The answer to one `capture show --raw`: bytes, and the pointer naming them. +/// +/// Separate from [`Selected`] because the two carry different things — one holds +/// decoded lines and the other holds bytes — and a single type with both would +/// invite a caller to read whichever field happened to be populated. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RawSelected { + /// The handle these bytes came from. + pub handle: String, + /// The capture's total size in bytes. + pub bytes: u64, + /// The clamped, resolved start offset, inclusive. + pub from: u64, + /// The clamped, resolved end offset, exclusive. + pub to: u64, + /// The selected bytes, verbatim. Never decoded. + pub data: Vec, +} + +/// Select a byte range of `bytes`, verbatim. +/// +/// **Deliberately not an arm of [`select`].** That function opens with +/// `String::from_utf8_lossy` and every arm reads the decoded view, so a raw arm +/// inside it would sit one refactor away from being decoded too. A separate +/// function is what makes "raw never decodes" a structural property rather than a +/// convention — there is no decoded value in this scope to accidentally read. +#[must_use] +pub fn select_raw( + handle: &Handle, + bytes: &[u8], + from: Option, + to: Option, +) -> RawSelected { + let len = bytes.len(); + // Clamped at both ends, exactly as `Selection::Lines` is. `try_from` + // saturating to `usize::MAX` then `min(len)` means a 64-bit offset on a + // 32-bit target clamps to the end rather than wrapping. + let start = usize::try_from(from.unwrap_or(0)) + .unwrap_or(usize::MAX) + .min(len); + let end = usize::try_from(to.unwrap_or(len as u64)) + .unwrap_or(usize::MAX) + .min(len); + // An inverted range selects nothing rather than panicking on the slice. + let data = bytes.get(start..end).unwrap_or(&[]).to_vec(); + RawSelected { + handle: handle.to_string(), + bytes: len as u64, + from: start as u64, + to: end.max(start) as u64, + data, + } } /// Apply `selection` to `bytes`. @@ -646,13 +1189,18 @@ pub struct Selected { #[must_use] pub fn select(handle: &Handle, bytes: &[u8], selection: &Selection) -> Selected { let decoded = String::from_utf8_lossy(bytes); + // `from_utf8_lossy` returns `Borrowed` if and only if the whole input was + // valid UTF-8, and `Owned` if and only if it inserted at least one + // replacement character. That is documented behaviour rather than an + // allocation optimisation, so the discriminant is a faithful answer to "did + // the decode replace anything" and costs no second pass. + let lossy = matches!(decoded, std::borrow::Cow::Owned(_)); let all: Vec<&str> = decoded.lines().collect(); let numbered = |index: usize, text: &str| Line { number: index + 1, text: text.to_owned(), }; let selected = match selection { - Selection::Summary => Vec::new(), Selection::Lines { from, to } => { // Clamped at both ends, and an inverted range selects nothing rather // than panicking on the slice — `5:2` is a caller error that costs an @@ -672,12 +1220,19 @@ pub fn select(handle: &Handle, bytes: &[u8], selection: &Selection) -> Selected .filter(|(_, text)| text.contains(needle.as_str())) .map(|(index, text)| numbered(index, text)) .collect(), + // Neither selects a line, for two different reasons that happen to have + // the same answer: `Summary` is the pointer by definition, and a byte + // range is not a line view at all — `select_raw` answers that one, and + // returning the pointer here keeps the two functions from producing two + // different answers for one request. + Selection::Summary | Selection::Raw { .. } => Vec::new(), }; Selected { handle: handle.to_string(), bytes: bytes.len() as u64, lines: all.len(), selected, + lossy, } } @@ -696,7 +1251,16 @@ pub fn select(handle: &Handle, bytes: &[u8], selection: &Selection) -> Selected /// Returns an error when the state root cannot be resolved, or when the store /// exists and cannot be read. pub fn list(repo_root: &Path) -> Result> { - let dir = captures_dir(repo_root)?; + list_in(&captures_dir(repo_root)?) +} + +/// [`list`] over a store directory named outright — [`store_in`]'s seam. +/// +/// # Errors +/// +/// As [`list`]. +pub fn list_in(dir: &Path) -> Result> { + let dir = dir.to_path_buf(); if !dir.exists() { return Ok(Vec::new()); } @@ -729,6 +1293,445 @@ pub fn list(repo_root: &Path) -> Result> { Ok(found) } +/// One call's capture outcome — the **invocation** identity (CLOUD-917). +/// +/// Two identities, and deduplication may not erase either. The blob is keyed by +/// CONTENT, so identical bytes are one record and the record carries no +/// timestamp; this row is keyed by CALL, so forty calls that produced the same +/// bytes are one blob and forty rows. A session that ran one command repeatedly +/// can still say which call was which, and the timestamp a response genuinely +/// needs lives here — where it is a fact about an invocation — rather than on the +/// blob, where it would break §6 byte-stability. +/// +/// **Pointer-only** (non-negotiable rule 4): a digest **or** a reason id, never +/// bytes. [`CallRow::class`] is a path *class* rather than a path, because where +/// a host spilled a response is disk layout and differs per machine. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct CallRow { + /// Monotone within [`CallRow::session`], from zero. **Not a clock**: it is + /// the count of rows already recorded for this session, read under the same + /// lock the append takes, so two writers cannot mint one ordinal. + pub order: u64, + /// The host's own session id, so ordering is scoped to a conversation. + pub session: String, + /// Which surface the bytes came from — a fixed token, never a path. + pub source: String, + /// The harness, as [`crate::hook::Harness::as_str`] spells it. + pub host: String, + /// The tool the host named. + pub tool: String, + /// The event, as [`crate::hook::Event::as_str`] spells it. + pub event: String, + /// The fidelity, as [`Fidelity::as_str`] spells it. + pub fidelity: String, + /// When the call was seen, RFC3339. + /// + /// **This is the timestamp the blob refuses to carry**, and it is legitimate + /// here for the reason the blob's absence is legitimate there: a capture keyed + /// by content must be a pure function of that content, while a row keyed by + /// invocation is a fact about a moment. It is therefore **never rendered** by + /// `capture list --calls` — a listing that printed it would stop being + /// byte-stable across runs (§6), which is the property the ordering rule + /// below exists to protect. + #[serde(skip_serializing_if = "Option::is_none")] + pub seen_at: Option, + /// For a spilled response, the CLASS of path it came from. Never a path. + #[serde(skip_serializing_if = "Option::is_none")] + pub class: Option, + /// The content digest. Exactly one of this and [`CallRow::absent`] is set. + #[serde(skip_serializing_if = "Option::is_none")] + pub digest: Option, + /// A recorded absence: one of this module's reason ids. + /// + /// **The key that distinguishes the two three-valued outcomes.** A row with + /// a digest and a row with an absence differ in which keys EXIST, not in a + /// count — so "the tool returned nothing" and "nobody looked" cannot collapse + /// into one record, which is CLOUD-251's vacuous pass on a new surface. + #[serde(skip_serializing_if = "Option::is_none")] + pub absent: Option, +} + +/// The bound on response captures (CLOUD-918). +/// +/// Enforced **at write time**, and the trigger is the write — never a clock, +/// never a schedule, never a background sweeper. That is +/// `.claude/rules/toolchain.md`'s split between a gate and a schedule, and +/// [`prune`]'s own doc already refuses a time-based sweeper on the same grounds. +#[derive( + Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize, schemars::JsonSchema, +)] +#[serde(deny_unknown_fields)] +pub struct CaptureConfig { + /// Total response-capture bytes the store may hold. Absent means the default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_bytes: Option, + /// Response-capture records the store may hold. Absent means the default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_records: Option, +} + +/// The default byte bound on response captures. +/// +/// **Bounded by default, unlike `exec` captures**, because a response arrives per +/// mediated call rather than per distinct command output. +pub const DEFAULT_RESPONSE_MAX_BYTES: u64 = 8 * 1024 * 1024; + +/// The default record bound on response captures. +pub const DEFAULT_RESPONSE_MAX_RECORDS: u64 = 1024; + +/// Evict oldest response captures until the store is inside `config`. +/// +/// **Oldest by the call log's recorded order**, never by mtime: the log is the +/// only thing that knows which call came first, and an mtime order would make +/// eviction a function of when the filesystem happened to touch a file. +/// +/// Only [`Stream::Response`] records are candidates. A `stdout` or `stderr` +/// capture is `exec`'s and is never evicted here, which is what keeps today's +/// behaviour byte-identical for that consumer. +/// +/// # Errors +/// +/// Returns an error when the store cannot be read or a record cannot be removed. +pub fn evict_to_budget(repo_root: &Path, config: Option<&CaptureConfig>) -> Result { + evict_to_budget_in(&captures_dir(repo_root)?, config) +} + +/// [`evict_to_budget`] over a store directory named outright — [`store_in`]'s +/// seam. +/// +/// # Errors +/// +/// As [`evict_to_budget`]. +pub fn evict_to_budget_in(dir: &Path, config: Option<&CaptureConfig>) -> Result { + let max_bytes = config + .and_then(|held| held.max_bytes) + .unwrap_or(DEFAULT_RESPONSE_MAX_BYTES); + let max_records = config + .and_then(|held| held.max_records) + .unwrap_or(DEFAULT_RESPONSE_MAX_RECORDS); + // FIRST, because the blob budget below can be satisfied while the log is + // not: see `bound_calls`. This is the only trim, and it is unconditional. + bound_calls(dir, max_records)?; + let held: Vec = list_in(dir)? + .into_iter() + .filter(|record| record.stream == Stream::Response.as_str()) + .collect(); + let mut total: u64 = held.iter().map(|record| record.bytes).sum(); + let mut count = held.len() as u64; + if total <= max_bytes && count <= max_records { + return Ok(0); + } + // THE LOG'S APPEND ORDER, never `calls_in`'s sorted view. That view is sorted + // by `(session, order)` for §6 byte-stability, and `order` is monotone only + // WITHIN a session — so across two sessions the sorted view is + // session-lexicographic, and consuming it would evict every row of the + // alphabetically-first session before any row of a later-sorting one. A + // store holding an older `zzz` and a newer `aaa` would then evict its newest + // responses first. Append order is the only cross-session order the log + // actually carries. A digest named by no row cannot be ordered at all, so it + // is evicted last rather than first — guessing an order for it would be + // inventing provenance. + let ordered = read_calls(&dir.join("calls")); + // A digest-keyed lookup and a taken-set, rather than a scan inside a scan. + // The row count is not bounded by the record count — one row per call, many + // calls per record — so the nested form is quadratic in the log's length, on + // a path that runs per tool call. Same order, linear cost. + let by_digest: std::collections::HashMap<&str, &Capture> = held + .iter() + .map(|record| (record.digest.as_str(), record)) + .collect(); + let mut taken: std::collections::HashSet<&str> = std::collections::HashSet::new(); + let mut candidates: Vec = Vec::new(); + for row in &ordered { + let Some(digest) = row.digest.as_deref() else { + continue; + }; + if let Some(found) = by_digest.get(digest) + && taken.insert(found.digest.as_str()) + { + candidates.push((*found).clone()); + } + } + for record in &held { + if !taken.contains(record.digest.as_str()) { + candidates.push(record.clone()); + } + } + let mut removed = 0; + for record in candidates { + if total <= max_bytes && count <= max_records { + break; + } + let path = dir.join(format!("{}-{}", record.stream, record.digest)); + if std::fs::remove_file(&path).is_ok() { + total = total.saturating_sub(record.bytes); + count = count.saturating_sub(1); + removed += 1; + } + } + Ok(removed) +} + +/// How many times [`record_call`] retries a held lock before giving up. +/// +/// Bounded rather than unbounded: this runs on the mediated path, where a stuck +/// holder must not block a tool call. +const CALL_LOCK_ATTEMPTS: u32 = 50; + +/// How long [`record_call`] waits between attempts. +const CALL_LOCK_BACKOFF: std::time::Duration = std::time::Duration::from_millis(2); + +/// Append one call row, minting its per-session order under the lock. +/// +/// JSONL, one object per line, and byte-stable because `serde_json` emits a +/// struct's fields in declaration order. +/// +/// **A lost lock RETRIES rather than skipping**, which is the opposite of +/// [`Spool::commit`]'s watermark publish and deliberately so. There, a skipped +/// publish costs a reader one stale length and the next publish carries both. +/// Here, a skipped row is a call nobody recorded — precisely what this log +/// exists to prevent — so contention waits instead. +/// +/// # Errors +/// +/// Returns an error when the state root cannot be resolved or the log cannot be +/// appended to. The mediated caller does not propagate it: on that surface a +/// storage failure is recorded and reported, never raised. +pub fn record_call(repo_root: &Path, row: &CallRow) -> Result<()> { + record_call_in(&captures_dir(repo_root)?, row) +} + +/// [`record_call`] into a store directory named outright — [`store_in`]'s seam. +/// +/// # Errors +/// +/// As [`record_call`]. +pub fn record_call_in(dir: &Path, row: &CallRow) -> Result<()> { + let dir = dir.to_path_buf(); + create_store_dir(&dir)?; + let path = dir.join("calls"); + let Some(lock) = take_call_lock(&dir)? else { + anyhow::bail!( + "the call log lock in {} stayed held; the row was not recorded", + dir.display() + ); + }; + let order = next_order(&path, &row.session); + let mut minted = row.clone(); + minted.order = order; + let line = serde_json::to_string(&minted).context("render a call row")?; + let mut options = std::fs::OpenOptions::new(); + options.append(true).create(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(STORE_FILE_MODE); + } + let appended = options + .open(&path) + .and_then(|mut file| writeln!(file, "{line}")); + drop(lock); + appended.with_context(|| format!("append to the call log {}", path.display()))?; + Ok(()) +} + +/// The call log's exclusive lock, or `None` when the bound was exhausted. +/// +/// **One lock, two writers**, which is what makes it worth a function rather +/// than a block: the append in [`record_call_in`] and the whole-file rewrite in +/// [`trim_calls`] contend on the same bytes, and a rewrite that renames a new +/// inode over the old one while an append holds a descriptor on the old one +/// loses that appended row outright — the row this log exists to keep. +/// +/// `try_lock` is the same call `Spool::commit` makes; the difference is what +/// each does with a refusal — there it returns, here it waits. Bounded, because +/// an unbounded wait on the mediated path would let a stuck holder block a tool +/// call, and no Batten failure may do that. `None` is the exhausted bound, and +/// the two callers read it differently: an unrecorded row is a failure, an +/// untrimmed log is not. +/// +/// # Errors +/// +/// Returns an error when the lock file cannot be opened, or when the lock call +/// itself fails for a reason other than contention. +fn take_call_lock(dir: &Path) -> Result> { + let lock_path = dir.join("calls.lock"); + let lock = std::fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&lock_path) + .with_context(|| format!("open the call log lock {}", lock_path.display()))?; + for _ in 0..CALL_LOCK_ATTEMPTS { + match fs4::FileExt::try_lock(&lock) { + Ok(()) => return Ok(Some(lock)), + Err(fs4::TryLockError::WouldBlock) => { + std::thread::sleep(CALL_LOCK_BACKOFF); + } + Err(fs4::TryLockError::Error(err)) => { + return Err(anyhow::Error::from(err)) + .with_context(|| format!("take the call log lock {}", lock_path.display())); + } + } + } + Ok(None) +} + +/// The smallest number of bytes a rendered [`CallRow`] can occupy. +/// +/// A floor, deliberately far below the real minimum, because it is only ever +/// used to decide whether the log is SMALL ENOUGH to skip parsing: a floor that +/// is too low costs an unnecessary parse, while one that is too high skips a +/// trim that was due. `a_rendered_row_never_falls_under_the_row_floor` is the +/// gate on the direction that matters. +const MIN_CALL_ROW_BYTES: u64 = 64; + +/// Hold the log inside its bound, ahead of the blob budget. +/// +/// **Ahead of it, and that ordering is the whole function.** Response blobs are +/// content-addressed, so a session repeating one response leaves the byte and +/// record counts flat while the log gains a row per call — a store permanently +/// inside its blob budget whose log grows forever. Trimming after the budget +/// check therefore never ran in exactly the state the bound exists for, and +/// [`next_order`]'s cost argument rests on that bound being real. +/// +/// **One `stat` on the common path.** The log is read and parsed only when its +/// SIZE could hold more rows than the bound allows, which is what keeps this +/// affordable on the mediated post-tool path: the alternative is a full parse +/// per tool call, which is the cost shape [`next_order`] exists to avoid. +/// +/// # Errors +/// +/// Returns an error when the log cannot be read, rewritten, or published. +fn bound_calls(dir: &Path, max_records: u64) -> Result<()> { + let allowed = max_records.saturating_mul(4); + let path = dir.join("calls"); + let Ok(metadata) = std::fs::metadata(&path) else { + // No log is not a failure to read one: nothing has been recorded yet. + return Ok(()); + }; + if metadata.len() <= allowed.saturating_mul(MIN_CALL_ROW_BYTES) { + return Ok(()); + } + let Some(lock) = take_call_lock(dir)? else { + // An untrimmed log is not a failure — the next call tries again, and the + // holder that refused this lock is itself a writer. Failing here would + // turn contention into a reported storage error. + return Ok(()); + }; + // Append order here too, and for a sharper reason than the eviction walk's: + // `trim_calls` keeps a SUFFIX, so a session-lexicographic order would keep + // the alphabetically-last session's rows and drop everything recent from + // every other one. + let ordered = read_calls(&dir.join("calls")); + let trimmed = trim_calls(dir, &ordered, max_records); + drop(lock); + trimmed +} + +/// Drop the oldest rows so the log stays inside `max_records`. +/// +/// **Bounded by the same number the blobs are**, rather than a second knob: a +/// row and the record it names are one fact about one call, so keeping rows for +/// records that have been evicted would leave the log describing a store that no +/// longer exists. Rewritten whole under the caller's lock, because a partial +/// rewrite of an append-only log is worse than an oversized one. +/// +/// A generous multiple of the record bound, not the bound itself: an ABSENCE row +/// names no record, and those are exactly the rows a reader most wants to keep — +/// "no record" must not come to mean "no calls". +fn trim_calls(dir: &Path, ordered: &[CallRow], max_records: u64) -> Result<()> { + let allowed = usize::try_from(max_records.saturating_mul(4)).unwrap_or(usize::MAX); + if ordered.len() <= allowed { + return Ok(()); + } + let keep = &ordered[ordered.len() - allowed..]; + let mut rendered = String::new(); + for row in keep { + let line = serde_json::to_string(row).context("render a call row")?; + rendered.push_str(&line); + rendered.push('\n'); + } + let path = dir.join("calls"); + let staging = dir.join("calls.trim"); + std::fs::write(&staging, rendered) + .with_context(|| format!("stage the trimmed call log {}", staging.display()))?; + std::fs::rename(&staging, &path) + .with_context(|| format!("publish the trimmed call log {}", path.display()))?; + Ok(()) +} + +/// The next ordinal for `session`, read from the tail of the log. +/// +/// **Backwards, and that is a cost decision rather than a style one.** This runs +/// on the mediated post-tool path — the hottest path in the binary — and the +/// obvious implementation parses every row written so far to count the ones +/// matching this session, which makes the per-call cost grow with the session's +/// own history. That is precisely the shape CLOUD-851 measured: a store +/// acquisition that regressed `check` 2.103x with every test still green, +/// because none of them measured invocation cost. Caught in review, before the +/// bench arm that would have measured it landed. +/// +/// Scanning from the end stops at the newest row for this session, which in the +/// common case — a session appending to its own recent rows — is the last line. +/// A session with no rows yet walks the log once and answers zero; the bound on +/// THAT is the log trimming in [`evict_to_budget_in`]. +fn next_order(path: &Path, session: &str) -> u64 { + let Ok(text) = std::fs::read_to_string(path) else { + return 0; + }; + for line in text.lines().rev() { + if let Ok(row) = serde_json::from_str::(line) + && row.session == session + { + return row.order.saturating_add(1); + } + } + 0 +} + +/// Read the log, skipping any line that does not parse. +/// +/// A torn or hand-edited line is skipped rather than reported, for [`list`]'s +/// reason: a row nothing wrote is not evidence about a call, and inventing one +/// from it would be a fabricated provenance record. +fn read_calls(path: &Path) -> Vec { + let Ok(text) = std::fs::read_to_string(path) else { + return Vec::new(); + }; + text.lines() + .filter_map(|line| serde_json::from_str::(line).ok()) + .collect() +} + +/// Every recorded call, in a fixed order. +/// +/// Sorted by `(session, order)` — never by mtime and never by `seen_at`, for +/// [`list`]'s §6 reason: an ordering that is a function of when things happened +/// makes two runs over an unchanged log disagree. +/// +/// # Errors +/// +/// Returns an error when the state root cannot be resolved. +pub fn calls(repo_root: &Path) -> Result> { + calls_in(&captures_dir(repo_root)?) +} + +/// [`calls`] over a store directory named outright — [`store_in`]'s seam. +/// +/// # Errors +/// +/// As [`calls`]. +pub fn calls_in(dir: &Path) -> Result> { + let mut rows = read_calls(&dir.join("calls")); + rows.sort_by(|left, right| { + left.session + .cmp(&right.session) + .then(left.order.cmp(&right.order)) + }); + Ok(rows) +} + /// Remove every capture in the repository's store, returning how many went. /// /// **The whole lifecycle, and that is the design.** A capture is content-addressed @@ -775,6 +1778,411 @@ mod tests { } } + // `#[cfg(unix)]`, because the constants it reads exist only there — an + // ungated test does not compile on Windows, which `cross-check` covers. + #[cfg(unix)] + #[test] + fn the_store_mode_is_decided_by_a_constant_rather_than_by_the_umask() { + // THE EXTRACTED DECISION, which is what `.claude/rules/rust.md` asks for + // where the environment cannot produce the failing condition: this sandbox + // runs as root, so permission bits never bite and a test that tried to + // assert enforcement would assert its own premise. What is checkable is + // the value the code decided on, and that it is owner-only. + assert_eq!(STORE_DIR_MODE, 0o700); + assert_eq!(STORE_FILE_MODE, 0o600); + // Beside `secrets.rs`, whose modes these follow: a capture store that can + // hold a tool response is at least as sensitive as a key store. + assert_eq!( + STORE_DIR_MODE & 0o077, + 0, + "the store is group/other-readable" + ); + assert_eq!( + STORE_FILE_MODE & 0o077, + 0, + "a record is group/other-readable" + ); + } + + #[cfg(unix)] + #[test] + fn a_stored_capture_and_its_store_are_created_owner_only() { + // The bits we SET, read back off the filesystem — not enforcement, which + // root would defeat. This is the half a constant assertion cannot cover: + // that the decision actually reaches the two `create` calls. + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join(format!("batten-store-mode-{}", std::process::id())); + drop(std::fs::remove_dir_all(&dir)); + let store = dir.join("captures"); + create_store_dir(&store).unwrap(); + assert_eq!( + std::fs::metadata(&store).unwrap().permissions().mode() & 0o777, + STORE_DIR_MODE + ); + // The record's mode is set at creation rather than chmod'ed after, so the + // bytes are never briefly world-readable. Exercised through the same + // options the store builds. + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(STORE_FILE_MODE); + } + let record = store.join("response-deadbeef"); + drop(options.open(&record).unwrap()); + assert_eq!( + std::fs::metadata(&record).unwrap().permissions().mode() & 0o777, + STORE_FILE_MODE + ); + drop(std::fs::remove_dir_all(&dir)); + } + + /// A store directory under this process's own scratch space. + /// + /// `*_in` rather than the repo-rooted entry points, for `scratch_live`'s + /// reason: resolving the state root reads the OS data directory, and a unit + /// test must not write into a developer's. + fn scratch_store(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("batten-calls-{}-{name}", std::process::id())); + drop(std::fs::remove_dir_all(&dir)); + dir + } + + /// A call row with the fields a test does not care about filled in. + fn row(session: &str, digest: Option<&str>, absent: Option<&str>) -> CallRow { + CallRow { + order: 0, + session: session.to_owned(), + source: "response".to_owned(), + host: "claude-code".to_owned(), + tool: "Bash".to_owned(), + event: "post-tool".to_owned(), + fidelity: Fidelity::DecodedContent.as_str().to_owned(), + seen_at: None, + class: None, + digest: digest.map(str::to_owned), + absent: absent.map(str::to_owned), + } + } + + #[test] + fn a_recorded_absence_is_a_reason_id_rather_than_a_missing_row() { + // CLOUD-251's collapse, refused at the record shape: "the tool returned + // nothing" and "nobody looked" differ in which KEYS EXIST, not in a + // count, so no reader can conflate them by comparing numbers. + let captured = serde_json::to_string(&row("s", Some("beef"), None)).unwrap(); + let absent = + serde_json::to_string(&row("s", None, Some("capture-store-unwritable"))).unwrap(); + assert!(captured.contains("\"digest\"")); + assert!(!captured.contains("\"absent\"")); + assert!(absent.contains("\"absent\"")); + assert!(!absent.contains("\"digest\"")); + assert_ne!(captured, absent); + } + + #[test] + fn a_call_row_never_carries_a_path_or_a_byte() { + // Rule 4 at the record. The spill source is a path CLASS, so even the + // field that describes where bytes came from cannot name a directory. + let mut carried = row("s", Some("beef"), None); + carried.class = Some("host-temp".to_owned()); + let rendered = serde_json::to_string(&carried).unwrap(); + assert!( + !rendered.contains('/'), + "a row looks like a path: {rendered}" + ); + assert!(!rendered.contains("bytes")); + } + + #[test] + fn a_call_order_is_monotone_within_a_session_and_scoped_to_it() { + // Minted from the count of rows already recorded for THIS session, under + // the lock the append takes — never from a clock, which is what keeps two + // runs over an unchanged log in agreement (§6). + let root = scratch_store("call-order"); + for _ in 0..3 { + record_call_in(&root, &row("alpha", Some("aa"), None)).unwrap(); + } + record_call_in(&root, &row("beta", Some("bb"), None)).unwrap(); + let recorded = calls_in(&root).unwrap(); + let alpha: Vec = recorded + .iter() + .filter(|held| held.session == "alpha") + .map(|held| held.order) + .collect(); + assert_eq!(alpha, vec![0, 1, 2]); + // A second session starts at zero rather than continuing the first: the + // ordinal answers "which call within this conversation". + let beta: Vec = recorded + .iter() + .filter(|held| held.session == "beta") + .map(|held| held.order) + .collect(); + assert_eq!(beta, vec![0]); + } + + #[test] + fn identical_bytes_are_one_blob_and_two_rows() { + // The two identities, as the property that makes provenance worth having. + // Dedup collapses content and must not collapse invocations. + let root = scratch_store("two-identities"); + let first = store_in(&root, Stream::Response, b"same").unwrap(); + let second = store_in(&root, Stream::Response, b"same").unwrap(); + assert_eq!(first.digest, second.digest); + record_call_in(&root, &row("s", Some(&first.digest), None)).unwrap(); + record_call_in(&root, &row("s", Some(&second.digest), None)).unwrap(); + let blobs: Vec = list_in(&root) + .unwrap() + .into_iter() + .filter(|held| held.stream == Stream::Response.as_str()) + .collect(); + assert_eq!(blobs.len(), 1, "identical bytes became two records"); + assert_eq!( + calls_in(&root).unwrap().len(), + 2, + "two calls became one row" + ); + } + + #[test] + fn the_call_view_is_byte_stable_across_runs() { + // §6. Reading twice over an unchanged log must agree, which is what + // forbids an mtime ordering and forbids rendering `seen_at`. + let root = scratch_store("call-stable"); + let mut stamped = row("s", Some("cc"), None); + stamped.seen_at = Some("2026-08-23T00:00:00Z".to_owned()); + record_call_in(&root, &stamped).unwrap(); + record_call_in(&root, &row("s", Some("dd"), None)).unwrap(); + assert_eq!(calls_in(&root).unwrap(), calls_in(&root).unwrap()); + } + + #[test] + fn an_exec_capture_is_unbounded_because_todays_behaviour_is_unchanged() { + // The bound is for RESPONSES, whose growth law is per call. `exec` + // captures grow per DISTINCT output and `prune` is their whole lifecycle, + // so a bound computed for the other denominator must not reach them. + let root = scratch_store("exec-unbounded"); + for index in 0..4_u8 { + store_in(&root, Stream::Stdout, &[index]).unwrap(); + } + let tight = CaptureConfig { + max_bytes: Some(1), + max_records: Some(1), + }; + assert_eq!(evict_to_budget_in(&root, Some(&tight)).unwrap(), 0); + assert_eq!( + list_in(&root).unwrap().len(), + 4, + "an exec capture was evicted" + ); + } + + #[test] + fn eviction_takes_the_oldest_recorded_call_rather_than_the_newest() { + // The call log's order is the authority on "oldest", never an mtime. An + // inverted walk here would drop the capture a caller most likely still + // wants, which is why the direction is pinned rather than assumed. + let root = scratch_store("evict-oldest"); + let old = store_in(&root, Stream::Response, b"oldest").unwrap(); + let mid = store_in(&root, Stream::Response, b"middle").unwrap(); + let new = store_in(&root, Stream::Response, b"newest").unwrap(); + for digest in [&old.digest, &mid.digest, &new.digest] { + record_call_in(&root, &row("s", Some(digest), None)).unwrap(); + } + let two = CaptureConfig { + max_bytes: None, + max_records: Some(2), + }; + assert_eq!(evict_to_budget_in(&root, Some(&two)).unwrap(), 1); + let left: Vec = list_in(&root) + .unwrap() + .into_iter() + .filter(|held| held.stream == Stream::Response.as_str()) + .map(|held| held.digest) + .collect(); + assert!(!left.contains(&old.digest), "the newest was evicted"); + assert!(left.contains(&new.digest)); + } + + #[test] + fn an_absent_capture_table_means_the_engine_default_rather_than_unbounded() { + // Absent is the DEFAULT, not "no bound": response capture is bounded by + // default because it grows per call, and reading an absent table as + // unbounded would inherit `exec`'s posture into the surface the bound + // exists for. + // The defaults are consts, so asserting they are positive is a + // tautology clippy rightly refuses. What is checkable is the BEHAVIOUR + // an absent table produces: bounded by the default, not unbounded. + let root = scratch_store("evict-default"); + store_in(&root, Stream::Response, b"small").unwrap(); + // Well inside the default, so nothing goes... + assert_eq!(evict_to_budget_in(&root, None).unwrap(), 0); + // ...and the default is a real bound rather than "no bound": the same + // store under a table of one record evicts, which an unbounded reading + // could never do. + let one = CaptureConfig { + max_bytes: None, + max_records: Some(0), + }; + assert_eq!(evict_to_budget_in(&root, Some(&one)).unwrap(), 1); + } + + #[test] + fn eviction_crosses_sessions_by_append_order_rather_than_by_session_name() { + // `order` is monotone only WITHIN a session, so the sorted listing view is + // session-lexicographic across two of them. Consuming that view evicts by + // NAME: here the older session sorts last, so a sorted walk would take the + // newest response first — the exact inversion the oldest-first contract + // forbids, and one a single-session test cannot see. + // + // Red on the defect: put `calls_in(dir)?` back in `evict_to_budget_in` and + // the newest capture is the one that goes. + let root = scratch_store("evict-cross-session"); + let old = store_in(&root, Stream::Response, b"older").unwrap(); + let new = store_in(&root, Stream::Response, b"newer").unwrap(); + // The OLDER call belongs to the session whose id sorts LAST. + record_call_in(&root, &row("zzz", Some(&old.digest), None)).unwrap(); + record_call_in(&root, &row("aaa", Some(&new.digest), None)).unwrap(); + let one = CaptureConfig { + max_bytes: None, + max_records: Some(1), + }; + assert_eq!(evict_to_budget_in(&root, Some(&one)).unwrap(), 1); + let left: Vec = list_in(&root) + .unwrap() + .into_iter() + .filter(|held| held.stream == Stream::Response.as_str()) + .map(|held| held.digest) + .collect(); + assert!( + left.contains(&new.digest), + "the newest capture was evicted because its session name sorted first" + ); + assert!(!left.contains(&old.digest)); + } + + #[test] + fn a_deduplicated_store_still_bounds_its_call_log() { + // THE STATE THE BOUND EXISTS FOR, and the one a trim behind the blob + // budget never reached. Blobs are content-addressed, so a session + // repeating one response holds the byte and record counts flat while the + // log gains a row per call: the budget check is satisfied forever and the + // log grows forever with it. `next_order` walks backwards from the tail, + // so an unbounded log is an unbounded scan on the mediated path. + // + // Red on the defect: move `bound_calls` behind the `total <= max_bytes` + // return and the log keeps every row. + let root = scratch_store("bound-dedup"); + let stored = store_in(&root, Stream::Response, b"same").unwrap(); + let one = CaptureConfig { + max_bytes: None, + max_records: Some(1), + }; + // Well past `max_records * 4` rows, all naming the one deduplicated blob. + for _ in 0..12 { + record_call_in(&root, &row("s", Some(&stored.digest), None)).unwrap(); + evict_to_budget_in(&root, Some(&one)).unwrap(); + } + let rows = calls_in(&root).unwrap(); + assert!( + rows.len() <= 4, + "the log kept {} rows against a bound of 4", + rows.len() + ); + // The bound keeps the NEWEST rows: a trim that kept the head would make + // the log describe a prefix of the session and nothing since. + assert!( + rows.last().is_some_and(|last| last.order >= 8), + "the trim kept the head rather than the tail" + ); + } + + #[test] + fn a_rendered_row_never_falls_under_the_row_floor() { + // `bound_calls` skips the parse when the log is too SMALL to hold more + // rows than the bound allows, which is only sound in one direction: a + // floor above the real minimum skips a trim that was due. The minimal row + // — every optional key absent — is the case that would break it. + let minimal = CallRow { + order: 0, + session: String::new(), + source: String::new(), + host: String::new(), + tool: String::new(), + event: String::new(), + fidelity: String::new(), + seen_at: None, + class: None, + digest: None, + absent: None, + }; + let rendered = serde_json::to_string(&minimal).unwrap(); + assert!( + rendered.len() as u64 >= MIN_CALL_ROW_BYTES, + "the minimal row renders in {} bytes, under the {MIN_CALL_ROW_BYTES}-byte floor", + rendered.len() + ); + } + + #[test] + fn every_fidelity_token_is_distinct() { + // The tokens reach a byte-stable record (§6), where two values sharing + // one name would make the record ambiguous rather than merely ugly. + let mut tokens: Vec<&str> = Fidelity::ALL + .iter() + .map(|fidelity| fidelity.as_str()) + .collect(); + tokens.sort_unstable(); + let count = tokens.len(); + tokens.dedup(); + assert_eq!(tokens.len(), count, "two fidelity values share a token"); + } + + #[test] + fn exactly_two_fidelity_values_may_be_called_byte_perfect() { + // The reserved word, pinned at the type. `is_byte_perfect` is the one + // authority every other site consults, so widening it is a deliberate + // edit that reds here and in `tests/capture_fidelity.rs` rather than a + // claim that spreads through prose. + let admitted: Vec<&str> = Fidelity::ALL + .iter() + .filter(|fidelity| fidelity.is_byte_perfect()) + .map(|fidelity| fidelity.as_str()) + .collect(); + assert_eq!(admitted, vec!["lexical-bytes", "spill-file"]); + } + + #[test] + fn a_prefix_fidelity_never_claims_a_declared_length_it_did_not_measure() { + // Three-valued, and the middle value is the one that would otherwise + // collapse: a truncation signal with NO declared total is not a total of + // zero, and reading it as one would let a partial capture answer "you + // have all zero bytes of it". + let no_total = Fidelity::Prefix { + captured: 12, + declared: None, + }; + let zero_total = Fidelity::Prefix { + captured: 12, + declared: Some(0), + }; + assert_ne!(no_total, zero_total); + // And neither claims completeness, which only the two admitted values do. + assert!(!no_total.is_byte_perfect()); + assert!(!zero_total.is_byte_perfect()); + } + + #[test] + fn a_decoded_capture_is_exact_for_its_member_and_claims_nothing_wider() { + // The distinction CLOUD-917 reserves the word for. `DecodedContent` is + // byte-exact for what it decoded and is not a reproduction of the + // document the host framed, because a reserialize renormalizes key + // order, escaping and whitespace. + assert!(!Fidelity::DecodedContent.is_byte_perfect()); + assert!(Fidelity::LexicalBytes.is_byte_perfect()); + assert_ne!(Fidelity::DecodedContent, Fidelity::LexicalBytes); + } + /// A live-capture directory under this process's own scratch space. fn scratch_live(name: &str) -> PathBuf { let dir = std::env::temp_dir().join(format!("batten-spool-{}-{name}", std::process::id())); @@ -787,8 +2195,8 @@ mod tests { // One separator per promise: `:` names bytes that will never change, // `@` names a file that is still growing. A reader handed the wrong one // must be unable to treat it as the other. - assert_eq!(live_handle(Stream::Stdout, "42.0"), "stdout@42.0"); - assert!(!live_handle(Stream::Stdout, "42.0").contains(':')); + assert_eq!(live_handle(LiveStream::STDOUT, "42.0"), "stdout@42.0"); + assert!(!live_handle(LiveStream::STDOUT, "42.0").contains(':')); } #[test] @@ -798,7 +2206,7 @@ mod tests { // publish lost the lock. The bytes land, the length does not, and a // reader must see the old length rather than the new bytes. let dir = scratch_live("watermark"); - let mut spool = Spool::open_in(&dir, Stream::Stdout, "unit").unwrap(); + let mut spool = Spool::open_in(&dir, LiveStream::STDOUT, "unit").unwrap(); spool.commit(b"first").unwrap(); let held = std::fs::OpenOptions::new() @@ -815,7 +2223,7 @@ mod tests { // rather than an error — losing a race to an honest writer says nothing // about the capture, and the next read gets it. assert_eq!( - read_live_in(&dir, Stream::Stdout, "unit", 0, 4096).unwrap(), + read_live_in(&dir, LiveStream::STDOUT, "unit", 0, 4096).unwrap(), LiveRead::Busy ); @@ -828,7 +2236,8 @@ mod tests { ); drop(held); - let LiveRead::Bytes(seen) = read_live_in(&dir, Stream::Stdout, "unit", 0, 4096).unwrap() + let LiveRead::Bytes(seen) = + read_live_in(&dir, LiveStream::STDOUT, "unit", 0, 4096).unwrap() else { panic!("the lock is free again"); }; @@ -839,7 +2248,8 @@ mod tests { // IDEMPOTENT: the same range re-read is the same bytes, which is what // makes "more context" a repeatable question rather than a stream to // parse. - let LiveRead::Bytes(again) = read_live_in(&dir, Stream::Stdout, "unit", 0, 4096).unwrap() + let LiveRead::Bytes(again) = + read_live_in(&dir, LiveStream::STDOUT, "unit", 0, 4096).unwrap() else { panic!("the lock is free again"); }; @@ -847,13 +2257,13 @@ mod tests { // And the next publish carries both chunks, so nothing is lost by lagging. spool.commit(b"-third").unwrap(); - let LiveRead::Bytes(all) = read_live_in(&dir, Stream::Stdout, "unit", 0, 4096).unwrap() + let LiveRead::Bytes(all) = read_live_in(&dir, LiveStream::STDOUT, "unit", 0, 4096).unwrap() else { panic!("the lock is free again"); }; assert_eq!(all, b"first-second-third"); assert_eq!( - live_watermark_in(&dir, Stream::Stdout, "unit"), + live_watermark_in(&dir, LiveStream::STDOUT, "unit"), Some(all.len() as u64) ); } @@ -863,10 +2273,10 @@ mod tests { let dir = scratch_live("absent"); std::fs::create_dir_all(&dir).unwrap(); assert_eq!( - read_live_in(&dir, Stream::Stdout, "never", 0, 16).unwrap(), + read_live_in(&dir, LiveStream::STDOUT, "never", 0, 16).unwrap(), LiveRead::Absent ); - assert_eq!(live_watermark_in(&dir, Stream::Stdout, "never"), None); + assert_eq!(live_watermark_in(&dir, LiveStream::STDOUT, "never"), None); } #[test] @@ -876,11 +2286,12 @@ mod tests { // instead of waiting. If that handle truncated, a timed-out drain would // seal an empty capture over a run that produced output. let dir = scratch_live("reopen"); - let mut spool = Spool::open_in(&dir, Stream::Stdout, "unit").unwrap(); + let mut spool = Spool::open_in(&dir, LiveStream::STDOUT, "unit").unwrap(); spool.commit(b"kept").unwrap(); let mut second = spool.reopen().unwrap(); second.commit(b"-more").unwrap(); - let LiveRead::Bytes(seen) = read_live_in(&dir, Stream::Stdout, "unit", 0, 4096).unwrap() + let LiveRead::Bytes(seen) = + read_live_in(&dir, LiveStream::STDOUT, "unit", 0, 4096).unwrap() else { panic!("nothing else holds this lock"); }; diff --git a/crates/batten/src/cli.rs b/crates/batten/src/cli.rs index 1e7cc5c2e..822aeb9ee 100644 --- a/crates/batten/src/cli.rs +++ b/crates/batten/src/cli.rs @@ -352,6 +352,10 @@ pub enum CaptureCommand { lines: Option, /// A case-sensitive literal substring; only lines containing it. grep: Option, + /// Write the selected bytes to stdout verbatim, with no decode. + raw: bool, + /// A 0-indexed half-open `FROM:TO` byte range, clamped to the capture. + bytes: Option, /// Emit the selection as byte-stable JSON instead of pointer lines. json: bool, }, @@ -359,6 +363,8 @@ pub enum CaptureCommand { List { /// Only captures of this stream. stream: Option, + /// List recorded calls instead of stored captures. + calls: bool, /// Emit the listing as byte-stable JSON instead of pointer lines. json: bool, }, @@ -791,10 +797,13 @@ fn capture_of(matches: &ArgMatches) -> Option { handle: matches.get_one::("handle").cloned()?, lines: matches.get_one::("lines").cloned(), grep: matches.get_one::("grep").cloned(), + raw: flag(matches, "raw"), + bytes: matches.get_one::("bytes").cloned(), json: flag(matches, "json"), }), ("list", matches) => Some(CaptureCommand::List { stream: matches.get_one::("stream").cloned(), + calls: flag(matches, "calls"), json: flag(matches, "json"), }), ("prune", matches) => Some(CaptureCommand::Prune { diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index d544a03ba..bca7faeda 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -228,6 +228,20 @@ pub struct Config { /// declared. The type and the predicate are [`crate::exec`]. #[serde(default, skip_serializing_if = "Option::is_none")] pub exec: Option, + /// The bound on RESPONSE captures (CLOUD-918). Absent means the engine + /// defaults. + /// + /// **`exec` captures stay unbounded and unchanged**, so the consumer that has + /// this behaviour today keeps it: `prune` remains the whole lifecycle there, + /// and that store is bounded by how many *distinct* outputs a repository + /// produces, because identical bytes are one record. A bound exists for + /// responses because response capture changes the growth law — per call rather + /// than per distinct output — and inheriting `exec`'s posture into that would + /// be adopting a bound computed for a different denominator. + /// + /// The type and the eviction are [`crate::capture`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub capture: Option, /// The suppression markers to count (CLOUD-36). Which comment shape waves /// a rule through is a property of the repository being gated, never of /// Batten; the type and the counting are [`crate::markers`]. @@ -837,6 +851,7 @@ impl Config { facts: Vec::new(), markers: Vec::new(), exec: None, + capture: None, exec_patterns: Vec::new(), waivers: Vec::new(), // An authority that declares no budget grants no exemption from one diff --git a/crates/batten/src/exec.rs b/crates/batten/src/exec.rs index f8490d1b0..fb0113dff 100644 --- a/crates/batten/src/exec.rs +++ b/crates/batten/src/exec.rs @@ -659,7 +659,12 @@ impl Drain { /// so "nobody sees this" is a way of writing, not an absence of one. struct ChildSink { /// Which of Batten's own streams this copies to. - stream: Stream, + /// + /// A [`capture::LiveStream`], not a [`Stream`]: a tee copies a child's pipe + /// to a terminal, and `Stream::Response` names bytes no child wrote. The + /// narrower type is what keeps the two sinks below a total choice rather + /// than a match with an unreachable third arm. + stream: capture::LiveStream, /// The per-line prefix, when the style asks for one. prefix: Option, /// Whether the next byte starts a line, so a prefix lands in the right place @@ -671,7 +676,7 @@ struct ChildSink { impl ChildSink { /// The sink `stream` gets under `settings`, for a child named `program`. - fn of(settings: &ExecConfig, program: &str, stream: Stream) -> Self { + fn of(settings: &ExecConfig, program: &str, stream: capture::LiveStream) -> Self { let style = settings.style; let showing = settings.tee && !style.suppresses_child(); let prefix = match style.style_only() { @@ -712,9 +717,10 @@ impl Write for ChildSink { // already accounted for. return Ok(buf.len()); } - match self.stream { - Stream::Stdout => self.dress(&mut std::io::stdout(), buf)?, - Stream::Stderr => self.dress(&mut std::io::stderr(), buf)?, + if self.stream.is_stderr() { + self.dress(&mut std::io::stderr(), buf)?; + } else { + self.dress(&mut std::io::stdout(), buf)?; } Ok(buf.len()) } @@ -723,9 +729,10 @@ impl Write for ChildSink { if self.discard { return Ok(()); } - match self.stream { - Stream::Stdout => std::io::stdout().flush(), - Stream::Stderr => std::io::stderr().flush(), + if self.stream.is_stderr() { + std::io::stderr().flush() + } else { + std::io::stdout().flush() } } } @@ -1372,13 +1379,13 @@ fn run_one( // deadlocks as soon as a child fills the one not being read. let out_drain = Drain::spawn( out_pipe, - ChildSink::of(settings, program, Stream::Stdout), - capture::Spool::open(repo_root, Stream::Stdout, &key)?, + ChildSink::of(settings, program, capture::LiveStream::STDOUT), + capture::Spool::open(repo_root, capture::LiveStream::STDOUT, &key)?, ); let err_drain = Drain::spawn( err_pipe, - ChildSink::of(settings, program, Stream::Stderr), - capture::Spool::open(repo_root, Stream::Stderr, &key)?, + ChildSink::of(settings, program, capture::LiveStream::STDERR), + capture::Spool::open(repo_root, capture::LiveStream::STDERR, &key)?, ); // Forwarding is installed only for a group Batten owns, so an invocation @@ -1681,7 +1688,7 @@ mod tests { // The tee loop writes what it read and expects the count back. A sink // that reported a short write because nobody was listening would spin on // bytes the capture already has. - let mut sink = ChildSink::of(&ExecConfig::DEFAULT, "child", Stream::Stdout); + let mut sink = ChildSink::of(&ExecConfig::DEFAULT, "child", capture::LiveStream::STDOUT); assert!(sink.discard, "the default shows the caller nothing"); assert_eq!(sink.write(b"hello").expect("a discard cannot fail"), 5); } @@ -1695,7 +1702,7 @@ mod tests { style: OutputStyle::Prefix, ..ExecConfig::DEFAULT }; - let mut sink = ChildSink::of(&settings, "child", Stream::Stdout); + let mut sink = ChildSink::of(&settings, "child", capture::LiveStream::STDOUT); let mut said = Vec::new(); sink.dress(&mut said, b"on").expect("write"); sink.dress(&mut said, b"e\ntw").expect("write"); @@ -1837,7 +1844,7 @@ mod tests { /// /// `Spool::open_in` rather than `Spool::open`: resolving the state root reads /// the OS data directory, and a unit test must not write into a developer's. - fn scratch_spool(name: &str, stream: Stream) -> capture::Spool { + fn scratch_spool(name: &str, stream: capture::LiveStream) -> capture::Spool { let dir = std::env::temp_dir().join(format!("batten-exec-unit-{}", std::process::id())); capture::Spool::open_in(&dir, stream, name).expect("open a scratch spool") } @@ -1871,7 +1878,7 @@ mod tests { let drain = Drain::spawn( NeverEnds { spoken: false }, std::io::sink(), - scratch_spool("never-ends", Stream::Stdout), + scratch_spool("never-ends", capture::LiveStream::STDOUT), ); // Wait for the one chunk to land, so the case asserts "kept what arrived" // rather than accidentally asserting "gave up before anything did". @@ -1905,7 +1912,7 @@ mod tests { let drain = Drain::spawn( &b"hello"[..], std::io::sink(), - scratch_spool("clean-eof", Stream::Stdout), + scratch_spool("clean-eof", capture::LiveStream::STDOUT), ); let mut report = Vec::new(); let (bytes, _spool) = drain diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 2dccef193..7462ec0b8 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -395,6 +395,75 @@ pub struct Capabilities { /// is the same kind of question as "what events does it emit", and two /// registries is how the answers come to disagree. pub attribution: AttributionCapabilities, + /// How faithfully this host's tool responses can be captured, per response + /// shape (CLOUD-917). + /// + /// A row group in the same table, for [`Capabilities::attribution`]'s + /// reason. **Deliberately not a [`Capability`]**, and that is a decision + /// rather than an omission: a [`crate::capture::Fidelity`] does not project + /// to a [`Declaration`] without inventing a mapping — is `SpillFile` a + /// `Yes`? is `Prefix` a `Partial`? — that would erase which of the five + /// values was measured, which is the collapse `Declaration`'s own four + /// values exist to prevent. So this is a second axis, exactly as + /// [`Capabilities::events`] is, and [`Capabilities::fidelity`] is its + /// projection. See [`Capability`]'s own doc: the scalar columns only. + pub capture: CaptureCapabilities, +} + +/// How faithfully one host's responses can be captured, per response shape +/// (CLOUD-917). +/// +/// Per shape rather than per host, because the answer genuinely differs by +/// surface: a member on the payload, a file the host spilled into and the +/// transcript are three different reachability questions, and one value for all +/// three would be right about at most one of them. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub struct CaptureCapabilities { + /// The response member on the post-tool payload — what [`decode`]'s alias + /// walk reads into [`Envelope::result`]. + pub post_tool_member: crate::capture::Fidelity, + /// A file the host spilled the response into, named on the payload. + pub spill_path: crate::capture::Fidelity, + /// The response as it appears in the host's transcript. + pub transcript: crate::capture::Fidelity, +} + +/// One surface a tool response can arrive on. +/// +/// The axis [`Capabilities::fidelity`] ranges over, in [`Event`]'s shape. These +/// are the surfaces the code actually reads, deliberately **not** a restatement +/// of the [`crate::capture::Fidelity`] values — a `SpilledFile` *shape* beside a +/// `SpillFile` *fidelity* would make the census tautological, asserting only +/// that a name equals itself. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum ResponseShape { + /// The response member on the post-tool payload. + PostToolMember, + /// A file the host spilled the response into. + SpillPath, + /// The host's transcript. + Transcript, +} + +impl ResponseShape { + /// Every response shape, so a census is derived rather than hand-kept. + pub const ALL: &'static [ResponseShape] = &[ + ResponseShape::PostToolMember, + ResponseShape::SpillPath, + ResponseShape::Transcript, + ]; + + /// The stable token, for byte-stable output (§6). + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + ResponseShape::PostToolMember => "post-tool-member", + ResponseShape::SpillPath => "spill-path", + ResponseShape::Transcript => "transcript", + } + } } /// What one host declares for one capability. @@ -759,6 +828,26 @@ impl Capabilities { self.advisory.delivered_on.contains(&raw_event) } + /// How faithfully this host's response can be captured on one shape + /// (CLOUD-917). + /// + /// The projection that makes the fidelity column's axis rangeable, the way + /// [`Capabilities::declares`] does for the scalar columns and + /// [`Capabilities::emits`] does for the event set. Every host answers for + /// every shape: the exhaustive `match` in [`Harness::capabilities`] plus + /// `#[non_exhaustive]` struct-literal construction already make a missing + /// cell a compile error, and what this adds is *reachability* — a cell that + /// is filled but that no [`ResponseShape`] can name would be a measurement + /// nothing can read. + #[must_use] + pub const fn fidelity(&self, shape: ResponseShape) -> crate::capture::Fidelity { + match shape { + ResponseShape::PostToolMember => self.capture.post_tool_member, + ResponseShape::SpillPath => self.capture.spill_path, + ResponseShape::Transcript => self.capture.transcript, + } + } + /// What this host declares for one scalar capability. /// /// The projection that makes the table's second axis rangeable. The `bool` @@ -1009,6 +1098,21 @@ const UNSURVEYED_ATTRIBUTION: AttributionCapabilities = AttributionCapabilities config_surface: Declaration::Unknown, }; +/// The capture row group every host but one shares: nothing reachable +/// (CLOUD-917). +/// +/// Shared rather than copied five times, for [`UNSURVEYED_ATTRIBUTION`]'s +/// reason. **`Unavailable` is the honest value here and not a placeholder**: it +/// says the host does not make the bytes reachable *here*, which is exactly what +/// an unmeasured surface supports. Only Claude Code's post-tool payload has been +/// measured in this repository, so widening any other cell is a measurement, +/// filed per host. +const UNSURVEYED_CAPTURE: CaptureCapabilities = CaptureCapabilities { + post_tool_member: crate::capture::Fidelity::Unavailable, + spill_path: crate::capture::Fidelity::Unavailable, + transcript: crate::capture::Fidelity::Unavailable, +}; + /// Claude Code's set: the converged core plus the four it alone offers. const CLAUDE_EVENTS: &[Event] = &[ Event::PreTool, @@ -1104,6 +1208,25 @@ impl Harness { // injection path. Neither `Yes` nor `No` is true of it. config_surface: Declaration::Partial, }, + // The one host with a measured capture row, and it is one cell + // of three. `tests/board-write-record.bats` reads this host's + // MCP content-block response shape, so the post-tool member is + // reachable — as DECODED content, because [`decode`] hands the + // engine an already-parsed value and the member's original + // bytes are gone by then. `LexicalBytes` would need the decoder + // to keep the member's raw span, which no surface here does, so + // claiming it would be the one claim + // [`crate::capture::Fidelity`]'s reserved word forbids. + // + // Nothing here spills a response to a file, and nothing reads + // the transcript for one, so both are unreachable rather than + // unmeasured — but `Unavailable` is the same answer either way + // and does not overstate which. + capture: CaptureCapabilities { + post_tool_member: crate::capture::Fidelity::DecodedContent, + spill_path: crate::capture::Fidelity::Unavailable, + transcript: crate::capture::Fidelity::Unavailable, + }, }, Harness::Cursor => Capabilities { events: CONVERGED_EVENTS, @@ -1130,6 +1253,7 @@ impl Harness { needs_fail_closed_config: true, stdout_must_stay_clean: false, attribution: UNSURVEYED_ATTRIBUTION, + capture: UNSURVEYED_CAPTURE, }, Harness::CopilotCli => Capabilities { events: CONVERGED_EVENTS, @@ -1152,6 +1276,7 @@ impl Harness { needs_fail_closed_config: false, stdout_must_stay_clean: false, attribution: UNSURVEYED_ATTRIBUTION, + capture: UNSURVEYED_CAPTURE, }, Harness::GeminiCli => Capabilities { events: CONVERGED_EVENTS, @@ -1176,6 +1301,7 @@ impl Harness { needs_fail_closed_config: false, stdout_must_stay_clean: true, attribution: UNSURVEYED_ATTRIBUTION, + capture: UNSURVEYED_CAPTURE, }, Harness::CodexCli => Capabilities { events: CONVERGED_EVENTS, @@ -1191,6 +1317,7 @@ impl Harness { needs_fail_closed_config: false, stdout_must_stay_clean: false, attribution: UNSURVEYED_ATTRIBUTION, + capture: UNSURVEYED_CAPTURE, }, Harness::ExitCode => Capabilities { events: CONVERGED_EVENTS, @@ -1221,6 +1348,18 @@ impl Harness { exposes_session_id: Declaration::Yes, config_surface: Declaration::No, }, + // `Unavailable` here is a measurement rather than a gap, for + // this arm's usual reason: this is not a third party. It is the + // normalized envelope Batten itself defines, and a caller + // composing it by hand carries no response surface at all — so + // the shape IS the answer. It coincides with the unsurveyed + // group's value and does not mean the same thing, which is why + // it is spelled out rather than borrowed. + capture: CaptureCapabilities { + post_tool_member: crate::capture::Fidelity::Unavailable, + spill_path: crate::capture::Fidelity::Unavailable, + transcript: crate::capture::Fidelity::Unavailable, + }, }, } } @@ -7624,6 +7763,91 @@ deny contains "refused by the module" if { } } + #[test] + fn every_host_declares_a_fidelity_for_every_response_shape_the_core_reads() { + // CLOUD-917's totality clause, and the third axis of this table after + // the event set and the scalar columns. Same division of labour as the + // scalar census above: the exhaustive `match` in `capabilities` plus + // `#[non_exhaustive]` struct-literal construction already make a missing + // CELL a compile error, so what this covers is the part the compiler + // cannot — a cell that is filled and that no `ResponseShape` names, + // which is a measurement nothing can range over. + for harness in Harness::ALL { + let capabilities = harness.capabilities(); + for shape in ResponseShape::ALL { + let declared = capabilities.fidelity(*shape); + assert!( + !declared.as_str().is_empty(), + "{} declares nothing for {}", + harness.as_str(), + shape.as_str() + ); + } + } + } + + #[test] + fn no_capability_token_names_the_fidelity_column() { + // What keeps the partition test below a two-subset XOR by CONSTRUCTION + // rather than by luck. The fidelity column is deliberately off the + // `Capability` axis (a `Fidelity` does not project to a `Declaration` + // without erasing which of five values was measured), so a later hand + // adding it there would have to add a third subset or break the + // partition — and this fails first, naming the reason. + for capability in Capability::ALL { + let token = capability.as_str(); + assert!( + !token.contains("fidelity") && !token.contains("capture"), + "{token} looks like the fidelity column, which is a second axis \ + rather than a scalar capability — see `Capabilities::capture`" + ); + } + } + + #[test] + fn five_of_six_hosts_declare_the_response_bytes_unreachable_rather_than_guessing() { + // The honest-value discipline, as a count. An unsurveyed response + // surface is `Unavailable`, never a guess, and exactly one host has a + // measurement here: Claude Code's post-tool member, read as DECODED + // content because the decoder hands the engine an already-parsed value. + let unreachable = Harness::ALL + .iter() + .filter(|harness| { + let capabilities = harness.capabilities(); + ResponseShape::ALL.iter().all(|shape| { + capabilities.fidelity(*shape) == crate::capture::Fidelity::Unavailable + }) + }) + .count(); + assert_eq!( + unreachable, 5, + "exactly one host has a measured capture row; widening another is a \ + measurement, filed per host" + ); + assert_eq!( + Harness::ClaudeCode + .capabilities() + .fidelity(ResponseShape::PostToolMember), + crate::capture::Fidelity::DecodedContent, + ); + } + + #[test] + fn an_unreachable_capture_is_a_different_value_from_a_decoded_one() { + // `an_absent_capability_is_a_different_value_from_an_undeclared_one`'s + // discipline on the fidelity axis: "nothing is reachable here" and "the + // decoded member is reachable" are two answers, and a reader that + // collapsed them would replay bytes it never had. + assert_ne!( + crate::capture::Fidelity::Unavailable, + crate::capture::Fidelity::DecodedContent, + ); + // And the reserved word separates them too, in the direction that + // matters: neither may be called byte-perfect. + assert!(!crate::capture::Fidelity::Unavailable.is_byte_perfect()); + assert!(!crate::capture::Fidelity::DecodedContent.is_byte_perfect()); + } + #[test] fn the_two_capability_subsets_partition_the_whole_table() { // What keeps `Capability::ATTRIBUTION` honest as a derivation source. The diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index a58146ae5..5e4b1e1f8 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -1009,17 +1009,53 @@ fn run_capture( handle, lines, grep, + raw, + bytes, json, - } => run_capture_show(&repo, handle, lines.as_deref(), grep.as_deref(), *json, out), - cli::CaptureCommand::List { stream, json } => { - run_capture_list(&repo, stream.as_deref(), *json, out) - } + } => run_capture_show( + &repo, + handle, + &ShowRequest { + lines: lines.as_deref(), + grep: grep.as_deref(), + raw: *raw, + byte_range: bytes.as_deref(), + json: *json, + }, + out, + ), + cli::CaptureCommand::List { + stream, + calls, + json, + } => run_capture_list(&repo, stream.as_deref(), *calls, *json, out), cli::CaptureCommand::Prune { yes, dry_run } => { run_capture_prune(&repo, *yes, *dry_run, mode, err) } } } +/// What one `capture show` asked for: which window, and in which encoding. +/// +/// Grouped rather than passed as five parameters, and the workspace's own +/// argument-count lint is what asked for it — correctly, because these are one +/// thing: a caller's request. Two selectors (`lines`, `grep`), one byte window +/// (`byte_range`), and two encodings (`raw`, `json`), whose legal combinations +/// [`run_capture_show`] decides in one place. +#[derive(Debug, Clone, Copy)] +struct ShowRequest<'a> { + /// A 1-indexed inclusive line range. + lines: Option<&'a str>, + /// A literal substring. + grep: Option<&'a str>, + /// Write the selected bytes verbatim. + raw: bool, + /// A 0-indexed half-open byte range. + byte_range: Option<&'a str>, + /// Emit a byte-stable document. + json: bool, +} + /// Read a frozen capture, with no second run of the command that made it. /// /// **Always [`ExitCode::Success`]**, for the same reason [`run_state_list`] is: @@ -1036,12 +1072,51 @@ fn run_capture( fn run_capture_show( repo: &Path, handle: &str, - lines: Option<&str>, - grep: Option<&str>, - json: bool, + asked: &ShowRequest<'_>, out: &mut dyn Write, ) -> Result { + let ShowRequest { + lines, + grep, + raw, + byte_range, + json, + } = *asked; let parsed = capture::Handle::parse(handle)?; + // REFUSED BEFORE ANYTHING IS READ, and refused rather than resolved + // (CLOUD-918). `--raw` and `--json` are two encodings of one selection, and a + // combination that had to pick between them silently is how a caller ends up + // with base64 where it wanted bytes. `--lines` and `--grep` are line views, + // which a byte stream is not — pairing either with `--raw` asks for two + // different products at once. + if raw && json { + return Err(UsageError::raise( + "capture show: --raw and --json are two encodings of the same selection; pass one. \ + --raw writes bytes, --json writes a byte-stable document", + )); + } + if raw && (lines.is_some() || grep.is_some()) { + return Err(UsageError::raise( + "capture show: --raw writes bytes and --lines/--grep select decoded lines; pass \ + --bytes to narrow a raw read", + )); + } + if byte_range.is_some() && (lines.is_some() || grep.is_some()) { + return Err(UsageError::raise( + "capture show: --bytes and --lines/--grep select differently; a byte range is not a \ + line range", + )); + } + // A byte range is its own selection, so it is resolved ahead of the line + // selectors rather than folded into their match — the two are not alternatives + // over one axis. + if raw || byte_range.is_some() { + let (from, to) = match byte_range { + Some(range) => parse_bytes(range)?, + None => (None, None), + }; + return run_capture_raw(repo, &parsed, from, to, raw, json, out); + } let selection = match (lines, grep) { (Some(_), Some(_)) => { return Err(UsageError::raise( @@ -1096,13 +1171,153 @@ fn run_capture_show( Ok(ExitCode::Success) } -/// List this repository's captures as handles. +/// Read a byte range of a capture — verbatim, or as a base64 document. +/// +/// Split out of [`run_capture_show`] because the two produce different things: a +/// line view is text this binary formats, and this is the child's own bytes +/// leaving the process untouched. Keeping them in one function would put a decoded +/// value in scope beside the raw path, which is exactly what `select_raw` exists +/// to avoid. +/// +/// **What the raw write bypasses**, stated because each omission is deliberate: +/// `writeln!` (so no trailing newline is added — the one thing every other arm of +/// this verb does), `serde_json::to_string_pretty`, and the whole `output::` +/// ladder. `out` is already a byte sink, so `write_all` is the whole mechanism. +/// +/// Rust's `std::io` performs no newline translation on any platform, so there is +/// no `\n` → `\r\n` hazard to guard. The platform claim worth stating is narrower: +/// on Windows a `Stdout` bound to a *console* goes through `WriteConsoleW`, which +/// requires valid UTF-8, and obtaining a true byte handle needs `unsafe` — which +/// the workspace lints forbid. So the verbatim guarantee is a guarantee about a +/// REDIRECTED stdout, which is the only way a program consumes these bytes. +fn run_capture_raw( + repo: &Path, + parsed: &capture::Handle, + from: Option, + to: Option, + raw: bool, + json: bool, + out: &mut dyn Write, +) -> Result { + let record = capture::Capture { + stream: parsed.stream.as_str(), + bytes: 0, + digest: parsed.digest.clone(), + }; + let bytes = capture::read(repo, &record).map_err(|_| { + UsageError::raise(format!( + "capture show: no capture at {parsed} — `batten capture list` names the ones this \ + repository holds" + )) + })?; + let answer = capture::select_raw(parsed, &bytes, from, to); + if raw { + // The one write in this binary that is not text. + out.write_all(&answer.data)?; + } else if json { + // Base64 rather than an escaped string, because §6 requires the document + // to be a function of the bytes and a lossy decode is not one; and rather + // than the bytes themselves, because a `-J` document is parsed by + // consumers that assume UTF-8. + writeln!( + out, + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "handle": answer.handle, + "bytes": answer.bytes, + "from": answer.from, + "to": answer.to, + "encoding": "base64", + "data": base64(&answer.data), + }))? + )?; + } else { + // Pointer-only by default here too (rule 4): a byte range with neither + // encoding named reports what it would return, never the payload. + writeln!( + out, + "{} {}..{} {} selected", + answer.handle, + answer.from, + answer.to, + answer.data.len() + )?; + } + Ok(ExitCode::Success) +} + +/// Standard base64, unpadded-free (RFC 4648 with `=` padding). +/// +/// ~20 lines rather than a dependency: `deny.toml` and +/// `tests/ambient_authority.rs` both price a new crate in the supply chain as a +/// decision to be argued, and this is the only base64 in the tool. +fn base64(bytes: &[u8]) -> String { + const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut encoded = String::with_capacity(bytes.len().div_ceil(3) * 4); + for chunk in bytes.chunks(3) { + let b0 = u32::from(chunk[0]); + let b1 = chunk.get(1).copied().map_or(0, u32::from); + let b2 = chunk.get(2).copied().map_or(0, u32::from); + let triple = (b0 << 16) | (b1 << 8) | b2; + let indices = [ + (triple >> 18) & 0x3F, + (triple >> 12) & 0x3F, + (triple >> 6) & 0x3F, + triple & 0x3F, + ]; + for (position, index) in indices.iter().enumerate() { + // The last chunk pads: two source bytes drop the final character and + // one source byte drops the final two. + if position > chunk.len() { + encoded.push('='); + } else { + encoded.push(char::from(ALPHABET[*index as usize])); + } + } + } + encoded +} + +/// List this repository's captures as handles, or its recorded calls. +/// +/// Two views over one store, on the two axes CLOUD-917 keeps separate: the blob +/// listing answers "which bytes does this repository hold", and `--calls` answers +/// "which calls happened". Dedup collapses the first and never the second, so +/// forty calls that printed the same thing are one line in the blob view and +/// forty in this one — which is the whole reason provenance is a second record. +/// +/// Both orderings are byte-stable (§6) and neither reads an mtime. The call view +/// never renders `seen_at`, because a listing that printed a timestamp would stop +/// agreeing with itself across runs. fn run_capture_list( repo: &Path, stream: Option<&str>, + calls: bool, json: bool, out: &mut dyn Write, ) -> Result { + if calls { + let recorded = capture::calls(repo)?; + if json { + writeln!(out, "{}", serde_json::to_string_pretty(&recorded)?)?; + } else { + for row in &recorded { + // Pointer-only: a handle or a reason id, the host, the event and + // the ordinal. Never bytes, never a path, never the timestamp. + let names = match (&row.digest, &row.absent) { + (Some(digest), _) => format!("response:{digest}"), + (None, Some(reason)) => format!("absent:{reason}"), + (None, None) => "absent:unrecorded".to_owned(), + }; + writeln!( + out, + "{names} {} {} {} #{}", + row.source, row.host, row.event, row.order + )?; + } + } + return Ok(ExitCode::Success); + } if let Some(stream) = stream { // Validated through the handle parser rather than a second list of stream // names, so the filter cannot come to disagree with the store about what a @@ -1186,6 +1401,53 @@ fn parse_line(range: &str, half: usize) -> Result { Ok(value) } +/// A `FROM:TO` byte range: 0-indexed, half-open, either half omittable. +/// +/// **Deliberately laxer than [`parse_line`] about an EMPTY half and exactly as +/// strict about a malformed one**, and the divergence is declared rather than +/// inherited. `parse_line` refuses `5:` because defaulting the end would make it +/// mean "the rest" without anyone saying so — the range is inclusive, so there is +/// no notation for an open end and a caller could not have meant one. A byte range +/// is half-open, so an absent bound is the *only* way to say "to the end" without +/// first learning the length, and reading it as such invents nothing. +/// +/// Three refusals, worded apart so a caller can tell them from each other: no +/// separator, a non-numeric half, and a half that overflows `u64`. The last is +/// separate because "too big" and "not a number" have different fixes. +/// +/// **It never compares against a length**, because it has none — an out-of-range +/// but well-formed bound is clamped, and the clamp lives in +/// [`capture::select_raw`] and only there. +fn parse_bytes(range: &str) -> Result<(Option, Option)> { + let shape = || { + UsageError::raise(format!( + "capture show: {range:?} is not a byte range — write `FROM:TO`, 0-indexed, `FROM` \ + inclusive and `TO` exclusive; either side may be omitted" + )) + }; + let (from, to) = range.split_once(':').ok_or_else(shape)?; + let half = |text: &str, side: &str| -> Result> { + let text = text.trim(); + if text.is_empty() { + return Ok(None); + } + if !text.bytes().all(|byte| byte.is_ascii_digit()) { + return Err(UsageError::raise(format!( + "capture show: the {side} bound {text:?} is not a number — a byte offset is \ + decimal digits" + ))); + } + text.parse::().map(Some).map_err(|_| { + UsageError::raise(format!( + "capture show: the {side} bound {text:?} is larger than {} — no capture is that \ + long", + u64::MAX + )) + }) + }; + Ok((half(from, "start")?, half(to, "end")?)) +} + /// List what the store holds. /// /// **Always [`ExitCode::Success`].** A stored finding is a record, not a fresh @@ -1878,8 +2140,8 @@ fn run_hook( // Failure is silent by design. A hook that cannot write a fact must not // become the reason work stops; the retry will simply deny again with the // same `Fix::Run`, which is the safe direction and a visible one. - if envelope.event == hook::Event::PostTool && !envelope.command.is_empty() { - record_agent_fact(overrides, &envelope); + if envelope.event == hook::Event::PostTool { + record_post_tool(overrides, &envelope, harness, &mut advice); } let (policy, waivers) = if bypass || (envelope.command.is_empty() && envelope.writes.is_none()) { @@ -2667,6 +2929,189 @@ fn record_agent_fact(overrides: &Overrides, envelope: &hook::Envelope) { let _ = receipt::record_sourced(&declared.name, &record); } +/// Everything the post-tool event records, in the order it must happen. +/// +/// Extracted from [`run_hook`] because the workspace's function-length lint asked +/// for it, and the lint was right: this is one decision — what a completed call +/// leaves behind — and `run_hook`'s body is a sequence of stages, not a place to +/// spell one out. +/// +/// **The capture is first, and textually first is the point.** CLOUD-917 makes it +/// the authoritative record and the row count a derived view of it, so projecting +/// first would permit a build where the count survived and the bytes did not. +fn record_post_tool( + overrides: &Overrides, + envelope: &hook::Envelope, + harness: hook::Harness, + advice: &mut Vec, +) { + // GATED ONLY ON THE RESPONSE MEMBER, never on the command. The + // `!command.is_empty()` conjunct below is correct for a FACT — a fact is + // keyed to a command that ran — and wrong for a capture, because the + // response of a structured tool is still a response. That conjunct is why + // every MCP call, every `Read` and every `Write` is missed today. + // + // A host that literally sends `"tool_response": null` is indistinguishable + // from one that sends no member at all, because `decode`'s alias walk + // produces `Value::Null` for both. Accepted rather than papered over: + // telling them apart is `decode`'s work and buys nothing any surveyed host + // produces. + if !envelope.result.is_null() { + capture_response(envelope, harness, advice); + } + // CLOUD-776's gate, byte-for-byte what it was: a fact is keyed to a command + // that ran, so a call carrying no command has no fact to record. + if !envelope.command.is_empty() { + record_agent_fact(overrides, envelope); + } +} + +/// Persist this post-tool response as a local capture (CLOUD-919). +/// +/// **Returns nothing, and that is the failure posture rather than laziness.** +/// CLOUD-917's decision, applied: hook execution continues, the exit code is +/// unchanged and never `2` — a storage failure is not a policy verdict — and the +/// outcome is *recorded and reported* instead of raised. It takes `&mut advice` +/// rather than returning a `Result` for the same reason: a `?` here would cross +/// the fill-and-drain span the advisory buffer lives in, and the one escape that +/// span tolerates is a config failure at exit 3. +/// +/// This is deliberately the OPPOSITE posture from [`capture::store`]'s own +/// "never a silent skip" doc, and the surface is the reason. `store` is called by +/// `exec`, where an unrecorded capture is a lie about a command a human asked +/// for. Here it is called on the mediated path, where the non-negotiable is that +/// no Batten failure can block a tool call. +/// +/// **No repo root, no capture.** [`hook_authority_root`] falls back to the cwd, +/// so capturing unconditionally would scatter state roots across the filesystem +/// on every post-tool call made anywhere — something nothing in this binary did +/// before. A call from outside a repository records the reason and stores +/// nothing. +/// +/// Three outcomes, kept distinct (CLOUD-917): an absent member never reaches +/// here, a present-but-empty one is a real record of zero bytes, and a +/// non-empty one is its bytes at the declared fidelity. The provenance row is +/// what tells the first two apart — one carries a digest, the other a reason id, +/// and they differ in which keys exist rather than in a count. +fn capture_response(envelope: &hook::Envelope, harness: hook::Harness, advice: &mut Vec) { + let mut note = |reason: &str| { + // Pointer-only: the reason id, never a path and never a byte count that + // could fingerprint the content. The same id reaches `doctor`. + advice.push(format!("hook.capture.response: {reason}")); + }; + // NO FALLBACK TO THE CWD, which is what makes the doc above true: resolving + // to wherever the agent happens to be standing would mint a state root there + // on every post-tool call. The anchor rather than `.` for the second half of + // the same reason (CLOUD-824): every other repository read in `run_hook` + // resolves through `hook_authority_root`, and a capture written under one + // root while its budget is read from another is two authorities for one + // call. + let Ok(root) = git::repo_root(hook_authority_root()) else { + note(capture::STATE_ROOT_UNRESOLVED); + return; + }; + let decoded = match capture::decode_response(&envelope.result) { + Ok(decoded) => decoded, + Err(reason) => { + note(reason); + record_absence(&root, envelope, harness, reason, advice); + return; + } + }; + let Ok(stored) = capture::store(&root, capture::Stream::Response, &decoded.bytes) else { + note(capture::STORE_UNWRITABLE); + record_absence(&root, envelope, harness, capture::STORE_UNWRITABLE, advice); + return; + }; + let row = capture::CallRow { + order: 0, + // A host that names no session still gets a row, under a token that says + // so: dropping the row would make an unnamed session look like no calls, + // and inventing an id would make two of them look like one. + session: envelope + .session + .clone() + .unwrap_or_else(|| "unnamed".to_owned()), + source: "post-tool-member".to_owned(), + host: harness.as_str().to_owned(), + tool: envelope.raw_tool.clone(), + event: hook::Event::PostTool.as_str().to_owned(), + fidelity: decoded.fidelity.as_str().to_owned(), + // The clock is read at the boundary, as `record_agent_fact` reads it and + // for the same reason. It is a fact about the CALL, which is why it may + // live on this row while the content-addressed blob still refuses one. + seen_at: Some(receipt::rfc3339_utc( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |since_epoch| since_epoch.as_secs()), + )), + class: None, + digest: Some(stored.digest), + absent: None, + }; + if capture::record_call(&root, &row).is_err() { + // THE ONE UNRECORDABLE FAILURE (CLOUD-917 clause 4): the provenance write + // itself. The advisory is the only channel left, which is why it is not + // conditional on the row. + note(capture::STORE_UNWRITABLE); + } + // Write-time only, and only for responses. `exec` captures are not + // candidates, so that consumer's behaviour is byte-identical to before. + // An `Err` here is a store that could not be READ or REWRITTEN — the bytes + // are already published and the row is already written, so it is never a + // budget refusal. `BUDGET_EXHAUSTED` would send a `doctor` reader to the + // wrong remedy, which is the whole point of a reason id. + if capture::evict_to_budget(&root, capture_budget().as_ref()).is_err() { + note(capture::STORE_UNWRITABLE); + } +} + +/// Record that a capture did not happen, so "no record" cannot mean "no calls". +fn record_absence( + root: &Path, + envelope: &hook::Envelope, + harness: hook::Harness, + reason: &'static str, + advice: &mut Vec, +) { + let row = capture::CallRow { + order: 0, + // A host that names no session still gets a row, under a token that says + // so: dropping the row would make an unnamed session look like no calls, + // and inventing an id would make two of them look like one. + session: envelope + .session + .clone() + .unwrap_or_else(|| "unnamed".to_owned()), + source: "post-tool-member".to_owned(), + host: harness.as_str().to_owned(), + tool: envelope.raw_tool.clone(), + event: hook::Event::PostTool.as_str().to_owned(), + fidelity: capture::Fidelity::Unavailable.as_str().to_owned(), + seen_at: None, + class: None, + digest: None, + absent: Some(reason.to_owned()), + }; + if capture::record_call(root, &row).is_err() { + advice.push(format!( + "hook.capture.response: {}", + capture::STORE_UNWRITABLE + )); + } +} + +/// The `[capture]` bound, or `None` when the authority cannot be read. +/// +/// Read separately from the policy load below, and deliberately tolerant: a +/// capture must not fail because config did not parse, so an unreadable +/// authority means the engine defaults rather than an error. +fn capture_budget() -> Option { + let text = std::fs::read_to_string(hook_authority_root().join(config::CONFIG_FILE)).ok()?; + let parsed: config::Config = toml::from_str(&text).ok()?; + parsed.capture +} + /// The directory `batten hook` reads its authority from (CLOUD-824). /// /// **Never the session's cwd on its own.** `load_policy` reads `./batten.toml` diff --git a/crates/batten/src/surface.rs b/crates/batten/src/surface.rs index ff38443a2..ef46889ae 100644 --- a/crates/batten/src/surface.rs +++ b/crates/batten/src/surface.rs @@ -792,6 +792,74 @@ const GREP: FlagDecl = FlagDecl { value: ValueDecl::Str, }; +/// `--raw` on `capture show`: the selected bytes, verbatim (CLOUD-918). +/// +/// **The one operation in this binary whose output is not text**, and therefore +/// the one that must never appear under `-J`: a raw byte stream and a byte-stable +/// JSON document are different contracts, and a flag combination that had to pick +/// one silently is how a caller ends up with base64 where it wanted bytes. So +/// `--raw` with `--json`, `--lines` or `--grep` is refused rather than resolved. +/// +/// It is an ENCODING rather than a selector: `--bytes` chooses what to read and +/// this chooses how it leaves the process. `--raw` alone is the whole capture. +const RAW: FlagDecl = FlagDecl { + id: "raw", + long: Some("raw"), + short: None, + help: "Write the selected bytes to stdout verbatim, with no decode and no added newline", + env: EnvDecl::None, + global: false, + positional: false, + required: false, + hidden: false, + rung: Rung::None, + value: ValueDecl::Bool, +}; + +/// `--bytes FROM:TO` on `capture show`: a byte range (CLOUD-918). +/// +/// 0-indexed and half-open where [`LINES`] is 1-indexed and inclusive, which is +/// deliberate: byte ranges have to tile, so `0:N` then `N:M` must cover a record +/// exactly once. Either half may be omitted. Clamped for [`LINES`]'s reason — an +/// out-of-range end is a caller who wants the rest — while a MALFORMED bound is a +/// usage error, because a caller who wrote nonsense should be told rather than +/// handed a plausible answer. +const BYTES: FlagDecl = FlagDecl { + id: "bytes", + long: Some("bytes"), + short: None, + help: "A 0-indexed half-open byte range, `FROM:TO`, either side omittable, clamped to the \ + capture", + env: EnvDecl::None, + global: false, + positional: false, + required: false, + hidden: false, + rung: Rung::None, + value: ValueDecl::Str, +}; + +/// `--calls` on `capture list`: the per-call provenance view (CLOUD-918). +/// +/// A second view over the same store rather than a second verb, because the +/// question is the same one — what does this repository hold — asked on the +/// invocation axis instead of the content axis. The blob listing answers "which +/// bytes", this answers "which calls", and dedup collapses the first without +/// collapsing the second. +const CALLS: FlagDecl = FlagDecl { + id: "calls", + long: Some("calls"), + short: None, + help: "List recorded calls instead of stored captures, in a byte-stable order", + env: EnvDecl::None, + global: false, + positional: false, + required: false, + hidden: false, + rung: Rung::None, + value: ValueDecl::Bool, +}; + /// `--stream ` on `capture list`: narrow the listing. /// /// A plain string rather than a `ValueEnum`, because the set it validates against @@ -1093,6 +1161,11 @@ pub const SURFACE: &[CommandDecl] = &[ // the child's own and were already paid for once; withholding them would leave // re-running the command as the only way to see more, which is the behaviour // this issue exists to delete. The default selection is still the pointer. + // + // Since CLOUD-918 it is also the one verb whose product is not TEXT. `--raw` + // writes bytes to stdout verbatim, which is why it is refused alongside + // `--json`: the two are different contracts over the same selection, and the + // `-J` ladder is byte-stable text by construction. CommandDecl { path: "capture show", about: "Print a capture's pointer, or the lines a selection asks for, with no second run", @@ -1102,6 +1175,8 @@ pub const SURFACE: &[CommandDecl] = &[ FlagDecl::positional("handle", "The `:` handle to read"), LINES, GREP, + RAW, + BYTES, JSON, ], }, @@ -1111,7 +1186,7 @@ pub const SURFACE: &[CommandDecl] = &[ about: "List this repository's captures as handles, in a fixed order", data_channel: true, effect: Effect::Read, - flags: &[STREAM, JSON], + flags: &[STREAM, CALLS, JSON], }, // `destructive`, not `write`: what it removes is a record of a run that has // already happened, and recovering one means re-running the command — which is diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index ef7bbaf03..d5f31b208 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -853,6 +853,17 @@ pub const CENSUS: &[FieldCoverage] = &[ `exec_pattern`, which is compared on its own row", ), }, + FieldCoverage { + field: "capture", + coverage: Coverage::NoMonotoneReading( + "a retention bound on a local store (CLOUD-918): how many response captures the \ + store keeps and how many bytes they may occupy. No rule's verdict reads the \ + store — an `exec_pattern` predicate scans the bytes in the process that captured \ + them, and `capture show` is navigation rather than a gate — so a tighter bound \ + evicts a record an agent might have wanted to re-read and weakens no finding. \ + Tightening and loosening are both just storage", + ), + }, FieldCoverage { field: "markers", coverage: Coverage::Compared(&[WeakeningKind::MarkerRemoved]), diff --git a/crates/batten/tests/capture_fidelity.rs b/crates/batten/tests/capture_fidelity.rs new file mode 100644 index 000000000..c1187394c --- /dev/null +++ b/crates/batten/tests/capture_fidelity.rs @@ -0,0 +1,146 @@ +//! "byte-perfect" is a reserved word, and this is what reserves it (CLOUD-917). +//! +//! [`batten::capture::Fidelity`] carries five values and exactly two of them may +//! be described as a faithful reproduction of the bytes a host framed. The other +//! three each fail differently: a decoded member is exact for what it decoded and +//! renormalizes key order, escaping and whitespace on the way back out; a prefix +//! makes no completeness claim at all; and an unavailable surface holds nothing. +//! +//! Without a mechanism this is a sentence in a doc comment, which is feedforward +//! only. The rule the issue states is that **no doc comment, output line, record +//! field or test name** may describe a reserialized decoded value that way — so +//! the gate reads the module's own prose and the type's own rendering, and +//! compares both against the one authority, `Fidelity::is_byte_perfect`. +//! +//! ## Why the sources are read with `include_str!` +//! +//! Compile-time, so there is no runtime path to resolve and no absolute path in a +//! failure message (non-negotiable rule 4, and `doctor.rs`'s +//! `a_reason_id_never_carries_a_path` for the same reason). It also means a +//! rename of either file is a compile error here rather than a scan that quietly +//! finds nothing — the `mise-tasks/` no-extension defect +//! (`.claude/rules/scanning.md`) in a different costume. + +/// The term itself, spelled once so the scan cannot drift from the rule. +const RESERVED: &str = "byte-perfect"; + +/// The two values that may carry it, by token. +const ADMITTED: &[&str] = &["LexicalBytes", "SpillFile"]; + +/// The three that may not. +const REFUSED: &[&str] = &["DecodedContent", "Prefix", "Unavailable"]; + +const CAPTURE_SRC: &str = include_str!("../src/capture.rs"); +const HOOK_SRC: &str = include_str!("../src/hook.rs"); + +/// The doc paragraphs of a source file. +/// +/// A paragraph rather than a line, because the claim and the value it is about +/// are routinely on different lines of one comment — scanning line by line would +/// pass a comment that says "byte-perfect" two lines above the value it means. +/// +/// **A blank doc line ends a paragraph**, and getting that wrong is what made +/// the first version of this scan unusable: treating a whole doc BLOCK as one +/// paragraph means a type whose docs legitimately discuss all five values can +/// never mention the term at all, so the only way to pass is to stop explaining +/// the rule. A paragraph is what a reader sees as one, and the reserved word +/// binds to the claim in front of them. +fn doc_paragraphs(src: &str) -> Vec { + let mut paragraphs = Vec::new(); + let mut current: Vec<&str> = Vec::new(); + let mut flush = |current: &mut Vec<&str>| { + if !current.is_empty() { + paragraphs.push(current.join(" ")); + current.clear(); + } + }; + for line in src.lines() { + let trimmed = line.trim_start(); + let body = trimmed + .strip_prefix("///") + .or_else(|| trimmed.strip_prefix("//!")); + match body { + // A doc line with nothing on it is the paragraph break a reader sees. + Some(text) if text.trim().is_empty() => flush(&mut current), + Some(text) => current.push(text.trim()), + None => flush(&mut current), + } + } + flush(&mut current); + paragraphs +} + +#[test] +fn only_a_lexical_or_spill_fidelity_is_ever_called_byte_perfect() { + // Half one: the prose. Every doc paragraph that uses the reserved term must + // name one of the two values it is true of, and must NOT name any of the + // three it is false of. + // + // A NEGATED mention counts as a mention, deliberately: "not byte-perfect" + // beside `DecodedContent` reads correctly to a human and still puts the + // phrase one careless edit away from being a claim. So the refused values + // avoid the hyphenated term entirely and say what they ARE instead — which + // is why `Fidelity`'s own variant docs are worded the way they are. + for (file, src) in [("capture.rs", CAPTURE_SRC), ("hook.rs", HOOK_SRC)] { + for paragraph in doc_paragraphs(src) { + if !paragraph.contains(RESERVED) { + continue; + } + // This module's own header states the rule and necessarily names + // every value while doing so; it is the one paragraph exempt, and it + // is exempt by being in a different file from the two scanned. + let names_admitted = ADMITTED.iter().any(|value| paragraph.contains(value)); + let named_refused: Vec<&&str> = REFUSED + .iter() + .filter(|value| paragraph.contains(**value)) + .collect(); + assert!( + names_admitted, + "{file}: a paragraph uses {RESERVED:?} without naming a value it \ + is true of ({ADMITTED:?})" + ); + assert!( + named_refused.is_empty(), + "{file}: a paragraph uses {RESERVED:?} beside {named_refused:?}, \ + which cannot be described that way — state what the value IS \ + rather than what it is not" + ); + } + } +} + +#[test] +fn the_rendered_fidelity_note_carries_the_claim_exactly_where_the_type_does() { + // Half two: the rendering, in both directions, so the claim cannot drift + // out of the output while staying in the type or vice versa. + for fidelity in batten::capture::Fidelity::ALL { + let note = fidelity.note(); + assert_eq!( + note.contains(RESERVED), + fidelity.is_byte_perfect(), + "{}: the rendered note and `is_byte_perfect` disagree about the \ + reserved word", + fidelity.as_str() + ); + } +} + +#[test] +fn no_fidelity_token_or_note_carries_a_path() { + // §6 and rule 4 together, the sibling of `doctor.rs`'s + // `a_reason_id_never_carries_a_path`: these strings reach a byte-stable + // record and a `doctor` row, where an absolute path would differ per machine + // and leak disk layout. + for fidelity in batten::capture::Fidelity::ALL { + assert!( + !fidelity.as_str().contains('/'), + "{} looks like a path", + fidelity.as_str() + ); + assert!( + !fidelity.note().contains('/'), + "{}'s note looks like a path", + fidelity.as_str() + ); + } +} diff --git a/crates/batten/tests/cli.rs b/crates/batten/tests/cli.rs index 2c7983c2e..a71c02ed7 100644 --- a/crates/batten/tests/cli.rs +++ b/crates/batten/tests/cli.rs @@ -9044,6 +9044,269 @@ fn the_navigation_verbs_declare_themselves_read_in_the_spec() { assert_eq!(effect("capture prune"), "destructive"); } +// --- the byte-exact route (CLOUD-918) ---------------------------------------- +// +// Every test above reads a capture through the LINE view, which is an explicitly +// lossy projection: `select` decodes with `from_utf8_lossy` and the module doc +// says so. That made the module's own promise — "the bytes stay exact in the +// store" — unfalsifiable, because no operation returned them. +// +// THE ASSERTION IS ON BYTES, and that choice is what gives these tests teeth. A +// line count or a rendered string would survive a `--raw` path that decoded +// somewhere along the way; comparing the raw stdout to the bytes the child wrote +// does not. Routing `--raw` through `from_utf8_lossy` reds the first two +// immediately, which is this row's CLOUD-418 obligation. + +/// Run a child that writes `printf`-escaped `bytes`, capturing them. +#[cfg(unix)] +fn run_raw_bytes(repo: &std::path::Path, home: &std::path::Path, escaped: &str) -> Output { + batten() + .args(["exec", "--capture-only"]) + .args(["--", "sh", "-c", &format!("printf '{escaped}'")]) + .current_dir(repo) + .state_home(home) + .env_remove("BATTEN_FAIL_ON_WARNING") + .output() + .expect("run batten exec") +} + +#[cfg(unix)] +#[test] +fn an_invalid_utf8_byte_round_trips_through_raw_unchanged() { + // A lone continuation byte: valid nowhere in UTF-8, so a decode replaces it + // with U+FFFD and the bytes that come back are not the bytes that went in. + let (repo, home) = capture_repo("raw-invalid-utf8"); + assert_eq!( + run_raw_bytes(&repo, &home, "before\\377after") + .status + .code(), + Some(0) + ); + let handle = stdout_handle(&repo, &home); + let read = run_capture(&repo, &home, &["show", &handle, "--raw"]); + assert_eq!(read.status.code(), Some(0)); + assert_eq!( + read.stdout, + b"before\xffafter".to_vec(), + "the raw read decoded the bytes instead of returning them" + ); +} + +#[cfg(unix)] +#[test] +fn a_nul_byte_round_trips_through_raw_unchanged() { + // A NUL survives a UTF-8 decode, so this catches a different defect from the + // case above: a path that treats the capture as a C string, or that trims. + let (repo, home) = capture_repo("raw-nul"); + assert_eq!( + run_raw_bytes(&repo, &home, "a\\000b").status.code(), + Some(0) + ); + let handle = stdout_handle(&repo, &home); + let read = run_capture(&repo, &home, &["show", &handle, "--raw"]); + assert_eq!(read.status.code(), Some(0)); + assert_eq!(read.stdout, b"a\0b".to_vec()); +} + +#[cfg(unix)] +#[test] +fn a_raw_read_adds_no_trailing_newline() { + // The one thing every other arm of this verb does. A caller reassembling a + // capture from ranges would otherwise get a byte per range it never stored. + let (repo, home) = capture_repo("raw-no-newline"); + assert_eq!(run_raw_bytes(&repo, &home, "tight").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + let read = run_capture(&repo, &home, &["show", &handle, "--raw"]); + assert_eq!(read.stdout, b"tight".to_vec()); +} + +#[cfg(unix)] +#[test] +fn a_raw_byte_range_returns_exactly_that_range_and_tiles() { + // Half-open is what makes ranges composable: `0:3` then `3:6` must reassemble + // the record with nothing doubled and nothing dropped. An inclusive + // convention would repeat a byte at every seam. + let (repo, home) = capture_repo("raw-range"); + assert_eq!(run_raw_bytes(&repo, &home, "abcdef").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + let head = run_capture(&repo, &home, &["show", &handle, "--raw", "--bytes", "0:3"]); + let tail = run_capture(&repo, &home, &["show", &handle, "--raw", "--bytes", "3:6"]); + assert_eq!(head.stdout, b"abc".to_vec()); + assert_eq!(tail.stdout, b"def".to_vec()); + let mut tiled = head.stdout.clone(); + tiled.extend_from_slice(&tail.stdout); + assert_eq!(tiled, b"abcdef".to_vec()); + // An omitted bound means the end, which is the only way to say so without + // first learning the length. + let rest = run_capture(&repo, &home, &["show", &handle, "--raw", "--bytes", "3:"]); + assert_eq!(rest.stdout, b"def".to_vec()); +} + +#[cfg(unix)] +#[test] +fn a_byte_range_past_the_end_is_clamped_rather_than_refused() { + // `Selection::Lines`'s posture, on the byte axis: widening a window is the + // point, so an out-of-range but well-formed bound answers. + let (repo, home) = capture_repo("raw-clamp"); + assert_eq!(run_raw_bytes(&repo, &home, "short").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + let wide = run_capture( + &repo, + &home, + &["show", &handle, "--raw", "--bytes", "0:5000"], + ); + assert_eq!(wide.status.code(), Some(0)); + assert_eq!(wide.stdout, b"short".to_vec()); + // Entirely past the end is an empty answer, not a failure. + let past = run_capture( + &repo, + &home, + &["show", &handle, "--raw", "--bytes", "1000:2000"], + ); + assert_eq!(past.status.code(), Some(0)); + assert!(past.stdout.is_empty()); + // And an inverted range selects nothing rather than panicking. + let inverted = run_capture(&repo, &home, &["show", &handle, "--raw", "--bytes", "4:1"]); + assert_eq!(inverted.status.code(), Some(0)); + assert!(inverted.stdout.is_empty()); +} + +#[cfg(unix)] +#[test] +fn a_malformed_byte_bound_is_a_usage_error_rather_than_a_clamp() { + // The split that makes clamping safe to offer: a caller who guessed a window + // gets an answer, and a caller who wrote nonsense gets told. Exit 1, never a + // policy verdict. + let (repo, home) = capture_repo("raw-malformed"); + assert_eq!(run_raw_bytes(&repo, &home, "body").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + for range in ["notanumber:4", "0:nope", "nocolon"] { + let refused = run_capture(&repo, &home, &["show", &handle, "--raw", "--bytes", range]); + assert_eq!( + refused.status.code(), + Some(1), + "{range:?} was not refused as a usage error" + ); + } +} + +#[cfg(unix)] +#[test] +fn raw_and_json_name_two_encodings_and_are_refused_together() { + // Two encodings of one selection. Resolving it silently is how a caller ends + // up with base64 where it wanted bytes, so the combination is refused. + let (repo, home) = capture_repo("raw-vs-json"); + assert_eq!(run_raw_bytes(&repo, &home, "body").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + let refused = run_capture(&repo, &home, &["show", &handle, "--raw", "-J"]); + assert_eq!(refused.status.code(), Some(1)); + assert!(refused.stdout.is_empty(), "a refusal emitted a document"); +} + +#[cfg(unix)] +#[test] +fn raw_and_a_line_selection_are_refused_together() { + // A byte stream is not a line view. `--bytes` is how a raw read narrows. + let (repo, home) = capture_repo("raw-vs-lines"); + assert_eq!( + run_raw_bytes(&repo, &home, "one\\ntwo\\n").status.code(), + Some(0) + ); + let handle = stdout_handle(&repo, &home); + for selector in [["--lines", "1:2"], ["--grep", "one"]] { + let refused = run_capture( + &repo, + &home, + &["show", &handle, "--raw", selector[0], selector[1]], + ); + assert_eq!( + refused.status.code(), + Some(1), + "--raw {} was not refused", + selector[0] + ); + } +} + +#[cfg(unix)] +#[test] +fn a_byte_selection_under_json_is_base64_and_names_its_encoding() { + // `--raw` and `--json` are refused together, so this is the OTHER encoding of + // a byte range: base64 rather than an escaped string, because §6 requires the + // document to be a function of the bytes and a lossy decode is not one. + let (repo, home) = capture_repo("raw-json-doc"); + assert_eq!( + run_raw_bytes(&repo, &home, "before\\377after") + .status + .code(), + Some(0) + ); + let handle = stdout_handle(&repo, &home); + let read = run_capture(&repo, &home, &["show", &handle, "--bytes", "0:12", "-J"]); + assert_eq!(read.status.code(), Some(0)); + let document: serde_json::Value = + serde_json::from_slice(&read.stdout).expect("a JSON document"); + assert_eq!(document["encoding"], "base64"); + assert_eq!(document["from"], 0); + assert_eq!(document["to"], 12); + // The document CARRIED the invalid byte, which is the property under test: a + // JSON string could not have, and `serde_json` would have refused to emit + // one. Twelve bytes encode to sixteen characters of the base64 alphabet, and + // asserting the shape here rather than a literal keeps this test from + // re-implementing the encoder it is checking — the exact-value case is below. + let data = document["data"].as_str().expect("data is a string"); + assert_eq!(data.len(), 16, "12 bytes should encode to 16 characters"); + assert!( + data.bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'+' | b'/' | b'=')), + "the payload is not base64: {data}" + ); +} + +#[cfg(unix)] +#[test] +fn the_base64_payload_is_the_standard_encoding_of_the_selected_bytes() { + // The exact-value half, on a body whose encoding is unambiguous, so the + // encoder is pinned against RFC 4648 rather than against itself. `abcdef` is + // 6 bytes — two whole 3-byte groups, so no padding is involved either. + let (repo, home) = capture_repo("raw-json-exact"); + assert_eq!(run_raw_bytes(&repo, &home, "abcdef").status.code(), Some(0)); + let handle = stdout_handle(&repo, &home); + let read = run_capture(&repo, &home, &["show", &handle, "--bytes", "0:6", "-J"]); + let document: serde_json::Value = + serde_json::from_slice(&read.stdout).expect("a JSON document"); + assert_eq!(document["data"], "YWJjZGVm"); + // And a length that is not a multiple of three pads, which is where a + // hand-rolled encoder usually goes wrong. + let one = run_capture(&repo, &home, &["show", &handle, "--bytes", "0:1", "-J"]); + let document: serde_json::Value = serde_json::from_slice(&one.stdout).expect("a JSON document"); + assert_eq!(document["data"], "YQ=="); + let two = run_capture(&repo, &home, &["show", &handle, "--bytes", "0:2", "-J"]); + let document: serde_json::Value = serde_json::from_slice(&two.stdout).expect("a JSON document"); + assert_eq!(document["data"], "YWI="); +} + +#[cfg(unix)] +#[test] +fn a_byte_range_without_an_encoding_is_a_pointer_rather_than_the_payload() { + // Rule 4 holds on this path too: naming a window with neither encoding + // reports what it WOULD return. Content stays something a caller asks for. + let (repo, home) = capture_repo("raw-pointer"); + assert_eq!( + run_raw_bytes(&repo, &home, "secretish").status.code(), + Some(0) + ); + let handle = stdout_handle(&repo, &home); + let pointed = run_capture(&repo, &home, &["show", &handle, "--bytes", "0:9"]); + assert_eq!(pointed.status.code(), Some(0)); + let text = stdout(&pointed); + assert!(text.contains("0..9"), "the pointer names no window: {text}"); + assert!( + !text.contains("secretish"), + "the pointer carried the payload: {text}" + ); +} + /// Run `batten payload field --harness --name ` over `payload`. fn run_payload_field(name: &str, payload: &str) -> Output { let mut command = batten(); diff --git a/crates/batten/tests/fixtures/hooks/claude-code-posttool.json b/crates/batten/tests/fixtures/hooks/claude-code-posttool.json new file mode 100644 index 000000000..acc41e61e --- /dev/null +++ b/crates/batten/tests/fixtures/hooks/claude-code-posttool.json @@ -0,0 +1,15 @@ +{ + "hook_event_name": "PostToolUse", + "session_id": "sess-posttool-bench", + "cwd": "/repo", + "tool_name": "Bash", + "tool_use_id": "call-bench-1", + "tool_input": { + "command": "git status --porcelain" + }, + "tool_response": { + "stdout": "modified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\nmodified: crates/batten/src/capture.rs\nmodified: crates/batten/src/hook.rs\n", + "stderr": "", + "interrupted": false + } +} diff --git a/hk.pkl b/hk.pkl index 22c9660a1..50c37241a 100644 --- a/hk.pkl +++ b/hk.pkl @@ -399,6 +399,7 @@ local gate = new Mapping { "crates/batten/src/action.rs", "crates/batten/src/attribution.rs", "crates/batten/src/budget.rs", + "crates/batten/src/capture.rs", "crates/batten/src/ci.rs", "crates/batten/src/commit.rs", "crates/batten/src/config.rs", diff --git a/man/batten-capture-list.1 b/man/batten-capture-list.1 index 1df17f1e5..4e1a168e6 100644 --- a/man/batten-capture-list.1 +++ b/man/batten-capture-list.1 @@ -4,7 +4,7 @@ .SH NAME batten\-capture\-list \- List this repository\*(Aqs captures as handles, in a fixed order .SH SYNOPSIS -\fBbatten capture list\fR [\fB\-\-stream\fR] [\fB\-J\fR|\fB\-\-json\fR] [\fB\-h\fR|\fB\-\-help\fR] +\fBbatten capture list\fR [\fB\-\-stream\fR] [\fB\-\-calls\fR] [\fB\-J\fR|\fB\-\-json\fR] [\fB\-h\fR|\fB\-\-help\fR] .SH DESCRIPTION List this repository\*(Aqs captures as handles, in a fixed order .SH OPTIONS @@ -12,6 +12,9 @@ List this repository\*(Aqs captures as handles, in a fixed order \fB\-\-stream\fR Only captures of this stream .TP +\fB\-\-calls\fR +List recorded calls instead of stored captures, in a byte\-stable order +.TP \fB\-J\fR, \fB\-\-json\fR Emit byte\-stable JSON instead of pointer lines .TP diff --git a/man/batten-capture-show.1 b/man/batten-capture-show.1 index 252dc9a25..2d3121b80 100644 --- a/man/batten-capture-show.1 +++ b/man/batten-capture-show.1 @@ -4,7 +4,7 @@ .SH NAME batten\-capture\-show \- Print a capture\*(Aqs pointer, or the lines a selection asks for, with no second run .SH SYNOPSIS -\fBbatten capture show\fR [\fB\-\-lines\fR] [\fB\-\-grep\fR] [\fB\-J\fR|\fB\-\-json\fR] [\fB\-h\fR|\fB\-\-help\fR] <\fIhandle\fR> +\fBbatten capture show\fR [\fB\-\-lines\fR] [\fB\-\-grep\fR] [\fB\-\-raw\fR] [\fB\-\-bytes\fR] [\fB\-J\fR|\fB\-\-json\fR] [\fB\-h\fR|\fB\-\-help\fR] <\fIhandle\fR> .SH DESCRIPTION Print a capture\*(Aqs pointer, or the lines a selection asks for, with no second run .SH OPTIONS @@ -15,6 +15,12 @@ A 1\-indexed inclusive line range, `FROM:TO`, clamped to the capture \fB\-\-grep\fR Only lines containing this literal substring .TP +\fB\-\-raw\fR +Write the selected bytes to stdout verbatim, with no decode and no added newline +.TP +\fB\-\-bytes\fR +A 0\-indexed half\-open byte range, `FROM:TO`, either side omittable, clamped to the capture +.TP \fB\-J\fR, \fB\-\-json\fR Emit byte\-stable JSON instead of pointer lines .TP diff --git a/mise-tasks/perf-assert.sh b/mise-tasks/perf-assert.sh index af95eb905..c55403839 100755 --- a/mise-tasks/perf-assert.sh +++ b/mise-tasks/perf-assert.sh @@ -65,8 +65,14 @@ set -euo pipefail # perceives and the wiring is what they experience. `hook` stays budgeted # alongside it so the launcher's own share stays attributable — a regression in # one and not the other says where to look. +# +# `posttool` is budgeted at the same floor (CLOUD-919). Naming it here is what +# ARMS the presence gate below: from now on a `perf` run that does not emit +# `path=posttool` is exit 2 rather than a green run with one fewer measurement, +# which is what makes the arm's existence non-optional rather than a courtesy. BUDGETS='noop 100 passthrough 100 +posttool 100 hook 100 wired 100' diff --git a/mise-tasks/perf-compare.sh b/mise-tasks/perf-compare.sh index f51b7e57e..44351d3d3 100755 --- a/mise-tasks/perf-compare.sh +++ b/mise-tasks/perf-compare.sh @@ -77,7 +77,23 @@ NOISE_FLOOR="1.102" # measured null maximum, n=30; reported, never compared agai # # EXEMPT ROWS ARE REPORTED ON EVERY RUN, never silent. An accepted regression # that stops being visible is a raised threshold with extra steps. -EXEMPT="wired|1.60|2026-11-30|CLOUD-843: a migrated gate compiles per mediated call. Measured: the same predicate against CLOUD-857's projected input.call.segments is 1.17x, so this expires with that row rather than standing on its own" +# The `posttool` row is the ONE case where the pairing does not measure a +# regression at all, and the ratio says so: 1.806x, far past a noise floor of +# 1.102x and not remotely a slowdown of the same work. The base binary performs +# NO capture on a post-tool call — CLOUD-919 is what adds one — so the arm's two +# sides are "no write" and "an fsync'd small-file write plus a provenance append", +# and dividing one by the other prices the FEATURE rather than its cost drifting. +# The number is still worth having and still gated: `perf-assert` holds the arm +# to the absolute 100ms budget, where it sits two orders of magnitude inside, and +# `mem:core`'s own reading is that an absolute ceiling is the honest instrument +# for a path whose baseline does not exist yet. +# +# It expires on the same terms every other row does, and the expiry is the point: +# once this lands, `main` carries a binary that captures, so the next branch's +# pairing compares write to write and the ordinary 1.30 applies. A row left +# standing past that would be exempting a real regression. +EXEMPT="posttool|2.20|2026-11-30|CLOUD-919 adds a per-call capture write the base binary does not perform, so this arm's pairing measures write-vs-no-write rather than a regression. Measured 1.806x on one container; perf-assert still holds it to the absolute 100ms budget. Lapses once main carries a capturing binary and the pairing compares like with like +wired|1.60|2026-11-30|CLOUD-843: a migrated gate compiles per mediated call. Measured: the same predicate against CLOUD-857's projected input.call.segments is 1.17x, so this expires with that row rather than standing on its own" # Injectable so the lapse case is testable without waiting for a date to pass — # the shape `in-progress-drain` uses for `WIP_DRAIN_TODAY`. diff --git a/mise-tasks/perf-pair.sh b/mise-tasks/perf-pair.sh index 649e64c8c..e82cfe8f3 100755 --- a/mise-tasks/perf-pair.sh +++ b/mise-tasks/perf-pair.sh @@ -171,6 +171,22 @@ readonly FIXTURE_HOOK="$PWD/crates/batten/tests/fixtures/hooks/claude-code.json" # A read, which the pinned fixture's config does not select — the shape match-all # newly delivers, and the one `perf-assert` budgets as `passthrough` (CLOUD-777). readonly FIXTURE_PASSTHROUGH="$PWD/crates/batten/tests/fixtures/hooks/claude-code-passthrough.json" +# The post-tool payload (CLOUD-919). See `perf.sh` for why this arm exists and why +# its fixture is the size it is. +readonly FIXTURE_POSTTOOL="$PWD/crates/batten/tests/fixtures/hooks/claude-code-posttool.json" + +# A HERMETIC STATE ROOT, and MANDATORY here rather than merely tidy. The post-tool +# arm writes, and this task runs the BASE binary and the HEAD binary against one +# tree: the base stores nothing and the head stores a capture, so a shared ambient +# root would leave the store in a state that depends on which arm ran first — +# making the ratio a fact about ordering rather than about the diff. +# +# This is the floor, not the whole of it: it keeps a developer's real store out of +# the measurement, and the non-writing arms need nothing more. The writing arm +# needs the arms SEPARATED as well, which `pair` does per arm below. +export XDG_DATA_HOME="$OUT_DIR/state" +export APPDATA="$OUT_DIR/state" +export LOCALAPPDATA="$OUT_DIR/state" check_repo="$OUT_DIR/check-repo" mkdir -p "$check_repo" cp "$FIXTURE_REPO/batten.toml.in" "$check_repo/batten.toml" @@ -199,6 +215,31 @@ pair() { case "$id" in hook | wired) flags+=(--input "$FIXTURE_HOOK") ;; passthrough) flags+=(--input "$FIXTURE_PASSTHROUGH") ;; + posttool) + flags+=(--input "$FIXTURE_POSTTOOL") + # ONE STATE ROOT PER ARM, AND EMPTY BEFORE EVERY RUN. This is the only + # arm whose binaries WRITE, and both would otherwise share the store: the + # head arm's capture would then be a blob the base arm's run already + # created, and the log both arms read would carry the other's rows. Two + # order-dependencies at once — dedup hitting on one arm and missing on the + # other, and a log that grows across runs so a later run pays more than an + # earlier one. Neither divides out of a ratio, which is the one thing + # `perf-compare` reads. + # + # `env` rather than an export, for the reason the `wired` arms already + # state: `--shell=none` runs argv directly, so a per-arm environment can + # only be supplied by prefixing the command. The extra exec is identical + # on both arms and divides out. + # + # `--prepare` runs before EVERY timing run of EVERY command, so each + # measured invocation starts from an empty store — the state the arm means + # to price. It is `rm` as argv, not a shell line, because `--shell=none` + # governs the prepare command too. + local base_state="$OUT_DIR/state-base" head_state="$OUT_DIR/state-head" + flags+=(--prepare "rm -rf $base_state $head_state") + base_cmd="env XDG_DATA_HOME=$base_state APPDATA=$base_state LOCALAPPDATA=$base_state $base_cmd" + head_cmd="env XDG_DATA_HOME=$head_state APPDATA=$head_state LOCALAPPDATA=$head_state $head_cmd" + ;; esac if ! (cd "$dir" && hyperfine "${flags[@]}" "$base_cmd" "$head_cmd" >/dev/null 2>"$OUT_DIR/$id.err"); then @@ -244,6 +285,11 @@ pair hook "$check_repo" "$base_bin hook --harness claude-code" "$head_bin hook - # — and `perf-assert` budgets it, which is what this file's own census enforces. pair passthrough "$check_repo" "$base_bin hook --harness claude-code" "$head_bin hook --harness claude-code" +# The POST-TOOL path (CLOUD-919). This is the arm that prices the per-call capture +# write, and the one where the base and head binaries genuinely differ in what they +# do rather than only in how fast they do it. +pair posttool "$check_repo" "$base_bin hook --harness claude-code" "$head_bin hook --harness claude-code" + # THE WIRED PATH (CLOUD-697): what `.claude/settings.json` actually invokes — # the number an agent waits on, and the one `perf-assert` budgets but no paired # arm covered. diff --git a/mise-tasks/perf.sh b/mise-tasks/perf.sh index f5effec5c..d80addf11 100755 --- a/mise-tasks/perf.sh +++ b/mise-tasks/perf.sh @@ -104,7 +104,30 @@ readonly FIXTURE_REPO="crates/batten/tests/fixtures/repos/forbid-clean" readonly FIXTURE_HOOK="crates/batten/tests/fixtures/hooks/claude-code.json" # A read, which no committed rule selects — the shape match-all newly delivers. readonly FIXTURE_PASSTHROUGH="crates/batten/tests/fixtures/hooks/claude-code-passthrough.json" -for f in "$FIXTURE_REPO/batten.toml.in" "$FIXTURE_REPO/lib.rs.in" "$FIXTURE_HOOK"; do +# THE POST-TOOL PATH (CLOUD-919), and the arm this task did not have. Every other +# hook arm feeds a PreToolUse payload, so the per-call write CLOUD-919 adds — on +# every post-tool event, therefore on every tool call — was invisible to `perf`, +# `perf-compare`, `perf-pair` and `perf-assert` alike. Running `perf-gate` proved +# nothing about it. +# +# THE FIXTURE'S SIZE IS ITSELF A MEASUREMENT CHOICE. ~1 KiB of `stdout`, because +# the cost being priced is dominated by the FIXED terms — resolving the repo root, +# `create_dir_all`, open, `write_all`, `sync_all`, `rename`, and one append to the +# call log — rather than by the byte count. One page keeps this measuring the write +# PATH instead of a copy. Twenty bytes would hide any per-byte term and would +# misrepresent a real Bash response, which is a command's stdout. A megabyte would +# measure `memcpy` and the spill logic, which is a SECOND arm rather than a bigger +# fixture — said out loud so this file does not grow one by accretion. +# +# An OBJECT response, not a content-block array: it is the shape the hot path +# actually carries, and it is also the shape `facts::rows_in` cannot read, so this +# arm exercises the path the capture-before-projection ordering is about. +readonly FIXTURE_POSTTOOL="crates/batten/tests/fixtures/hooks/claude-code-posttool.json" +# `FIXTURE_PASSTHROUGH` joins the loop too. Its absence from it was a pre-existing +# gap: a missing passthrough fixture failed late, inside hyperfine, rather than +# here with the message this loop exists to give. +for f in "$FIXTURE_REPO/batten.toml.in" "$FIXTURE_REPO/lib.rs.in" "$FIXTURE_HOOK" \ + "$FIXTURE_PASSTHROUGH" "$FIXTURE_POSTTOOL"; do if [[ ! -f "$f" ]]; then echo "::error:: perf: fixture $f is missing, so the measured input is not the one the test suite pins." >&2 exit 1 @@ -121,6 +144,17 @@ cp "$FIXTURE_REPO/lib.rs.in" "$CHECK_REPO/lib.rs" readonly ABS_BIN="$PWD/$BIN" readonly ABS_HOOK="$PWD/$FIXTURE_HOOK" readonly ABS_PASSTHROUGH="$PWD/$FIXTURE_PASSTHROUGH" +readonly ABS_POSTTOOL="$PWD/$FIXTURE_POSTTOOL" + +# A HERMETIC STATE ROOT for every arm, because the post-tool arm WRITES. Two +# reasons, and the second is what makes it apply to all of them rather than one: +# a benchmark must not deposit captures in a developer's real store, and every arm +# has to pay the same environment or the post-tool figure is not comparable with +# `hook`'s. Exported rather than prefixed onto one arm, because adding an `env` +# exec to a single arm would show up in that arm's absolute number. +export XDG_DATA_HOME="$OUT_DIR/state" +export APPDATA="$OUT_DIR/state" +export LOCALAPPDATA="$OUT_DIR/state" # One record per path, in a fixed order so two runs of this task differ only in # their numbers — byte-stable output, house style §6. @@ -144,6 +178,7 @@ measure() { case "$id" in hook | wired) args+=(--input "$ABS_HOOK") ;; passthrough) args+=(--input "$ABS_PASSTHROUGH") ;; + posttool) args+=(--input "$ABS_POSTTOOL") ;; esac # Guarded, not chained: a failed measurement must not fall through to the @@ -193,6 +228,16 @@ measure hook "$PWD" "$ABS_BIN hook --harness claude-code" # policy it never ran would show up here and nowhere else. measure passthrough "$PWD" "$ABS_BIN hook --harness claude-code" +# The POST-TOOL path (CLOUD-919). Same argv as `hook` and `passthrough`; the arm +# differs only in which fixture is piped in, which is what isolates the cost of +# the response capture from everything else the engine does. +# +# `posttool`, never `post_tool`: four regexes in `tests/perf-pair.bats` match arm +# ids with `[a-z]+`, and an underscore would make the budget-row count AND the +# pair-line count both miss the new arm — leaving `paired == budgeted + 1` true +# and the census test passing over the very hole it exists to catch. +measure posttool "$PWD" "$ABS_BIN hook --harness claude-code" + # The path that is actually WIRED, which is the one an agent waits on # (CLOUD-435). `hook` above measures the binary; this measures what # `.claude/settings.json` invokes, launcher and all — and the gap between them diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 14d81e565..fbd741fe7 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -26,6 +26,17 @@ } ] }, + "capture": { + "description": "The bound on RESPONSE captures (CLOUD-918). Absent means the engine\ndefaults.\n\n**`exec` captures stay unbounded and unchanged**, so the consumer that has\nthis behaviour today keeps it: `prune` remains the whole lifecycle there,\nand that store is bounded by how many *distinct* outputs a repository\nproduces, because identical bytes are one record. A bound exists for\nresponses because response capture changes the growth law — per call rather\nthan per distinct output — and inheriting `exec`'s posture into that would\nbe adopting a bound computed for a different denominator.\n\nThe type and the eviction are [`crate::capture`].", + "anyOf": [ + { + "$ref": "#/$defs/CaptureConfig" + }, + { + "type": "null" + } + ] + }, "ci": { "description": "The merge contract this repository commits to (CLOUD-54), **derived**\nfrom the host ruleset. Absent means the contract is not projected here;\npresent, it is what `config lint --host-rules` compares against. The host\nis always the authority — this is a copy a gate polices, never a second\nplace the fact is decided.", "anyOf": [ @@ -443,6 +454,31 @@ "max_tokens" ] }, + "CaptureConfig": { + "description": "The bound on response captures (CLOUD-918).\n\nEnforced **at write time**, and the trigger is the write — never a clock,\nnever a schedule, never a background sweeper. That is\n`.claude/rules/toolchain.md`'s split between a gate and a schedule, and\n[`prune`]'s own doc already refuses a time-based sweeper on the same grounds.", + "type": "object", + "properties": { + "max_bytes": { + "description": "Total response-capture bytes the store may hold. Absent means the default.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + }, + "max_records": { + "description": "Response-capture records the store may hold. Absent means the default.", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "additionalProperties": false + }, "Ci": { "description": "The `[ci]` table: this repository's committed copy of the host's contract.", "type": "object", diff --git a/tests/perf-assert.bats b/tests/perf-assert.bats index 542b5add8..436c13d27 100644 --- a/tests/perf-assert.bats +++ b/tests/perf-assert.bats @@ -24,6 +24,7 @@ setup() { | `passthrough` | 2.8 ms | 3.4 ms | ≤ 100 ms | | `check` | 2.5 ms | 3.2 ms | — | | `hook` | 2.7 ms | 3.5 ms | ≤ 100 ms | + | `posttool` | 3.0 ms | 3.8 ms | ≤ 100 ms | | `wired` | 3.4 ms | 4.1 ms | ≤ 100 ms | EOF } @@ -35,6 +36,7 @@ green_records() { path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 path=check p50=2.54 p95=3.17 mean=2.64 runs=100 path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 + path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 EOF } @@ -60,6 +62,7 @@ path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 path=check p50=2.54 p95=3.17 mean=2.64 runs=100 path=hook p50=90.1 p95=140.5 mean=95.2 runs=100 +path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 IN" [ "$status" -eq 1 ] @@ -77,6 +80,7 @@ path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 path=check p50=800.0 p95=1200.0 mean=850.0 runs=100 path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 +path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 IN" [ "$status" -eq 0 ] @@ -171,6 +175,7 @@ path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 path=check p50=2.54 p95=3.17 mean=2.64 runs=100 path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 +path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 path=wired p50=90.0 p95=140.5 mean=95.0 runs=100 IN" [ "$status" -eq 1 ] @@ -188,11 +193,43 @@ path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 path=check p50=2.54 p95=3.17 mean=2.64 runs=100 path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 +path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 IN" [ "$status" -eq 2 ] [[ "$output" == *"wired"* ]] } +# --- the post-tool path (CLOUD-919) ------------------------------------------ +# +# The arm that prices the response capture. Its presence gate is the whole +# reason the arm is non-optional: a `perf` run that stops emitting it must read +# as could-not-look, never as "the capture costs nothing". +@test "a missing posttool record is exit 2 — the capture cost cannot go unmeasured" { + run bash -c "'$GATE' '$README' <<'IN' +path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 +path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 +path=check p50=2.54 p95=3.17 mean=2.64 runs=100 +path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 +path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 +IN" + [ "$status" -eq 2 ] + [[ "$output" == *"posttool"* ]] +} + +@test "a posttool path over budget is named, with its measurement and its ceiling" { + run bash -c "'$GATE' '$README' <<'IN' +path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 +path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 +path=check p50=2.54 p95=3.17 mean=2.64 runs=100 +path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 +path=posttool p50=90.0 p95=150.5 mean=95.0 runs=100 +path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 +IN" + [ "$status" -eq 1 ] + [[ "$output" == *"posttool"* ]] + [[ "$output" == *"150.5"* ]] +} + @test "a README with no wired row fails, so the budget cannot be enforced unpublished" { published="$BATS_TEST_TMPDIR/no-wired.md" grep -v '`wired`' "$README" >"$published" diff --git a/tests/perf-pair.bats b/tests/perf-pair.bats index 4c9446a20..71ede5a95 100644 --- a/tests/perf-pair.bats +++ b/tests/perf-pair.bats @@ -29,16 +29,17 @@ setup() { } @test "every arm is measured in the pinned fixture repo" { - # Five paths, each pointing at the same materialised fixture. `wired` joins + # Six paths, each pointing at the same materialised fixture. `wired` joins # them (CLOUD-697): its two arms differ in which WIRING runs, not in the - # directory hyperfine is invoked from. `passthrough` joined with CLOUD-777. + # directory hyperfine is invoked from. `passthrough` joined with CLOUD-777, + # `posttool` with CLOUD-919. # # The `env -C` those arms carry (CLOUD-824) is not an exception to this. It # sets the CHILD's cwd so each binary reads its own tree's `batten.toml` — # exactly what the deleted launcher's `cd` did, and what the case above # protects — while hyperfine itself is still invoked from the pinned fixture. run bash -c "grep -cE '^pair [a-z]+ .*\\\$check_repo' '$TASK'" - [ "$output" -eq 5 ] + [ "$output" -eq 6 ] } # THE GAP THIS CLOSES (CLOUD-697). `perf-assert` budgets four paths; this task @@ -53,7 +54,7 @@ setup() { # loses it — which this case caught on its first run. budgeted=$(sed -n "/^BUDGETS='/,/'$/p" "$BATS_TEST_DIRNAME/../mise-tasks/perf-assert.sh" | tr -d "'" | sed 's/^BUDGETS=//' | grep -cE '^[a-z]+ [0-9]+$') - paired=$(grep -cE '^pair (noop|check|hook|wired|passthrough) ' "$TASK") + paired=$(grep -cE '^pair (noop|check|hook|wired|passthrough|posttool) ' "$TASK") # perf-assert budgets the gated paths only; `check` is measured and ungated, # so the paired set is the budgeted set plus it. [ "$paired" -eq $((budgeted + 1)) ]