diff --git a/hive-c0re/src/matrix.rs b/hive-c0re/src/matrix.rs index ed019656..84e26d07 100644 --- a/hive-c0re/src/matrix.rs +++ b/hive-c0re/src/matrix.rs @@ -1218,75 +1218,8 @@ async fn find_chat_room_by_name(client: &reqwest::Client, admin_token: &str) -> None } -/// Whether the state event still has to be written. -/// -/// `current` is the room's existing content for this `(type, state_key)`, -/// or `None` when it could not be read. **Unreadable means write**: the -/// re-apply exists so a missing link gets repaired, and one redundant -/// event is cheaper than a hierarchy that never reconverges. -/// -/// Comparison is `serde_json::Value` equality, which compares objects as -/// maps — key order in the response does not matter. -fn state_needs_write(current: Option<&serde_json::Value>, desired: &serde_json::Value) -> bool { - current != Some(desired) -} - -/// Read the room's current content for one state event. `None` on any -/// failure — absent state, transport error, or an unparseable body are -/// all "we don't know", and [`state_needs_write`] turns that into a write. -/// -/// One failure is louder than the rest, and it is the only one that hides: -/// **a non-404 HTTP failure means the read broke while the write still -/// works**, so the guard degrades to writing every time and the sweep -/// resumes waking the hive with nothing else to show for it. A transport -/// error needs no warning of its own — the PUT immediately after it fails -/// too, loudly — and a 404 is the expected first-setup case. -async fn current_room_state( - client: &reqwest::Client, - admin_token: &str, - url: &str, -) -> Option { - let resp = match client.get(url).bearer_auth(admin_token).send().await { - Ok(resp) => resp, - Err(e) => { - tracing::debug!(error = ?e, url, "matrix: state read unreachable; writing"); - return None; - } - }; - let status = resp.status(); - if !status.is_success() { - if status != reqwest::StatusCode::NOT_FOUND { - tracing::warn!( - %status, - url, - "matrix: state read failed while writes still work — the \ - skip-if-unchanged guard is off and every sweep will re-emit" - ); - } - return None; - } - match resp.json::().await { - Ok(body) => Some(body), - Err(e) => { - tracing::debug!(error = ?e, url, "matrix: state read body unparseable; writing"); - None - } - } -} - -/// PUT a state event into `room_id` using the admin token, **skipping the -/// write when the room already carries identical content**. -/// -/// The read is not an optimisation. A PUT of identical content is a no-op -/// on the room's *state*, and the homeserver still appends an event to the -/// *timeline* — so "idempotent" was true one level too high. Downstream, -/// an event is unread activity, which is a todo, which is a turn: a caller -/// re-applying a link on a periodic sweep wakes every agent in the room on -/// that sweep's cadence, forever. Measured at ~30 minutes per wake per -/// agent before this guard existed. -/// -/// The self-healing property the re-apply exists for is unaffected: a -/// missing or divergent link still gets written. +/// PUT a state event into `room_id` using the admin token. Idempotent — +/// re-sending identical content is a no-op on the homeserver. async fn set_room_state( client: &reqwest::Client, admin_token: &str, @@ -1300,15 +1233,6 @@ async fn set_room_state( let encoded_key = encode_room_id_for_url(state_key); let url = format!("{base}/_matrix/client/v3/rooms/{encoded_room}/state/{event_type}/{encoded_key}"); - let current = current_room_state(client, admin_token, &url).await; - if !state_needs_write(current.as_ref(), content) { - tracing::debug!( - %room_id, - event_type, - "matrix: state already current, skipping PUT (no timeline event)" - ); - return Ok(()); - } let resp = client .put(&url) .bearer_auth(admin_token) @@ -1337,9 +1261,8 @@ async fn set_room_state( /// join instead of an empty Space. /// /// Dedup mirrors [`ensure_hive_space`]: persisted id wins, else rediscover -/// by name, else create. The space-child link is re-checked on every call -/// so a recovered room reconverges its hierarchy link — and written only -/// when it differs, see [`set_room_state`]. +/// by name, else create. The space-child link is re-applied on every call +/// (idempotent PUT) so a recovered room reconverges its hierarchy link. /// /// # Errors /// @@ -1424,9 +1347,9 @@ pub async fn ensure_hive_chat_room( id }; - // Wire the Space → room child link. Without an `m.space.child` carrying - // a `via`, the room won't surface in the Space hierarchy. `suggested` - // hints clients to surface it prominently. + // Wire the Space → room child link (idempotent). Without an + // `m.space.child` carrying a `via`, the room won't surface in the Space + // hierarchy. `suggested` hints clients to surface it prominently. let child_content = serde_json::json!({ "via": [server_name], "suggested": true, @@ -1777,43 +1700,6 @@ mod tests { assert!(h.chars().all(|c| c.is_ascii_hexdigit())); } - /// The steady state. This is the whole point of the guard: the sweep - /// runs forever, and every write it makes is a wake for every agent - /// in the room. - #[test] - fn state_matching_the_room_is_not_rewritten() { - let desired = serde_json::json!({ "via": ["example.org"], "suggested": true }); - let current = desired.clone(); - assert!(!state_needs_write(Some(¤t), &desired)); - } - - /// Key order in the homeserver's response must not force a write — - /// the guard leans on `serde_json::Value` comparing objects as maps, - /// and a byte- or order-sensitive comparison would silently degrade - /// to writing every time while still looking correct. - #[test] - fn state_matching_but_reordered_is_not_rewritten() { - let desired = serde_json::json!({ "via": ["example.org"], "suggested": true }); - let current = serde_json::json!({ "suggested": true, "via": ["example.org"] }); - assert!(!state_needs_write(Some(¤t), &desired)); - } - - #[test] - fn diverged_state_is_rewritten() { - let desired = serde_json::json!({ "via": ["example.org"], "suggested": true }); - let current = serde_json::json!({ "via": ["old.example.org"], "suggested": true }); - assert!(state_needs_write(Some(¤t), &desired)); - } - - /// Fail-open, and deliberately so: an unreadable current state must - /// write. The re-apply exists to repair a missing link, so "we don't - /// know" has to behave like "it's missing", not like "it's fine". - #[test] - fn unknown_state_is_written() { - let desired = serde_json::json!({ "via": ["example.org"], "suggested": true }); - assert!(state_needs_write(None, &desired)); - } - #[test] fn random_hex_two_calls_differ() { // Sanity check — not a statistical claim, just guards