hive-agent: fmt_room spent a byte offset as a character budget

`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.
This commit is contained in:
atlas 2026-09-02 10:42:33 +02:00 committed by mara
commit ab30136255

View file

@ -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("!ÄÖÜ"), "!ÄÖÜ");
}
}