fix(facts): normalise a tool buffer to an array instead of refusing it

crates/batten/src/facts.rs

@@ -1536,58 +1536,180 @@ pub fn sourced(record: Option<&Sourced>, asked_for: &str) -> Look<usize> {
     }
 }
 
-/// Reduce a tool result buffer to a row count, or [`Look::CouldNotLook`] if its
-/// shape is not one this build recognises.
-///
-/// The buffer's shape is per-tool and only partly surveyed: an MCP tool returns a
-/// content-block array (measured — `tests/board-write-record.bats`), and a shell
-/// tool returns something this repository has not measured. **Answering `0` for a
-/// shape this build cannot read would be a guessed envelope becoming a silent
-/// fact**, which is the failure the capability table exists to prevent, so an
-/// unrecognised shape is could-not-look instead.
-///
-/// Two shapes are read, both with evidence in this tree: a **bare row array**,
-/// and the **content-block envelope**, whose text blocks are counted where they
-/// parse as JSON arrays — the same `fromjson?` posture `board-write-record` takes
-/// over the same bytes, so a block that is not JSON is skipped rather than
-/// aborting the read.
+/// Reduce a tool result buffer to a row count by **normalising it to an array**,
+/// or [`Look::CouldNotLook`] where no count can honestly be given.
+///
+/// # Why normalise rather than refuse (CLOUD-992)
+///
+/// This function used to read exactly two shapes — a bare row array and the
+/// content-block envelope — and answer could-not-look for everything else. The
+/// reasoning was sound and is preserved below: **answering `0` for a buffer this
+/// build cannot read would be a guessed envelope becoming a silent fact.**
+///
+/// What was wrong is that refusing is not the only way to avoid that, and it made
+/// the channel **unusable from any shell tool**. Measured on real Bash responses:
+/// every one is raw text, so the first match arm rejected it. The channel's first
+/// intended consumer declares a `gh` command, so it would have seen could-not-look
+/// on every call — a gate refusing every `gh pr ready`, unsatisfiable by running
+/// the very command its deny prints.
+///
+/// The alternative considered and rejected was making each declared command
+/// project an array (`gh … --jq '[…]'`). That puts the obligation on every future
+/// fact row, fails as could-not-look rather than loudly when forgotten, and is
+/// invisible in the rule that consumes it.
+///
+/// # The table
+///
+/// | buffer | rows |
+/// | -- | -- |
+/// | JSON array that is not wholly content blocks | its length — an empty one is a genuine zero |
+/// | content-block envelope (EVERY item a content block) | the sum over its blocks, each normalised by the rules below — could-not-look if ANY block says nothing |
+/// | a shell tool's envelope object (`{stdout, stderr}`) | its stream text, normalised by the rules below — the buffer is a MEMBER, never the object |
+/// | any other JSON object, or any non-string scalar | `1` — one element, wrapped |
+/// | text that parses as a JSON array | that array's length |
+/// | text that parses as any other JSON value | `1` |
+/// | text that is not JSON at all | `1` — one opaque row |
+/// | text that is empty or whitespace | could-not-look — see below |
+/// | [`serde_json::Value::Null`] | could-not-look — an absent buffer is not a reading |
+///
+/// **The invariant that must not regress: no unread shape is ever reported as
+/// `0`.** Wrapping preserves it exactly as refusing did — an opaque buffer counts
+/// as one row, never none — so a `rows == 0` predicate stays fail-closed.
+///
+/// **The empty buffer is the one place the three-valued reading survives, and it
+/// earns it.** A command that failed and printed nothing is indistinguishable
+/// from one that legitimately found nothing, so `0` and `1` are both guesses:
+/// `0` would let an unreviewed head through, `1` would deny a gate forever. Only
+/// could-not-look states what is actually known.
+///
+/// A tool whose output cannot be parsed as JSON wants an **adapter** rather than
+/// this fallback — the one-opaque-row reading keeps a gate fail-closed in the
+/// meantime and is inventoried as debt (CLOUD-993), not left as the design.
 ///
 /// No byte of the buffer is returned (rule 4). The count is the whole answer.
 #[must_use]
 pub fn rows_in(result: &serde_json::Value) -> Look<usize> {
-    let serde_json::Value::Array(items) = result else {
-        return Look::CouldNotLook;
-    };
-    let blocks: Vec<&serde_json::Value> = items.iter().filter(|item| is_text_block(item)).collect();
-    if blocks.is_empty() {
-        // A bare array of rows. An EMPTY array reaches here too and is a genuine
-        // zero — it is a shape we read that carried nothing, not a shape we
-        // failed to read.
-        return Look::Is(items.len());
-    }
-    let mut rows = 0;
-    let mut parsed_any = false;
-    for text in blocks
-        .iter()
-        .filter_map(|block| block.get("text").and_then(serde_json::Value::as_str))
-    {
-        if let Ok(serde_json::Value::Array(inner)) = serde_json::from_str::<serde_json::Value>(text)
-        {
-            rows += inner.len();
-            parsed_any = true;
+    match result {
+        // An absent buffer is not a reading. Distinct from an empty one, which
+        // at least says a tool answered.
+        serde_json::Value::Null => Look::CouldNotLook,
+        serde_json::Value::Array(items) => {
+            // ENVELOPE SEMANTICS DEMAND THE WHOLE ARRAY, NOT A MEMBER OF IT. A
+            // row array may legitimately contain a row that happens to be
+            // text-shaped, and treating that array as an envelope reads the one
+            // block and DROPS every other row: `[{"type":"text","text":"[]"},
+            // {"id":1}]` answered `Is(0)` for two rows, breaking the fail-closed
+            // invariant this function exists to hold. So an array is an envelope
+            // only when every item is a content block; anything else is a bare
+            // row array and counts its items.
+            //
+            // An EMPTY array lands here too and is a genuine zero — a shape we
+            // read that carried nothing, not a shape we failed to read.
+            if items.is_empty() || !items.iter().all(is_text_block) {
+                return Look::Is(items.len());
+            }
+            // The content-block envelope an MCP tool returns. Each block's text
+            // goes through the same normalisation a bare text buffer does, so the
+            // two entry points cannot disagree about one string.
+            // NO BLOCK IS SKIPPED, and that is the whole of this loop's contract.
+            // `is_text_block` guarantees a string `text`, so nothing can be
+            // silently filtered out here — a `filter_map` over the field would
+            // drop a block it could not read and then report the SUM OF THE REST
+            // as the buffer's count, which is how a partly-unreadable envelope
+            // came back `Is(0)`.
+            //
+            // And one unreadable block condemns the whole envelope. A block
+            // saying nothing contributes no knowable count, so folding it in as
+            // `0` would be a guess presented as a reading; could-not-look is the
+            // only honest answer for the buffer as a whole. That is also what
+            // keeps the single-empty-block envelope could-not-look, as it has
+            // always been.
+            let mut rows = 0;
+            for block in items {
+                let Some(text) = block.get("text").and_then(serde_json::Value::as_str) else {
+                    return Look::CouldNotLook;
+                };
+                let Some(count) = rows_in_text(text) else {
+                    return Look::CouldNotLook;
+                };
+                rows += count;
+            }
+            Look::Is(rows)
         }
+        serde_json::Value::String(text) => rows_in_text(text).map_or(Look::CouldNotLook, Look::Is),
+        // A SHELL TOOL'S BUFFER IS A MEMBER OF THIS OBJECT, NEVER THE OBJECT.
+        // This is the arm the whole capability turned on and the one a shape
+        // table could not see: Claude Code hands a Bash call's response back as
+        // `{stdout, stderr, …}`, so counting the object gives `1` for every
+        // shell command ever declared — measured, `printf '[1,2,3]'` recorded
+        // `rows 1`. Normalising buffers was necessary and not sufficient,
+        // because the buffer never arrived as one.
+        //
+        // `capture::decode_response` is the ONE authority on that shape. It
+        // already reads `stdout` then `stderr` in a declared order, already
+        // says so against the measured corpus, and is already the reader the
+        // capture store trusts. A second copy of the field list here is the
+        // "two copies drifted" failure this repository keeps recording — so
+        // this defers to it rather than restating it.
+        serde_json::Value::Object(_) => match crate::capture::decode_response(result) {
+            // A shell envelope: count what the tool actually printed.
+            Ok(decoded) if decoded.blocks > 0 => match std::str::from_utf8(&decoded.bytes) {
+                Ok(text) => rows_in_text(text).map_or(Look::CouldNotLook, Look::Is),
+                // Bytes that are not text are a shape this build did not read,
+                // never a buffer that carried nothing.
+                Err(_) => Look::CouldNotLook,
+            },
+            // An object carrying no readable stream member is not an envelope at
+            // all — it is a single JSON row, which is one element wrapped.
+            Ok(_) | Err(_) => Look::Is(1),
+        },
+        // A single number or boolean is one element. Wrapping it is what makes
+        // the count obvious rather than making the caller project.
+        serde_json::Value::Number(_) | serde_json::Value::Bool(_) => Look::Is(1),
     }
-    // Nothing parsed is NOT "zero rows": it is a shape this build did not
-    // understand, which is could not look.
-    if parsed_any {
-        Look::Is(rows)
-    } else {
-        Look::CouldNotLook
+}
+
+/// How many rows one text buffer carries, by the table on [`rows_in`], or
+/// `None` when it says nothing at all.
+///
+/// Split out because the bare-string arm and every text block inside a
+/// content-block envelope must answer identically — a second copy of this rule
+/// is how the two entry points come to disagree about the same string.
+///
+/// `Option<usize>` rather than [`Look<usize>`]: a count has only two outcomes
+/// here, and `Look`'s third — [`Look::IsNot`] — is not one of them. Returning
+/// `Look` would put an arm on every caller that no input can reach, and a
+/// wildcard over it is exactly how a future third outcome would be silently
+/// folded into "nothing to add".
+fn rows_in_text(text: &str) -> Option<usize> {
+    if text.trim().is_empty() {
+        // Nothing was said. Neither `0` nor `1` is knowable here; see the doc.
+        return None;
+    }
+    match serde_json::from_str::<serde_json::Value>(text) {
+        Ok(serde_json::Value::Array(inner)) => Some(inner.len()),
+        // Any other JSON value is one element; prose that does not parse is one
+        // opaque row. Both are "we saw something", which is what matters.
+        Ok(_) | Err(_) => Some(1),
     }
 }
 
+/// Whether one array item is a content block this build can read in full.
+///
+/// **A STRING `text` IS PART OF THE SHAPE, not a detail the reader checks
+/// later.** Keying only on `type` admitted `{"type":"text","text":7}` as a
+/// block, and the envelope loop then had to cope with a block it could not
+/// read — which it did by skipping it, so
+/// `[{"type":"text","text":"[]"},{"type":"text","text":7}]` answered `Is(0)`
+/// for a buffer half of which was never read.
+///
+/// Requiring the string here fixes it in the one place that decides, and it
+/// decides in the more useful direction: a malformed block is not a content
+/// block, so its array is not an envelope at all and counts as ordinary rows.
+/// Two rows is what that buffer carries, and saying so beats refusing it.
 fn is_text_block(value: &serde_json::Value) -> bool {
     value.get("type").and_then(serde_json::Value::as_str) == Some("text")
+        && value.get("text").is_some_and(serde_json::Value::is_string)
 }
 
 /// One agent-sourced fact a consumer declares: its name, and the command whose

crates/batten/src/lib.rs

@@ -3012,10 +3012,13 @@ fn drain_advisories(
 /// reason work stops: the next attempt denies again with the same `Fix::Run`,
 /// which is the safe direction and one the agent can see.
 fn record_agent_fact(overrides: &Overrides, envelope: &hook::Envelope) {
-    // An unrecognised buffer shape is `CouldNotLook` and records NOTHING. Writing
-    // a zero here would turn a shape this build cannot read into the fact "there
-    // are none", which is the guessed-envelope failure the whole capability table
-    // exists to prevent.
+    // A buffer that said nothing at all — absent, or empty — is `CouldNotLook`
+    // and records NOTHING. Writing a zero here would turn "the tool printed
+    // nothing" into the fact "there are none", which is the guessed-envelope
+    // failure the whole capability table exists to prevent. Every buffer that
+    // DID say something is a count now (CLOUD-992), down to one opaque row for
+    // prose that is not JSON, so the declared command no longer has to project
+    // its own output into an array to be readable here.
     let facts::Look::Is(rows) = facts::rows_in(&envelope.result) else {
         return;
     };