From a78e783c3b4f38ba24ca15289554210ac59d9c21 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 11:17:14 +0200 Subject: [PATCH 1/7] hive-forge: 10 behaviour tests for lint's pure helpers, which had none WIP commit so the mutation run below has a clean base to restore to. --- hive-forge/src/verbs/lint.rs | 160 +++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/hive-forge/src/verbs/lint.rs b/hive-forge/src/verbs/lint.rs index 86046482..4ba7a0d4 100644 --- a/hive-forge/src/verbs/lint.rs +++ b/hive-forge/src/verbs/lint.rs @@ -497,6 +497,166 @@ fn num(it: &Value) -> i64 { it.get("number").and_then(Value::as_i64).unwrap_or(0) } +#[cfg(test)] +mod tests { + use super::*; + + /// Every field on the forgejo structs is `Option`, so a fixture names + /// only what a case is about. Going through `from_value` rather than a + /// struct literal means these exercise the same deserialisation a real + /// API response does — a wire-shape change breaks the tests too. + fn issue(v: Value) -> Issue { + serde_json::from_value(v).expect("fixture is a valid Issue") + } + + fn pull(v: Value) -> PullRequest { + serde_json::from_value(v).expect("fixture is a valid PullRequest") + } + + /// `exclusive` is Forgejo's real scoped-label marker. A label that + /// merely *looks* scoped (a `/` in a plain label's name) must not + /// count, or `lint unlabeled` silently stops reporting an item that + /// genuinely has no `type/*`. + #[test] + fn only_an_exclusive_label_counts_as_scoped() { + let scoped = issue(json!({ "labels": [{ "name": "area/ops", "exclusive": true }] })); + assert!(has_scoped_label(&scoped, "area/")); + + let lookalike = issue(json!({ "labels": [{ "name": "area/ops", "exclusive": false }] })); + assert!( + !has_scoped_label(&lookalike, "area/"), + "a non-exclusive label with a slash in it is not a scoped label" + ); + + let unset = issue(json!({ "labels": [{ "name": "area/ops" }] })); + assert!(!has_scoped_label(&unset, "area/"), "absent means not exclusive"); + } + + /// The caller builds the prefix as `"/"`, which is what stops + /// `--scope area` from matching a label called `areaology`. + #[test] + fn the_scope_prefix_stops_at_the_slash() { + let other = issue(json!({ "labels": [{ "name": "areaology", "exclusive": true }] })); + assert!(!has_scoped_label(&other, "area/")); + + let wrong_scope = issue(json!({ "labels": [{ "name": "type/bug", "exclusive": true }] })); + assert!(!has_scoped_label(&wrong_scope, "area/")); + assert!(has_scoped_label(&wrong_scope, "type/")); + } + + #[test] + fn an_item_with_no_labels_has_no_scoped_label() { + assert!(!has_scoped_label(&issue(json!({ "labels": [] })), "area/")); + assert!(!has_scoped_label(&issue(json!({})), "area/")); + } + + /// `--reviewer NAME` and a bare `no-reviewer` are different questions: + /// "is X on it" versus "is anyone on it". + #[test] + fn requested_reviewer_asks_a_different_question_with_and_without_a_name() { + let pr = pull(json!({ "requested_reviewers": [{ "login": "argus" }] })); + assert!(has_requested_reviewer(&pr, None), "someone is requested"); + assert!(has_requested_reviewer(&pr, Some("argus"))); + assert!(!has_requested_reviewer(&pr, Some("iris"))); + + let bare = pull(json!({ "requested_reviewers": [] })); + assert!(!has_requested_reviewer(&bare, None)); + assert!(!has_requested_reviewer(&bare, Some("argus"))); + + // `null`, not an omitted key: `requested_reviewers` carries a + // `deserialize_with`, so serde requires it to be present even though + // its type is `Option`. The forge sends the key with a null value. + let absent = pull(json!({ "requested_reviewers": null })); + assert!(!has_requested_reviewer(&absent, None), "null is not a reviewer"); + } + + #[test] + fn logins_drops_users_that_have_none() { + let users: Vec = + serde_json::from_value(json!([{ "login": "atlas" }, {}, { "login": "mara" }])) + .expect("fixture"); + assert_eq!(logins(Some(&users)), vec!["atlas", "mara"]); + assert!(logins(None).is_empty()); + } + + /// Issues and PRs come back from the *same* endpoint; the only thing + /// telling them apart is whether `pull_request` is present. Getting + /// this wrong mislabels every row `lint` prints. + #[test] + fn is_pr_is_decided_by_the_pull_request_object() { + let plain = trim_issue(&issue(json!({ "number": 7, "title": "t" }))); + assert_eq!(plain["is_pr"], json!(false)); + assert_eq!(kind_label(&plain), "issue"); + + let pr = trim_issue(&issue(json!({ + "number": 8, "title": "t", "pull_request": { "merged": false }, + }))); + assert_eq!(pr["is_pr"], json!(true)); + assert_eq!(kind_label(&pr), "pr"); + } + + #[test] + fn trim_issue_keeps_the_fields_the_output_prints() { + let v = trim_issue(&issue(json!({ + "number": 42, + "title": "a title", + "assignees": [{ "login": "atlas" }, { "login": "argus" }], + }))); + assert_eq!(num(&v), 42); + assert_eq!(title(&v), "a title"); + assert_eq!(v["assignees"], json!(["atlas", "argus"])); + } + + #[test] + fn accessors_survive_a_row_that_is_missing_everything() { + let empty = json!({}); + assert_eq!(num(&empty), 0); + assert_eq!(title(&empty), ""); + assert_eq!(kind_label(&empty), "issue"); + } + + /// `All` is **absence** of the filter, not a third value — the forge + /// returns both kinds when `type` is omitted. Turning it into a value + /// would silently narrow every `--type all` query. + #[test] + fn the_both_kinds_filter_is_an_absent_query_param() { + assert!(Kind::All.query_type().is_none()); + assert!(matches!( + Kind::Issues.query_type(), + Some(IssueListIssuesQueryType::Issues) + )); + assert!(matches!( + Kind::Pulls.query_type(), + Some(IssueListIssuesQueryType::Pulls) + )); + } + + #[test] + fn state_maps_onto_both_query_enums() { + assert!(matches!( + State::Open.issue_state(), + IssueListIssuesQueryState::Open + )); + assert!(matches!( + State::Closed.issue_state(), + IssueListIssuesQueryState::Closed + )); + assert!(matches!(State::All.issue_state(), IssueListIssuesQueryState::All)); + assert!(matches!( + State::Open.pull_state(), + RepoListPullRequestsQueryState::Open + )); + assert!(matches!( + State::Closed.pull_state(), + RepoListPullRequestsQueryState::Closed + )); + assert!(matches!( + State::All.pull_state(), + RepoListPullRequestsQueryState::All + )); + } +} + fn title(it: &Value) -> &str { it.get("title").and_then(Value::as_str).unwrap_or("") } From b0507a093651cd1bf76f18c8f4d0a99f59224f21 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 11:22:42 +0200 Subject: [PATCH 2/7] hive-forge: move the lint test module to the end of the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clippy::items_after_test_module (pedantic, denied) — a #[cfg(test)] mod has to be the last item in its file. Pure move: same line count, same line multiset, no content change. `cargo test` was green while this was broken; only clippy saw it. --- hive-forge/src/verbs/lint.rs | 76 ++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/hive-forge/src/verbs/lint.rs b/hive-forge/src/verbs/lint.rs index 4ba7a0d4..4df42ef3 100644 --- a/hive-forge/src/verbs/lint.rs +++ b/hive-forge/src/verbs/lint.rs @@ -497,6 +497,36 @@ fn num(it: &Value) -> i64 { it.get("number").and_then(Value::as_i64).unwrap_or(0) } +fn title(it: &Value) -> &str { + it.get("title").and_then(Value::as_str).unwrap_or("") +} + +fn kind_label(it: &Value) -> &'static str { + if it.get("is_pr").and_then(Value::as_bool).unwrap_or(false) { + "pr" + } else { + "issue" + } +} + +/// Either pretty-print the JSON array (when --json) or fall back to a +/// caller-supplied human one-liner per item. +fn emit(client: &Client, items: &[Value], fmt: F) -> Result<()> +where + F: Fn(&Value) -> String, +{ + if client.json_mode() { + print_json(&Value::Array(items.to_vec())) + } else { + if items.is_empty() { + println!("(no matches)"); + } + for it in items { + println!("{}", fmt(it)); + } + Ok(()) + } +} #[cfg(test)] mod tests { use super::*; @@ -529,7 +559,10 @@ mod tests { ); let unset = issue(json!({ "labels": [{ "name": "area/ops" }] })); - assert!(!has_scoped_label(&unset, "area/"), "absent means not exclusive"); + assert!( + !has_scoped_label(&unset, "area/"), + "absent means not exclusive" + ); } /// The caller builds the prefix as `"/"`, which is what stops @@ -567,7 +600,10 @@ mod tests { // `deserialize_with`, so serde requires it to be present even though // its type is `Option`. The forge sends the key with a null value. let absent = pull(json!({ "requested_reviewers": null })); - assert!(!has_requested_reviewer(&absent, None), "null is not a reviewer"); + assert!( + !has_requested_reviewer(&absent, None), + "null is not a reviewer" + ); } #[test] @@ -641,7 +677,10 @@ mod tests { State::Closed.issue_state(), IssueListIssuesQueryState::Closed )); - assert!(matches!(State::All.issue_state(), IssueListIssuesQueryState::All)); + assert!(matches!( + State::All.issue_state(), + IssueListIssuesQueryState::All + )); assert!(matches!( State::Open.pull_state(), RepoListPullRequestsQueryState::Open @@ -656,34 +695,3 @@ mod tests { )); } } - -fn title(it: &Value) -> &str { - it.get("title").and_then(Value::as_str).unwrap_or("") -} - -fn kind_label(it: &Value) -> &'static str { - if it.get("is_pr").and_then(Value::as_bool).unwrap_or(false) { - "pr" - } else { - "issue" - } -} - -/// Either pretty-print the JSON array (when --json) or fall back to a -/// caller-supplied human one-liner per item. -fn emit(client: &Client, items: &[Value], fmt: F) -> Result<()> -where - F: Fn(&Value) -> String, -{ - if client.json_mode() { - print_json(&Value::Array(items.to_vec())) - } else { - if items.is_empty() { - println!("(no matches)"); - } - for it in items { - println!("{}", fmt(it)); - } - Ok(()) - } -} From 8f56f86df5635cca21d24e3c03fb605ed9a956e1 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 11:42:04 +0200 Subject: [PATCH 3/7] ci: refuse relative markdown links in rust doc comments check-doc-refs.sh's relative arm resolved every link against the linking file's directory. That is right for markdown and wrong for a .rs doc comment: rustdoc emits the href verbatim onto a page under target/doc///, so the link resolves against THAT directory instead. The consequence was a false pass, not a miss -- a link resolving from the source dir got certified while rendering broken in the published docs. Reproduced against main's script: a `](../../docs/README.md)` planted in hive-forge/src/client.rs resolves from hive-forge/src/ and the old rule exits 0. Refused rather than resolved rustdoc-aware. The repo has zero relative links in .rs doc comments (measured on 54daf9ac), so nothing needs the second reader, and the backticked repo-root form the root arm already gates renders correctly under both. Inferring module nesting would be machinery in service of a form nothing uses. Closes #3928 --- scripts/check-doc-refs.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/scripts/check-doc-refs.sh b/scripts/check-doc-refs.sh index b4ead89e..24894438 100755 --- a/scripts/check-doc-refs.sh +++ b/scripts/check-doc-refs.sh @@ -60,6 +60,26 @@ while IFS=: read -r file lineno match; do [ -n "${match:-}" ] || continue exempt "$file" "$lineno" && continue link="${match#](}" + # A `.rs` doc comment has a second renderer, and it does not count `../` + # the way the rest of this arm assumes. rustdoc emits the href verbatim + # onto a page at `target/doc///`, so the link resolves + # against *that* directory, not the source file's. Resolving from the + # source therefore certifies links that render broken — a false pass, not + # a miss, and worse than no check. + # + # Refused rather than resolved rustdoc-aware: the repo has zero relative + # links in `.rs` doc comments (measured on this commit), so nothing needs + # the second reader, and the backticked repo-root form the root arm above + # already gates works under both. Inferring module nesting would be + # machinery in service of a form nothing uses. + case "$file" in + *.rs) + dead=$((dead + 1)) + printf '::error file=%s,line=%s::relative markdown link in a rust doc comment renders against the rustdoc output directory, not this file; use a backticked repo-root pointer instead: %s\n' \ + "$file" "$lineno" "$link" + continue + ;; + esac # Resolved from the linking file's directory, so `../` counts the same way # the renderer counts it. `-m` keeps a link that escapes the repo root # resolvable enough to report rather than erroring out here. From 74806f82a91de76d72cbf5b16f5014b61acbb2e9 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 10:35:44 +0200 Subject: [PATCH 4/7] hive-agent: 25 behaviour tests for stream_enrich, which had none WIP commit so the mutation tests below have a clean base to restore to. --- hive-agent/src/stream_enrich.rs | 378 ++++++++++++++++++++++++++++++++ 1 file changed, 378 insertions(+) diff --git a/hive-agent/src/stream_enrich.rs b/hive-agent/src/stream_enrich.rs index 39921b9d..654ac714 100644 --- a/hive-agent/src/stream_enrich.rs +++ b/hive-agent/src/stream_enrich.rs @@ -1048,3 +1048,381 @@ fn trim_str(s: &str, max: usize) -> String { fn json_str(s: &str) -> String { serde_json::to_string(s).unwrap_or_else(|_| s.to_owned()) } + +#[cfg(test)] +mod tests { + use super::*; + + fn one(v: &Value) -> TermMsg { + let mut ctx = ClassifyCtx::default(); + let rows = classify_stream_value(v, &mut ctx); + assert_eq!(rows.len(), 1, "expected exactly one row for {v}"); + rows.into_iter().next().unwrap() + } + + // ----------------------------------------------------------------- + // dispatch + // ----------------------------------------------------------------- + + #[test] + fn terminal_event_types_render_nothing() { + for t in ["result", "rate_limit_event"] { + let mut ctx = ClassifyCtx::default(); + assert!( + classify_stream_value(&json!({ "type": t }), &mut ctx).is_empty(), + "{t} should be dropped" + ); + } + } + + /// The catch-all exists so a claude event shape nobody anticipated stays + /// *visible* rather than vanishing. Losing it is silent, which is why it + /// gets a test rather than a comment. + #[test] + fn unrecognised_shape_falls_through_loudly() { + let m = one(&json!({ "type": "something_new", "detail": "x" })); + assert_eq!(m.level, Level::Warn); + assert_eq!(m.icon.as_deref(), Some("!")); + assert!(m.summary.contains("something_new")); + } + + #[test] + fn catch_all_truncates_a_huge_payload() { + let m = one(&json!({ "type": "unknown", "blob": "x".repeat(5_000) })); + assert_eq!(m.summary.chars().count(), 201, "200 chars plus the ellipsis"); + assert!(m.summary.ends_with('…')); + } + + /// Task events are matched on `subtype` *regardless of `type`* — a + /// deliberate asymmetry with every other arm, and one a refactor that + /// "tidies" the dispatch into a single match on `type` would silently + /// break. + #[test] + fn task_events_dispatch_on_subtype_not_type() { + let m = one(&json!({ + "type": "assistant", + "subtype": "task_started", + "task_id": "abcdef1234567890", + "description": "do a thing", + })); + assert!(m.summary.starts_with("task abcdef12 started"), "{}", m.summary); + assert!( + !m.summary.contains("1234567890"), + "task id is truncated to 8 chars" + ); + } + + #[test] + fn a_failed_task_notification_is_an_error_row() { + let m = one(&json!({ + "type": "x", "subtype": "task_notification", + "task_id": "t1", "status": "failed", + })); + assert_eq!(m.level, Level::Error); + } + + // ----------------------------------------------------------------- + // system events + // ----------------------------------------------------------------- + + #[test] + fn startup_noise_is_dropped_but_an_unknown_subtype_is_not() { + let mut ctx = ClassifyCtx::default(); + assert!( + classify_stream_value(&json!({ "type": "system", "subtype": "init" }), &mut ctx) + .is_empty() + ); + // The pairing is the point: "init produces nothing" only means + // something if a *different* subtype produces something. + let m = one(&json!({ "type": "system", "subtype": "brand_new" })); + assert_eq!(m.summary, "⚙ brand_new"); + } + + #[test] + fn system_severity_is_carried_by_subtype() { + let cases = [ + ("api_error", Level::Error), + ("api_retry", Level::Warn), + ("status", Level::Debug), + ]; + for (subtype, want) in cases { + let m = one(&json!({ "type": "system", "subtype": subtype })); + assert_eq!(m.level, want, "{subtype}"); + } + } + + /// Coalescing is what stops a heartbeat from flooding the terminal: rows + /// sharing a key collapse into one. A missing key is invisible in every + /// unit except the rendered stream. + #[test] + fn repeating_rows_carry_a_coalesce_key() { + let tick = one(&json!({ "type": "system", "subtype": "status" })); + assert_eq!(tick.coalesce_key.as_deref(), Some("status-tick")); + + let think = one(&json!({ + "type": "system", "subtype": "thinking_tokens", "estimated_tokens": 1234, + })); + assert_eq!(think.coalesce_key.as_deref(), Some("thinking-tok")); + assert_eq!(think.summary, "thinking… ~1234 tokens"); + + // Same subtype, no count: still coalesces, still says something. + let bare = one(&json!({ "type": "system", "subtype": "thinking_tokens" })); + assert_eq!(bare.summary, "thinking…"); + assert_eq!(bare.coalesce_key.as_deref(), Some("thinking-tok")); + } + + #[test] + fn commands_changed_expands_into_the_command_list() { + let m = one(&json!({ + "type": "system", "subtype": "commands_changed", + "commands": [{ "name": "compact" }, { "name": "loop" }], + })); + assert_eq!(m.summary, "⚙ commands changed · 2 available"); + assert_eq!(m.body.as_deref(), Some("/compact\n/loop")); + } + + #[test] + fn api_retry_summarises_every_field_it_was_given() { + let m = one(&json!({ + "type": "system", "subtype": "api_retry", + "attempt": 2, "max_retries": 5, + "error_status": 529, "retry_delay_ms": 1500.4, + })); + assert_eq!(m.summary, "⚠ api retry · 2/5 · HTTP 529 · 1500ms"); + } + + /// `error` wins over `error_status` — the operator gets the message, not + /// the number, whenever there is one. + #[test] + fn api_error_prefers_the_message_over_the_status() { + let with_msg = one(&json!({ + "type": "system", "subtype": "api_error", + "error": "overloaded", "error_status": 529, + })); + assert_eq!(with_msg.summary, "✗ api error · overloaded"); + + let status_only = + one(&json!({ "type": "system", "subtype": "api_error", "error_status": 529 })); + assert_eq!(status_only.summary, "✗ api error · HTTP 529"); + + let neither = one(&json!({ "type": "system", "subtype": "api_error" })); + assert_eq!(neither.summary, "✗ api error · unknown"); + } + + // ----------------------------------------------------------------- + // tool results + // ----------------------------------------------------------------- + + /// The 120-char boundary decides between an inline summary and an + /// expandable body, and it counts *characters*, so a multi-byte result is + /// the case that would break a byte-based rewrite. + #[test] + fn tool_result_summary_boundary_counts_characters() { + let at = "a".repeat(120); + assert_eq!(summarize_tool_result(&at, &at), at); + + let over = "a".repeat(121); + let s = summarize_tool_result(&over, &over); + assert!(s.starts_with("1L · "), "{s}"); + assert!(s.ends_with('…')); + + // 120 non-ASCII chars is 240 bytes: byte-length would call this long. + let wide = "é".repeat(120); + assert_eq!(summarize_tool_result(&wide, &wide), wide); + } + + #[test] + fn an_empty_tool_result_says_so() { + assert_eq!(summarize_tool_result("", ""), "(empty)"); + assert_eq!(summarize_tool_result(" \n\t ", ""), "(empty)"); + } + + #[test] + fn a_long_tool_result_reports_its_line_count() { + let txt = (0..40).map(|i| format!("line {i}\n")).collect::(); + let trimmed: String = txt.split_whitespace().collect::>().join(" "); + assert!(summarize_tool_result(&txt, &trimmed).starts_with("40L · ")); + } + + /// Only a *matched* pair is claude's wrapper. A result that merely starts + /// with the opening tag is real output and must survive intact. + #[test] + fn the_error_wrapper_is_stripped_only_when_balanced() { + assert_eq!( + strip_tool_use_error_wrapper("boom"), + "boom" + ); + assert_eq!( + strip_tool_use_error_wrapper(" boom "), + "boom" + ); + let unbalanced = "boom"; + assert_eq!(strip_tool_use_error_wrapper(unbalanced), unbalanced); + assert_eq!(strip_tool_use_error_wrapper("plain"), "plain"); + } + + fn tool_result(text: &str, is_error: bool, id: &str) -> Value { + json!({ + "type": "user", + "message": { "content": [{ + "type": "tool_result", + "tool_use_id": id, + "is_error": is_error, + "content": [{ "type": "text", "text": text }], + }]}, + }) + } + + #[test] + fn a_short_result_gets_an_icon_and_a_long_one_gets_a_body() { + let short = one(&tool_result("ok", false, "t1")); + assert_eq!(short.icon.as_deref(), Some("←")); + assert!(short.body.is_none(), "short results are not expandable"); + + let long = one(&tool_result(&"x".repeat(500), false, "t1")); + assert!(long.icon.is_none()); + assert_eq!(long.body.as_deref().map(str::len), Some(500)); + } + + #[test] + fn an_error_result_is_an_error_row_with_the_wrapper_gone() { + let m = one(&tool_result( + "file not found", + true, + "t1", + )); + assert_eq!(m.level, Level::Error); + assert_eq!(m.icon.as_deref(), Some("✗")); + assert_eq!(m.summary, "file not found"); + } + + /// The one piece of *cross-event* state in this module: the tool name is + /// learned from the assistant event and read back on the user event that + /// answers it. Nothing but a test spanning both events can catch it + /// breaking — and when it does, an inbox message silently renders as + /// plain text instead of markdown. + #[test] + fn a_recv_result_is_markdown_only_because_the_tool_use_was_seen_first() { + let mut ctx = ClassifyCtx::default(); + let body = "**bold** message from a peer"; + + // Without the correlation, it is an ordinary result. + let cold = classify_stream_value(&tool_result(body, false, "call-1"), &mut ctx); + assert_eq!(cold[0].body_format, None); + assert!(!cold[0].summary.starts_with("recv ←")); + + // Feed the assistant event that names the tool, then the same result. + classify_stream_value( + &json!({ + "type": "assistant", + "message": { "content": [{ + "type": "tool_use", "id": "call-1", + "name": "mcp__hyperhive__recv", "input": {}, + }]}, + }), + &mut ctx, + ); + let warm = classify_stream_value(&tool_result(body, false, "call-1"), &mut ctx); + assert!(warm[0].summary.starts_with("recv ← "), "{}", warm[0].summary); + assert_eq!(warm[0].body_format, Some(BodyFormat::Markdown)); + assert_eq!(warm[0].body.as_deref(), Some(body)); + } + + #[test] + fn a_recv_result_with_no_content_stays_an_ordinary_row() { + let mut ctx = ClassifyCtx::default(); + ctx.record_tool_use("call-1", "mcp__hyperhive__recv"); + let m = one_with(&tool_result(" ", false, "call-1"), &mut ctx); + assert!(!m.summary.starts_with("recv ←")); + assert_eq!(m.summary, "(empty)"); + } + + fn one_with(v: &Value, ctx: &mut ClassifyCtx) -> TermMsg { + let rows = classify_stream_value(v, ctx); + assert_eq!(rows.len(), 1, "expected exactly one row for {v}"); + rows.into_iter().next().unwrap() + } + + // ----------------------------------------------------------------- + // assistant content + // ----------------------------------------------------------------- + + #[test] + fn assistant_text_becomes_a_markdown_body_with_no_summary() { + let m = one(&json!({ + "type": "assistant", + "message": { "content": [{ "type": "text", "text": "# hi" }]}, + })); + assert_eq!(m.summary, ""); + assert_eq!(m.body.as_deref(), Some("# hi")); + assert_eq!(m.body_format, Some(BodyFormat::Markdown)); + } + + #[test] + fn blank_assistant_text_produces_no_row_at_all() { + let mut ctx = ClassifyCtx::default(); + let rows = classify_stream_value( + &json!({ + "type": "assistant", + "message": { "content": [{ "type": "text", "text": " \n " }]}, + }), + &mut ctx, + ); + assert!(rows.is_empty()); + } + + #[test] + fn one_assistant_event_can_produce_several_rows() { + let mut ctx = ClassifyCtx::default(); + let rows = classify_stream_value( + &json!({ + "type": "assistant", + "message": { "content": [ + { "type": "text", "text": "doing it" }, + { "type": "thinking", "thinking": "hmm" }, + { "type": "tool_use", "id": "t1", "name": "Bash", + "input": { "command": "ls" } }, + { "type": "something_else" }, + ]}, + }), + &mut ctx, + ); + assert_eq!(rows.len(), 3, "the unknown content block is skipped"); + assert_eq!(rows[1].icon.as_deref(), Some("💭")); + // The tool_use was recorded for later correlation as a side effect. + assert_eq!(ctx.tool_name("t1"), Some("Bash")); + } + + // ----------------------------------------------------------------- + // string helpers + // ----------------------------------------------------------------- + + #[test] + fn token_counts_shorten_at_each_threshold() { + assert_eq!(fmt_tok(0), "0"); + assert_eq!(fmt_tok(999), "999"); + assert_eq!(fmt_tok(1_000), "1k"); + assert_eq!(fmt_tok(999_999), "999k"); + assert_eq!(fmt_tok(1_000_000), "1.0M"); + assert_eq!(fmt_tok(1_250_000), "1.2M"); + } + + #[test] + fn trim_str_collapses_whitespace_and_counts_characters() { + assert_eq!(trim_str("a b\n\tc", 99), "a b c"); + assert_eq!(trim_str("abcdef", 3), "abc…"); + // Exactly at the limit: no ellipsis. + assert_eq!(trim_str("abc", 3), "abc"); + // 4 chars / 8 bytes — a byte-based limit would cut this at 2. + assert_eq!(trim_str("éééé", 4), "éééé"); + } + + #[test] + fn matrix_ids_shorten_to_something_recognisable() { + assert_eq!(fmt_user("@mara:pr1ma.darkest.space"), "@mara"); + assert_eq!(fmt_user("@mara"), "@mara"); + assert_eq!(fmt_room("#hive-chat:pr1ma.darkest.space"), "#hive-chat"); + assert_eq!(fmt_room("!abcdefghijkl:server"), "!abcdefgh"); + assert_eq!(fmt_room("plain name"), "plain name"); + } +} From ab3013625533e5007a943c67649463f1698fa2eb Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 10:42:33 +0200 Subject: [PATCH 5/7] hive-agent: fmt_room spent a byte offset as a character budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `r.find(':')` returns a BYTE offset; it was being passed to `r.chars().take(colon.min(9))` as a CHARACTER budget. For a multi-byte room-id local part the two disagree, and the extra characters come out of the server half: fmt_room("!ÄÖÜ:server") == "!ÄÖÜ:se" // want "!ÄÖÜ" The doc comment above it already said the `chars().take()` was there to handle non-ASCII, so the intent was recorded and the implementation was half of it. Split on the colon first, then take 9 characters of the local part. ASCII behaviour is unchanged and pinned by the existing case: `!abcdefghijkl:server` -> `!abcdefgh` before and after. Found by the tests in the previous commit — written red, then fixed. --- hive-agent/src/stream_enrich.rs | 37 +++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/hive-agent/src/stream_enrich.rs b/hive-agent/src/stream_enrich.rs index 654ac714..81724b38 100644 --- a/hive-agent/src/stream_enrich.rs +++ b/hive-agent/src/stream_enrich.rs @@ -1009,9 +1009,11 @@ fn fmt_tok(n: u64) -> String { fn fmt_room(r: &str) -> String { if r.starts_with('!') { // Room id: keep only the local part before the colon (up to 9 chars). - // Use chars().take() so we never slice on a non-ASCII byte boundary. - let colon = r.find(':').unwrap_or(r.len()); - r.chars().take(colon.min(9)).collect() + // Both halves of that are character counts — `find` returns a byte + // offset, and spending it as a character budget lets a multi-byte + // local part buy extra characters from the server half. + let local = r.split(':').next().unwrap_or(r); + local.chars().take(9).collect() } else if r.starts_with('#') { // Alias: keep `#name` part before the server. r.split(':').next().unwrap_or(r).to_owned() @@ -1089,7 +1091,11 @@ mod tests { #[test] fn catch_all_truncates_a_huge_payload() { let m = one(&json!({ "type": "unknown", "blob": "x".repeat(5_000) })); - assert_eq!(m.summary.chars().count(), 201, "200 chars plus the ellipsis"); + assert_eq!( + m.summary.chars().count(), + 201, + "200 chars plus the ellipsis" + ); assert!(m.summary.ends_with('…')); } @@ -1105,7 +1111,11 @@ mod tests { "task_id": "abcdef1234567890", "description": "do a thing", })); - assert!(m.summary.starts_with("task abcdef12 started"), "{}", m.summary); + assert!( + m.summary.starts_with("task abcdef12 started"), + "{}", + m.summary + ); assert!( !m.summary.contains("1234567890"), "task id is truncated to 8 chars" @@ -1323,7 +1333,11 @@ mod tests { &mut ctx, ); let warm = classify_stream_value(&tool_result(body, false, "call-1"), &mut ctx); - assert!(warm[0].summary.starts_with("recv ← "), "{}", warm[0].summary); + assert!( + warm[0].summary.starts_with("recv ← "), + "{}", + warm[0].summary + ); assert_eq!(warm[0].body_format, Some(BodyFormat::Markdown)); assert_eq!(warm[0].body.as_deref(), Some(body)); } @@ -1425,4 +1439,15 @@ mod tests { assert_eq!(fmt_room("!abcdefghijkl:server"), "!abcdefgh"); assert_eq!(fmt_room("plain name"), "plain name"); } + + /// A short room id keeps its whole local part and stops at the colon. + /// The non-ASCII case is the one that used to leak: `find(':')` is a + /// *byte* offset and it was being spent as a *character* budget, so a + /// multi-byte local part bought extra characters from the server half. + #[test] + fn a_room_id_never_shows_part_of_the_server() { + assert_eq!(fmt_room("!short:server"), "!short"); + assert_eq!(fmt_room("!ÄÖÜ:server"), "!ÄÖÜ"); + assert_eq!(fmt_room("!ÄÖÜ"), "!ÄÖÜ"); + } } From 584db1e9c42ada67dbf36aa77e3ababa86c96a57 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 10:43:54 +0200 Subject: [PATCH 6/7] hive-agent: build the multi-line test fixture without format_collect clippy::format_collect (pedantic, denied) fires on `(0..40).map(|i| format!(...)).collect::()`. The test only needs 40 non-empty lines, not numbered ones, so `"line\n".repeat(40)` says the same thing without the lint. --- hive-agent/src/stream_enrich.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hive-agent/src/stream_enrich.rs b/hive-agent/src/stream_enrich.rs index 81724b38..866f2569 100644 --- a/hive-agent/src/stream_enrich.rs +++ b/hive-agent/src/stream_enrich.rs @@ -1249,7 +1249,7 @@ mod tests { #[test] fn a_long_tool_result_reports_its_line_count() { - let txt = (0..40).map(|i| format!("line {i}\n")).collect::(); + let txt = "line\n".repeat(40); let trimmed: String = txt.split_whitespace().collect::>().join(" "); assert!(summarize_tool_result(&txt, &trimmed).starts_with("40L · ")); } From 91ab5f9d9c8afdb422ff3671d398c89a55223d0e Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 11:32:15 +0200 Subject: [PATCH 7/7] hive-agent: pin classify_tool_result's own 120-char boundary Mutation testing found this arm untested: classify_tool_result has its OWN `<= 120` deciding icon-vs-body, separate from the one in summarize_tool_result that picks the summary text. The existing case used 2 and 500 characters, so neither arm went near the boundary and moving it to 119 changed nothing. Verified the new test fails under exactly that mutation. --- hive-agent/src/stream_enrich.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/hive-agent/src/stream_enrich.rs b/hive-agent/src/stream_enrich.rs index 866f2569..9f972a97 100644 --- a/hive-agent/src/stream_enrich.rs +++ b/hive-agent/src/stream_enrich.rs @@ -1283,6 +1283,24 @@ mod tests { }) } + /// `classify_tool_result` has its **own** `<= 120`, separate from the one + /// in `summarize_tool_result`, and the two have to agree: one picks the + /// summary text, the other picks icon-vs-body. A result that summarises as + /// short but renders with a body is the inconsistency this pins. + /// Added because mutation testing found the gap — the case below uses 2 + /// and 500 characters, so neither arm goes near the boundary and moving it + /// to 119 changed nothing. + #[test] + fn the_icon_or_body_choice_turns_on_the_same_120_boundary() { + let at = one(&tool_result(&"a".repeat(120), false, "t1")); + assert_eq!(at.icon.as_deref(), Some("←"), "120 chars is still short"); + assert!(at.body.is_none()); + + let over = one(&tool_result(&"a".repeat(121), false, "t1")); + assert!(over.icon.is_none(), "121 chars stops being short"); + assert_eq!(over.body.as_deref().map(str::len), Some(121)); + } + #[test] fn a_short_result_gets_an_icon_and_a_long_one_gets_a_body() { let short = one(&tool_result("ok", false, "t1"));