Every test read: tests that could not fail now can, three real faults they were hiding are fixed, and tests of nothing are gone

All 993 tests were read for whether they test the code that runs. What was found:

Tests that tested a copy. Nine checked logic written out inside the test body, or a function that exists only for tests, so they passed whatever the product did. The logic is now a function the product calls and the test calls the same one: the IRC CAP LS multiline reading (connect.rs - and the product read the continuation star in one field only, which is the fault the test's own story said was fixed), a rank change's wording (incoming.rs), Discord's Retry-After wait (http.rs), Matrix's held unreadable messages and their cap (relock.rs). Two more tested helpers compiled only for tests - the Matrix call offering rule, which lives in the window and now has its own test there, and the DM-list removal that no product path performs; both are deleted. A metadata capability test now checks the real list of capabilities asked for.

A test that could not fail. An offer nobody takes "gives up rather than waiting forever" passed when it waited, because its timeout counted as success; replaced with two that can fail. Doing so found that cancelling an outgoing DCC offer nobody had accepted did nothing for the three minutes the offer lasts, since cancel was only looked at after someone connected. Fixed.

The forget-account sweep. Its test found a field by substring, so removing `buffers` or `presence` from the sweep still passed (`restored_buffers`, `matrix_presence`), and it could not see fields written `std::collections::HashSet`. Six sets - wants_connected, discord_gated, irc_monitor, replaying, silenced, irc_rostered - were never swept; a removed account kept its silenced buffers and its MONITOR answer. Swept now, whole-word, with a test that runs the sweep on populated state.

Smaller: three duplicates of place_tests removed, the Gifs cache's cap now tested, "1 viewers" in a raid line (a test had pinned it) now "1 viewer", three Sneedchat live probes assert something real and a manual probe that waited three minutes for a person and asserted nothing is gone. Three ignored probes (a session check, a page fetch, a Tor health print) remain diagnostics that assert nothing, by design.

964 pass, 27 ignored (live services, hardware); clippy -D warnings clean. Each replaced test was checked by breaking the code under it.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

src/backend/discord/http.rs

@@ -112,12 +112,13 @@ pub(super) async fn send_write(request: reqwest::RequestBuilder) -> Result<reqwe
 /// number is not a reason to hammer, and one asking for half an hour is not a
 /// wait anybody would sit through inside a request.
 pub(super) fn retry_after(resp: &reqwest::Response) -> Duration {
-    let seconds = resp
-        .headers()
-        .get("retry-after")
-        .and_then(|v| v.to_str().ok())
-        .and_then(|v| v.parse::<f64>().ok())
-        .unwrap_or(1.0);
+    pause_for(resp.headers().get("retry-after").and_then(|v| v.to_str().ok()))
+}
+
+/// The wait a `Retry-After` value asks for; none given, or none that reads as
+/// a number, is a second.
+fn pause_for(seconds: Option<&str>) -> Duration {
+    let seconds = seconds.and_then(|v| v.parse::<f64>().ok()).unwrap_or(1.0);
     Duration::from_millis(((seconds.max(0.0) * 1000.0) as u64).clamp(200, 30_000))
 }
 
@@ -217,19 +218,9 @@ pub(super) fn form_errors(resp: &Value) -> Option<String> {
 
 #[cfg(test)]
 mod pacing_tests {
+    use super::pause_for;
     use std::time::Duration;
 
-    /// The wait Discord asked for, read the way it sends it: seconds, with a
-    /// decimal point, in a header.
-    ///
-    /// Clamped at both ends on purpose - a limit with no number is not a
-    /// reason to hammer, and a half-hour one is not a wait to sit through
-    /// inside a request - and this is the arithmetic that decides both.
-    fn pause_for(seconds: Option<&str>) -> Duration {
-        let seconds = seconds.and_then(|v| v.parse::<f64>().ok()).unwrap_or(1.0);
-        Duration::from_millis(((seconds.max(0.0) * 1000.0) as u64).clamp(200, 30_000))
-    }
-
     #[test]
     fn a_wait_is_taken_as_asked() {
         assert_eq!(pause_for(Some("1.5")), Duration::from_millis(1500));

src/backend/irc/connect.rs

@@ -701,6 +701,21 @@ pub(super) fn cap_list<'a>(param: Option<&'a str>, suffix: Option<&'a str>) -> &
     param.or(suffix).unwrap_or("")
 }
 
+/// One line of a `CAP LS` answer: whether another is coming, and what this one
+/// lists.
+///
+/// A multiline answer puts "*" in one field and the list in the other, and a
+/// live network is what proved that which field is which cannot be assumed:
+/// looking for the star in one only meant the last continuation line was taken
+/// for the whole answer, and every capability advertised after it was silently
+/// never requested. Libera offers nineteen; this client was seeing nine.
+pub(super) fn cap_ls_page<'a>(param: Option<&'a str>, suffix: Option<&'a str>) -> (bool, Vec<&'a str>) {
+    let fields = [param, suffix];
+    let more = fields.iter().any(|f| f.map(str::trim) == Some("*"));
+    let caps = fields.into_iter().flatten().filter(|f| f.trim() != "*").flat_map(str::split_whitespace).collect();
+    (more, caps)
+}
+
 /// Which of a `CAP NEW` offer are worth asking for.
 ///
 /// The intersection with `WANTED_CAPS`, because a server offering something
@@ -854,13 +869,8 @@ pub(super) async fn offered_caps_raw(sender: &Sender, stream: &mut ClientStream)
     loop {
         let msg = wait_for(stream, |m| matches!(&m.command, Command::CAP(_, CapSubCommand::LS, _, _))).await.ok()?;
         let Command::CAP(_, _, ref param, ref suffix) = msg.command else { return None };
-        // A multiline answer puts "*" where a single-line one puts the list,
-        // so the list is whichever field is not the continuation marker.
-        let more = param.as_deref() == Some("*");
-        let list = if more { suffix.as_deref().unwrap_or("") } else { cap_list(param.as_deref(), suffix.as_deref()) };
-        for cap in list.split_whitespace() {
-            offered.push(cap.to_string());
-        }
+        let (more, caps) = cap_ls_page(param.as_deref(), suffix.as_deref());
+        offered.extend(caps.into_iter().map(str::to_string));
         if !more {
             tracing::debug!("irc: server offers {} capabilities: {}", offered.len(), offered.join(" "));
             return Some(offered);
@@ -1199,6 +1209,21 @@ mod quit_tests {
     }
 }
 
+#[cfg(test)]
+mod wanted_cap_tests {
+    use super::WANTED_CAPS;
+
+    /// Both spellings of metadata, because networks that shipped it first
+    /// still advertise the draft one; a rename in the list would end the
+    /// negotiation for one of them without anything saying so.
+    #[test]
+    fn both_spellings_of_metadata_are_asked_for() {
+        assert!(WANTED_CAPS.contains(&"draft/metadata"));
+        assert!(WANTED_CAPS.contains(&"metadata-2"));
+        assert!(!WANTED_CAPS.contains(&"metadata"));
+    }
+}
+
 #[cfg(test)]
 mod cap_new_tests {
     use super::caps_worth_requesting;
@@ -1239,7 +1264,7 @@ mod cap_new_tests {
 
 #[cfg(test)]
 mod cap_tests {
-    use super::cap_list;
+    use super::{cap_list, cap_ls_page};
     use irc::proto::{CapSubCommand, Command, Message};
 
     /// Which field of a CAP LS line holds the list, and which holds the "*"
@@ -1254,21 +1279,37 @@ mod cap_tests {
     fn a_multiline_offer_is_read_to_the_end() {
         // Both orderings of (param, suffix), because both occur.
         for (param, suffix) in [(Some("*"), Some("a b")), (Some("a b"), Some("*"))] {
-            let fields = [param, suffix];
-            let more = fields.iter().any(|f| f.map(str::trim) == Some("*"));
-            let caps: Vec<&str> = fields
-                .into_iter()
-                .flatten()
-                .filter(|f| f.trim() != "*")
-                .flat_map(str::split_whitespace)
-                .collect();
-            assert!(more, "a line carrying a star has more to come");
-            assert_eq!(caps, vec!["a", "b"]);
+            let (more, caps) = cap_ls_page(param, suffix);
+            assert!(more, "a line carrying a star has more to come: {param:?} {suffix:?}");
+            assert_eq!(caps, vec!["a", "b"], "{param:?} {suffix:?}");
         }
+        // The final line carries no star and is the end of the answer, whichever
+        // field its list is in.
+        for (param, suffix) in [(None, Some("c d")), (Some("c d"), None)] {
+            let (more, caps) = cap_ls_page(param, suffix);
+            assert!(!more, "{param:?} {suffix:?}");
+            assert_eq!(caps, vec!["c", "d"]);
+        }
+        // Nothing to list is an empty page, not a failure.
+        assert_eq!(cap_ls_page(Some("*"), None), (true, vec![]));
+        assert_eq!(cap_ls_page(None, None), (false, vec![]));
+    }
 
-        // The final line carries no star and is the end of the answer.
-        let fields = [None, Some("c d")];
-        assert!(!fields.iter().any(|f| f.map(str::trim) == Some("*")));
+    /// The same, through the parser the connection uses: a real server line
+    /// and the fields the crate makes of it.
+    #[test]
+    fn a_continuation_line_off_the_wire_says_more_is_coming() {
+        let msg: Message = ":irc.example CAP * LS * :multi-prefix away-notify\r\n".parse().unwrap();
+        let Command::CAP(_, CapSubCommand::LS, param, suffix) = &msg.command else { panic!("not CAP LS: {:?}", msg.command) };
+        let (more, caps) = cap_ls_page(param.as_deref(), suffix.as_deref());
+        assert!(more);
+        assert_eq!(caps, vec!["multi-prefix", "away-notify"]);
+
+        let last: Message = ":irc.example CAP * LS :sasl chghost\r\n".parse().unwrap();
+        let Command::CAP(_, CapSubCommand::LS, param, suffix) = &last.command else { panic!("not CAP LS: {:?}", last.command) };
+        let (more, caps) = cap_ls_page(param.as_deref(), suffix.as_deref());
+        assert!(!more);
+        assert_eq!(caps, vec!["sasl", "chghost"]);
     }
 
     /// Where the granted capabilities actually live in a parsed CAP ACK.

src/backend/irc/dcc.rs

@@ -1082,6 +1082,35 @@ pub async fn offer_file(state: &AppState, account_id: &str, nick: &str, path: &s
     Ok(id)
 }
 
+/// Waits for whoever the offer was made to, for as long as the offer stands.
+///
+/// Looks at the cancel flag as it waits. It used to look only once somebody
+/// had connected, so cancelling an offer nobody had taken did nothing for the
+/// three minutes the offer lasted, with the port open and the transfer shown
+/// as still going.
+async fn wait_for_taker(
+    listener: &tokio::net::TcpListener,
+    wait: std::time::Duration,
+    cancel: &std::sync::atomic::AtomicBool,
+) -> Result<(tokio::net::TcpStream, std::net::SocketAddr)> {
+    let deadline = std::time::Instant::now() + wait;
+    loop {
+        if cancel.load(std::sync::atomic::Ordering::Relaxed) {
+            bail!("cancelled");
+        }
+        let left = deadline.saturating_duration_since(std::time::Instant::now());
+        if left.is_zero() {
+            bail!("they did not accept it within {} seconds", wait.as_secs());
+        }
+        let slice = left.min(std::time::Duration::from_millis(250));
+        match tokio::time::timeout(slice, listener.accept()).await {
+            Err(_) => continue,
+            Ok(Err(e)) => return Err(anyhow!("waiting for them to connect: {e}")),
+            Ok(Ok(pair)) => return Ok(pair),
+        }
+    }
+}
+
 /// Waits for them to connect, then hands the file over.
 ///
 /// Takes what to do about progress rather than the state to do it to, the same
@@ -1096,14 +1125,9 @@ async fn serve_file(
     on_accepted: impl FnOnce(),
     mut on_progress: impl FnMut(Progress),
 ) -> Result<()> {
-    let accepted = tokio::time::timeout(OFFER_TIMEOUT, listener.accept()).await;
     // The listener is dropped either way once this returns, so an offer that
     // nobody took does not leave a port open behind it.
-    let (mut sock, peer) = match accepted {
-        Err(_) => bail!("they did not accept it within {} seconds", OFFER_TIMEOUT.as_secs()),
-        Ok(Err(e)) => return Err(anyhow!("waiting for them to connect: {e}")),
-        Ok(Ok(pair)) => pair,
-    };
+    let (mut sock, peer) = wait_for_taker(&listener, OFFER_TIMEOUT, cancel).await?;
     if cancel.load(std::sync::atomic::Ordering::Relaxed) {
         bail!("cancelled");
     }
@@ -2122,21 +2146,40 @@ mod transfer_tests {
 
     #[tokio::test]
     async fn an_offer_nobody_takes_gives_up_rather_than_waiting_forever() {
-        let dir = temp_dir("nobody");
-        let path = dir.join("ignored.bin");
-        std::fs::write(&path, b"x").unwrap();
         let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
+        let cancel = AtomicBool::new(false);
+        let began = std::time::Instant::now();
+        let err = wait_for_taker(&listener, std::time::Duration::from_millis(300), &cancel).await.unwrap_err();
+        assert!(format!("{err:#}").contains("did not accept"), "{err:#}");
+        assert!(began.elapsed() < std::time::Duration::from_secs(3), "it waited {:?}", began.elapsed());
+    }
 
-        let cancel = AtomicBool::new(true);
-        // Cancelled before anyone connects stands in for the timeout, which
-        // is three minutes and not worth a test's patience.
-        let err = tokio::time::timeout(
-            std::time::Duration::from_millis(200),
-            serve_file(listener, path.to_str().unwrap(), 1, 0, &cancel, || {}, |_| {}),
-        )
-        .await;
-        assert!(err.is_err() || err.unwrap().is_err(), "an offer nobody takes must not succeed");
-        let _ = std::fs::remove_dir_all(dir);
+    #[tokio::test]
+    async fn cancelling_an_offer_nobody_has_taken_ends_it_at_once() {
+        let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
+        let cancel = std::sync::Arc::new(AtomicBool::new(false));
+        let stop = cancel.clone();
+        tokio::spawn(async move {
+            tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+            stop.store(true, std::sync::atomic::Ordering::Relaxed);
+        });
+        let began = std::time::Instant::now();
+        // An hour's patience, so only the cancel can be what ends it.
+        let err = wait_for_taker(&listener, std::time::Duration::from_secs(3600), &cancel).await.unwrap_err();
+        assert!(format!("{err:#}").contains("cancelled"), "{err:#}");
+        assert!(began.elapsed() < std::time::Duration::from_secs(3), "it waited {:?}", began.elapsed());
+    }
+
+    #[tokio::test]
+    async fn someone_who_connects_is_the_one_taken() {
+        let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
+        let port = listener.local_addr().unwrap().port();
+        tokio::spawn(async move {
+            let _ = tokio::net::TcpStream::connect(("127.0.0.1", port)).await;
+        });
+        let cancel = AtomicBool::new(false);
+        let (_, peer) = wait_for_taker(&listener, std::time::Duration::from_secs(5), &cancel).await.expect("should accept");
+        assert!(peer.ip().is_loopback());
     }
 
     #[tokio::test]

src/backend/irc/incoming.rs

@@ -312,12 +312,7 @@ pub(super) async fn handle_message(
                     let Some(slot) = members.get_mut(target.as_str()) else { continue };
                     slot.rank = if granting { rank } else { MemberRank::None };
                     changed = true;
-                    let body = format!(
-                        "{from} {} {} {} {target}",
-                        if granting { "gives" } else { "takes" },
-                        rank_word(rank),
-                        if granting { "to" } else { "from" },
-                    );
+                    let body = rank_change_line(&from, granting, rank, target);
                     state.runtime.record_message(state, account_id, &channel, "channel", "*", &body, false, "system", None, None, false, None, Vec::new(), Vec::new(), None);
                 }
                 if changed {
@@ -1024,6 +1019,16 @@ pub(super) fn rank_word(rank: MemberRank) -> &'static str {
     }
 }
 
+/// The line for somebody being given a rank or losing one.
+pub(super) fn rank_change_line(from: &str, granting: bool, rank: MemberRank, target: &str) -> String {
+    format!(
+        "{from} {} {} {} {target}",
+        if granting { "gives" } else { "takes" },
+        rank_word(rank),
+        if granting { "to" } else { "from" },
+    )
+}
+
 /// A message's own id, where the server gives one.
 ///
 /// Used as the message id rather than a generated one, so a message seen twice -
@@ -1229,14 +1234,8 @@ mod tests {
     /// words are used for granting and for taking away.
     #[test]
     fn a_rank_change_reads_as_a_sentence() {
-        assert_eq!(
-            format!("bob {} {} {} carol", "gives", rank_word(MemberRank::Op), "to"),
-            "bob gives operator status to carol"
-        );
-        assert_eq!(
-            format!("bob {} {} {} carol", "takes", rank_word(MemberRank::Voice), "from"),
-            "bob takes voice from carol"
-        );
+        assert_eq!(rank_change_line("bob", true, MemberRank::Op, "carol"), "bob gives operator status to carol");
+        assert_eq!(rank_change_line("bob", false, MemberRank::Voice, "carol"), "bob takes voice from carol");
     }
 
     #[test]

src/backend/irc/metadata.rs

@@ -56,14 +56,6 @@ pub fn parse_keyvalue(args: &[String]) -> Option<(String, String, Option<String>
     Some((target, key, value))
 }
 
-/// Whether a capability name is one of the two spellings of metadata.
-/// Only the tests reach this - the capability names are matched inline
-/// where they are negotiated.
-#[cfg(test)]
-pub fn is_metadata_cap(cap: &str) -> bool {
-    matches!(cap, "draft/metadata" | "metadata-2")
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -92,12 +84,4 @@ mod tests {
         assert_eq!(parse_keyvalue(&args(&["alice"])), None);
         assert_eq!(parse_keyvalue(&[]), None);
     }
-
-    #[test]
-    fn both_spellings_of_the_capability_count() {
-        assert!(is_metadata_cap("draft/metadata"));
-        assert!(is_metadata_cap("metadata-2"));
-        assert!(!is_metadata_cap("metadata"));
-        assert!(!is_metadata_cap("away-notify"));
-    }
 }

src/backend/kick/cards.rs

@@ -517,7 +517,7 @@ mod tests {
         assert!(matches!(id_in_subscription("chatrooms.53906513.v2"), Some(Subscribed::Chatroom(53906513))));
     }
 
-    /// A poll is one thing happening over a minute    /// A poll is one thing happening over a minute, not a stream of events -
+    /// A poll is one thing happening over a minute, not a stream of events -
     /// Kick sends its update on every vote, and a line each would bury the
     /// conversation the poll is about.
     #[test]

src/backend/kick/chat.rs

@@ -600,7 +600,8 @@ pub(super) fn describe_action(event: &str, p: &serde_json::Value) -> Option<(&'s
         "StreamHostEvent" => {
             let who = text("host_username").or_else(who)?;
             Some(("raid", match p.get("number_viewers").and_then(|v| v.as_u64()) {
-                Some(n) if n > 0 => format!("{who} raided with {n} viewers"),
+                Some(1) => format!("{who} raided with 1 viewer"),
+                Some(n) if n > 1 => format!("{who} raided with {n} viewers"),
                 _ => format!("{who} raided the channel"),
             }))
         }
@@ -1334,7 +1335,7 @@ mod tests {
         let none = serde_json::json!({ "host_username": "friend", "number_viewers": 0 });
         assert_eq!(describe_action("StreamHostEvent", &none).map(|(_, l)| l).as_deref(), Some("friend raided the channel"));
         let one = serde_json::json!({ "host_username": "friend", "number_viewers": 1 });
-        assert_eq!(describe_action("StreamHostEvent", &one).map(|(_, l)| l).as_deref(), Some("friend raided with 1 viewers"));
+        assert_eq!(describe_action("StreamHostEvent", &one).map(|(_, l)| l).as_deref(), Some("friend raided with 1 viewer"));
         assert_eq!(action_card("StreamHostEvent", &none).unwrap().title, None, "no count to put on the card");
     }
 

src/backend/matrix/calls.rs

@@ -595,21 +595,6 @@ async fn openid_again(state: &AppState, account_id: &str) -> anyhow::Result<Valu
     .context("asking the homeserver to vouch for this account")
 }
 
-/// Who to call, and who to wait for./// Who to call, and who to wait for./// Who to call, and who to wait for.
-///
-/// Both ends of every pair see each other arrive, and if both call, both
-/// answer, and the call collides with itself. So the rule is one line and the
-/// same on both sides: the smaller id calls the larger. It has to be a
-/// property of the pair rather than of who arrived first, because "first" is
-/// not something two clients can agree on.
-///
-/// Kept for the tests that pin the rule down; nothing in the call path
-/// asks it yet, which is its own gap rather than a reason to lose it.
-#[cfg(test)]
-pub fn should_offer(own_key: &str, their_key: &str) -> bool {
-    own_key < their_key
-}
-
 #[cfg(test)]
 mod member_tests {
     use super::*;
@@ -699,17 +684,6 @@ mod member_tests {
         );
     }
 
-    #[test]
-    fn exactly_one_end_of_every_pair_offers() {
-        // Whichever way round the two clients ask, they must not agree.
-        assert!(should_offer("@a:example.org|AAA", "@b:example.org|BBB"));
-        assert!(!should_offer("@b:example.org|BBB", "@a:example.org|AAA"));
-        // Including two devices of the same person, which is the case that
-        // makes a user-id comparison alone wrong.
-        assert!(should_offer("@a:example.org|AAA", "@a:example.org|ZZZ"));
-        assert!(!should_offer("@a:example.org|ZZZ", "@a:example.org|AAA"));
-    }
-
     /// The fallback rides along whether or not the homeserver offered a relay:
     /// it is needed exactly when the relay is missing or unreachable, and the
     /// two look the same from here.

src/backend/matrix/directs.rs

@@ -118,35 +118,6 @@ pub(super) async fn record(state: &AppState, account_id: &str, peer_user_id: &st
     }
 }
 
-/// The list without a room, wherever it appears.
-///
-/// A person is dropped entirely when their last room goes: an empty array
-/// left behind is a person the account claims to have a DM list for and no
-/// DMs with, which other clients render as an empty conversation.
-///
-/// Only the tests call it. The live path rewrites the whole map rather than
-/// subtracting from it, which is a difference worth keeping an eye on: this
-/// is the rule the tests pin down, and nothing enforces that the live path
-/// still agrees with it.
-#[cfg(test)]
-pub fn without(content: &Value, room_id: &str) -> Value {
-    let Some(by_user) = content.as_object() else { return json!({}) };
-    let mut out = Map::new();
-    for (user_id, rooms) in by_user {
-        let kept: Vec<Value> = rooms
-            .as_array()
-            .into_iter()
-            .flatten()
-            .filter(|r| r.as_str() != Some(room_id))
-            .cloned()
-            .collect();
-        if !kept.is_empty() {
-            out.insert(user_id.clone(), Value::Array(kept));
-        }
-    }
-    Value::Object(out)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -194,17 +165,4 @@ mod tests {
         let again = with(&after, "@anna:poa.st", "!new:poa.st");
         assert_eq!(again["@anna:poa.st"], json!(["!one:poa.st", "!new:poa.st"]));
     }
-
-    #[test]
-    fn removing_a_room_takes_the_person_with_it_when_it_was_the_last_one() {
-        let before = json!({ "@anna:poa.st": ["!one:poa.st", "!two:poa.st"], "@bob:poa.st": ["!three:poa.st"] });
-        let after = without(&before, "!three:poa.st");
-        assert_eq!(after["@anna:poa.st"], json!(["!one:poa.st", "!two:poa.st"]));
-        // Not an empty list: a person with no rooms reads to other clients
-        // as a conversation with nothing in it.
-        assert!(after.get("@bob:poa.st").is_none());
-
-        let one_left = without(&before, "!one:poa.st");
-        assert_eq!(one_left["@anna:poa.st"], json!(["!two:poa.st"]));
-    }
 }

src/backend/matrix/probes.rs

@@ -131,123 +131,6 @@ mod live {
         .await;
     }
 
-    /// Phase-3 manual live check: creates a real *encrypted* room, starts
-    /// nobilis's own sync loop against it (as one "device" of the test
-    /// account), then idles for up to 3 minutes printing every message
-    /// this backend receives and decrypts. Meant to be run in the
-    /// background while a second, independent Matrix client (e.g. Element
-    /// Web, logged into the same account as a genuinely different device)
-    /// joins the room and sends a real message into it - the room id is
-    /// printed up front so it can be found there. This is the actual hard
-    /// gate for "full E2EE from day one": a message this backend never
-    /// touched the encryption of, decrypted correctly, proves the whole
-    /// device-key-upload + key-claim + Megolm-session path works against
-    /// a real second device, not just against itself.
-    ///
-    ///   MATRIX_USERNAME=... MATRIX_PASSWORD=... \
-    ///     cargo test --release -- --ignored --nocapture matrix_e2ee_read_probe
-    #[tokio::test]
-    #[ignore]
-    async fn matrix_e2ee_read_probe() {
-        let Ok(username) = std::env::var("MATRIX_USERNAME") else {
-            println!("MATRIX_USERNAME not set, skipping");
-            return;
-        };
-        let Ok(password) = std::env::var("MATRIX_PASSWORD") else {
-            println!("MATRIX_PASSWORD not set, skipping");
-            return;
-        };
-        let homeserver = std::env::var("MATRIX_HOMESERVER").unwrap_or_else(|_| "https://matrix.org".to_string());
-        let _ = rustls::crypto::ring::default_provider().install_default();
-
-        let login = auth::login(&homeserver, &username, &password, None).await.expect("login failed");
-        println!("logged in as {} (device {})", login.user_id, login.device_id);
-
-        let create_resp = http::post_json(
-            &format!("{}/_matrix/client/v3/createRoom", homeserver.trim_end_matches('/')),
-            Some(&login.access_token),
-            serde_json::json!({
-                "preset": "private_chat",
-                "name": "nobilis-matrix-phase3-probe",
-                "initial_state": [
-                    { "type": "m.room.encryption", "state_key": "", "content": { "algorithm": "m.megolm.v1.aes-sha2" } }
-                ],
-            }),
-        )
-        .await
-        .expect("createRoom failed");
-        let room_id = create_resp["room_id"].as_str().expect("no room_id in createRoom response").to_string();
-        println!("created ENCRYPTED test room {room_id} - join it with a second client (same account, different device) and send a message now.");
-
-        let data_dir = std::env::temp_dir().join(format!("nobilis-matrix-e2ee-probe-{}", std::process::id()));
-        std::fs::create_dir_all(&data_dir).unwrap();
-        let state = AppState {
-            store: std::sync::Arc::new(crate::store::Store::open(&data_dir.join("scrollback.db")).expect("opening store")),
-            accounts: std::sync::Arc::new(crate::accounts::AccountStore::open(data_dir.join("accounts.toml")).expect("opening accounts")),
-            events: crate::events::EventBus::new(),
-            runtime: std::sync::Arc::new(crate::runtime::Runtime::new()),
-            tor: std::sync::Arc::new(crate::net::tor::TorManager::new(&data_dir)),
-            shutdown: std::sync::Arc::new(tokio::sync::Notify::new()),
-            voice: std::sync::Arc::new(crate::backend::discord::voice::VoiceState::new()),
-            voice_prefs: std::sync::Arc::new(crate::audio::VoicePrefsStore::open(data_dir.join("voice.toml"))),
-            dcc_prefs: std::sync::Arc::new(crate::backend::irc::dcc::DccPrefsStore::open(data_dir.join("dcc.toml"))),
-            irc_sts: std::sync::Arc::new(crate::backend::irc::sts::StsStore::open(data_dir.join("irc-sts.toml"))),
-            highlights: std::sync::Arc::new(crate::highlights::HighlightStore::open(data_dir.join("highlights.toml"))),
-            ignores: std::sync::Arc::new(crate::ignores::IgnoreStore::open(data_dir.join("ignores.toml"))),
-        };
-
-        let config = MatrixAccountConfig {
-            use_tor: false,
-            strict_route: false,
-            homeserver_url: homeserver.clone(),
-            user_id: login.user_id.clone(),
-            password: password.clone(),
-            access_token: login.access_token.clone(),
-            device_id: login.device_id.clone(),
-            next_batch: None,
-            used_sliding_sync: false,
-            oauth_refresh_token: String::new(),
-        oauth_client_id: String::new(),
-        prefer_sliding_sync: false,
-            dehydration_enabled: false,
-            display_name: None,
-            rtc_focus_url: None,
-        };
-        let saved = state.accounts.add_matrix(config).expect("add_matrix failed");
-        spawn(state.clone(), saved);
-
-        let buffer_name = "nobilis-matrix-phase3-probe";
-        let mut buffer_id = None;
-        for _ in 0..30 {
-            tokio::time::sleep(Duration::from_secs(1)).await;
-            if let Some(b) = state.runtime.list_buffers().into_iter().find(|b| b.name == buffer_name) {
-                buffer_id = Some(b.id);
-                break;
-            }
-        }
-        let buffer_id = buffer_id.expect("buffer for the test room never appeared within 30s");
-        println!("buffer created: {buffer_id} - waiting up to 180s for a message from a second device...");
-
-        let mut seen_bodies: std::collections::HashSet<String> = std::collections::HashSet::new();
-        for i in 0..180 {
-            tokio::time::sleep(Duration::from_secs(1)).await;
-            if let Ok(msgs) = state.store.get_backlog(&buffer_id, 0, 20) {
-                for m in &msgs {
-                    if seen_bodies.insert(m.id.clone()) {
-                        println!("[{i}s] received from {}: {}", m.from, m.body);
-                    }
-                }
-            }
-        }
-
-        let _ = http::post_json(
-            &format!("{}/_matrix/client/v3/rooms/{}/leave", homeserver.trim_end_matches('/'), room_id),
-            Some(&login.access_token),
-            serde_json::json!({}),
-        )
-        .await;
-    }
-
     /// Phase-3/phase-4 combined live check, fully automated (no manual
     /// second client needed): "device A" is nobilis's own real production
     /// connect loop (spawn/run_sync, same code path a real account uses);

src/backend/matrix/relock.rs

@@ -48,6 +48,23 @@ fn pending() -> &'static std::sync::Mutex<HashMap<String, Vec<Locked>>> {
     PENDING.get_or_init(Default::default)
 }
 
+/// Adds a message to those waiting, and says whether it was new.
+///
+/// The same event twice is the same event: a sync that overlaps what was
+/// already seen would otherwise ask for one key repeatedly. And the list is
+/// held to `MAX_PENDING`, oldest first.
+fn hold(waiting: &mut Vec<Locked>, locked: Locked) -> bool {
+    if waiting.iter().any(|l| l.event_id == locked.event_id) {
+        return false;
+    }
+    waiting.push(locked);
+    if waiting.len() > MAX_PENDING {
+        let over = waiting.len() - MAX_PENDING;
+        waiting.drain(..over);
+    }
+    true
+}
+
 /// Remembers a message that could not be read, and asks for its key.
 ///
 /// Best-effort on both counts. A key request that cannot be sent is not worth
@@ -67,20 +84,14 @@ pub async fn note_locked(
     {
         let mut all = pending().lock().unwrap();
         let waiting = all.entry(account_id.to_string()).or_default();
-        // The same event twice is the same event: a sync that overlaps what
-        // was already seen would otherwise ask for one key repeatedly.
-        if waiting.iter().any(|l| l.event_id == event_id) {
-            return;
-        }
-        waiting.push(Locked {
+        let locked = Locked {
             buffer_id: buffer_id.to_string(),
             room_id: room_id.to_string(),
             event_id: event_id.to_string(),
             event: event.clone(),
-        });
-        if waiting.len() > MAX_PENDING {
-            let over = waiting.len() - MAX_PENDING;
-            waiting.drain(..over);
+        };
+        if !hold(waiting, locked) {
+            return;
         }
     }
 
@@ -158,10 +169,9 @@ mod tests {
     /// neighbours, and what anybody can see is a screen's worth.
     #[test]
     fn the_oldest_are_dropped_rather_than_growing_without_end() {
-        let mut waiting: Vec<Locked> = (0..MAX_PENDING + 50).map(|i| locked(&format!("${i}"))).collect();
-        if waiting.len() > MAX_PENDING {
-            let over = waiting.len() - MAX_PENDING;
-            waiting.drain(..over);
+        let mut waiting: Vec<Locked> = Vec::new();
+        for i in 0..MAX_PENDING + 50 {
+            assert!(hold(&mut waiting, locked(&format!("${i}"))));
         }
         assert_eq!(waiting.len(), MAX_PENDING);
         // The newest survive, which are the ones somebody is looking at.
@@ -174,11 +184,8 @@ mod tests {
     #[test]
     fn the_same_message_is_only_waited_on_once() {
         let mut waiting = vec![locked("$a")];
-        for id in ["$a", "$b", "$a"] {
-            if !waiting.iter().any(|l| l.event_id == id) {
-                waiting.push(locked(id));
-            }
-        }
+        let news: Vec<bool> = ["$a", "$b", "$a"].iter().map(|id| hold(&mut waiting, locked(id))).collect();
+        assert_eq!(news, [false, true, false], "only a message not already waiting is news");
         assert_eq!(waiting.len(), 2);
     }
 }

src/backend/matrix/stickers.rs

@@ -346,6 +346,8 @@ mod place_tests {
         assert!(parse_place("the pub").is_none());
         assert!(parse_place("91.0, 0.0").is_none());
         assert!(parse_place("0.0, 181.0").is_none());
+        // A latitude past the pole is somebody's phone number, not a place.
+        assert!(parse_place("910, 20").is_none());
     }
 
     /// Coordinates are written with a space after the comma as often as not,
@@ -363,6 +365,11 @@ mod place_tests {
         assert!(label.is_empty());
 
         assert!(split_place_and_label("just some words").is_none());
+
+        // A map link is a place in one word, and the name still follows it.
+        let (place, label) = split_place_and_label("https://www.openstreetmap.org/#map=15/51.5074/-0.1278 home").expect("a place");
+        assert_eq!(place, "https://www.openstreetmap.org/#map=15/51.5074/-0.1278");
+        assert_eq!(label, "home");
     }
 }
 
@@ -472,47 +479,6 @@ mod tests {
         assert!(read_pack(&serde_json::json!({ "images": [] }), "x").is_empty());
     }
 
-    #[test]
-    fn a_place_is_read_from_what_people_actually_paste() {
-        assert_eq!(parse_place("51.5074, -0.1278"), Some((51.5074, -0.1278)));
-        assert_eq!(parse_place("geo:51.5074,-0.1278;u=35"), Some((51.5074, -0.1278)));
-        assert_eq!(
-            parse_place("https://www.openstreetmap.org/#map=15/51.5074/-0.1278"),
-            Some((51.5074, -0.1278))
-        );
-        assert_eq!(
-            parse_place("https://www.google.com/maps/@51.5074,-0.1278,15z"),
-            Some((51.5074, -0.1278))
-        );
-    }
-
-    #[test]
-    fn two_numbers_that_are_not_a_place_are_refused() {
-        // A latitude past the pole is somebody's phone number, not a place.
-        assert_eq!(parse_place("910, 20"), None);
-        assert_eq!(parse_place("hello"), None);
-        assert_eq!(parse_place(""), None);
-    }
-
-    #[test]
-    fn a_place_and_its_name_come_apart() {
-        // The comma-space form is how people write coordinates, so the place
-        // is two words and the name is the rest.
-        assert_eq!(
-            split_place_and_label("51.5074, -0.1278 the pub"),
-            Some(("51.5074, -0.1278".to_string(), "the pub".to_string()))
-        );
-        assert_eq!(
-            split_place_and_label("51.5074,-0.1278"),
-            Some(("51.5074,-0.1278".to_string(), String::new()))
-        );
-        assert_eq!(
-            split_place_and_label("https://www.openstreetmap.org/#map=15/51.5074/-0.1278 home"),
-            Some(("https://www.openstreetmap.org/#map=15/51.5074/-0.1278".to_string(), "home".to_string()))
-        );
-        assert_eq!(split_place_and_label("where are you"), None);
-    }
-
     #[test]
     fn a_location_event_carries_both_halves() {
         let event = location_event(51.5074, -0.1278, "the pub");

src/backend/sneedchat/probes.rs

@@ -77,6 +77,12 @@ pub(super) mod live_probe {
                 println!("{i}: {}", line.trim().chars().take(400).collect::<String>());
             }
         }
+
+        // And the part that can fail: the reader this client really uses must
+        // still find the rooms on the page the site really serves.
+        let rooms = crate::backend::sneedchat::rooms::parse_rooms(&body);
+        println!("{} room(s) read: {:?}", rooms.len(), rooms.iter().map(|r| r.name.as_str()).collect::<Vec<_>>());
+        assert!(!rooms.is_empty(), "the room switcher is no longer where parse_rooms looks for it");
     }
 
     /// Whether the session this account already holds is still good.
@@ -146,13 +152,8 @@ pub(super) mod live_probe {
             pow::clear(&http, &login, 8).await.expect("failed to clear the gate");
             page = http.get(&login).await.expect("GET after clearing gate failed");
         }
-        let sitekey = page
-            .body
-            .split("data-sitekey=\"")
-            .nth(1)
-            .and_then(|rest| rest.split('"').next())
-            .expect("no data-sitekey on the login page")
-            .to_string();
+        let sitekey = crate::backend::sneedchat::captcha::site_key(&page.body)
+            .expect("site_key finds no captcha widget on the login page - the markup has changed");
         println!("sitekey: {sitekey}");
 
         // The widget opens with a GET of `<apiBase>/start?key=...`; every
@@ -228,7 +229,7 @@ pub(super) mod live_probe {
                     println!("  {name} = {shown:?}");
                 }
             }
-            None => println!("!! no /login/login form section found - the layout has changed"),
+            None => panic!("no /login/login form section found - the layout has changed"),
         }
 
         println!("--- captcha markers anywhere on the page ---");

src/gifs.rs

@@ -379,8 +379,20 @@ mod tests {
     fn what_was_looked_at_is_remembered_until_the_table_is_full() {
         remember("https://tenor.com/view/remember-a", None);
         remember("https://tenor.com/view/remember-b", Some(Embed::default()));
+        {
+            let known = KNOWN.lock().unwrap();
+            assert!(known.contains_key("https://tenor.com/view/remember-a"), "a page that said nothing is remembered too");
+            assert!(known.get("https://tenor.com/view/remember-b").is_some_and(|e| e.is_some()));
+        }
+
+        // And until the table is full: past the limit it starts again rather
+        // than growing for as long as the daemon runs, and what was just
+        // learned is never the thing thrown away.
+        for i in 0..KNOWN_LIMIT + 5 {
+            remember(&format!("https://tenor.com/view/fill-{i}"), None);
+        }
         let known = KNOWN.lock().unwrap();
-        assert!(known.contains_key("https://tenor.com/view/remember-a"), "a page that said nothing is remembered too");
-        assert!(known.get("https://tenor.com/view/remember-b").is_some_and(|e| e.is_some()));
+        assert!(known.len() <= KNOWN_LIMIT, "{} remembered", known.len());
+        assert!(known.contains_key(&format!("https://tenor.com/view/fill-{}", KNOWN_LIMIT + 4)), "the newest is kept");
     }
 }

src/runtime.rs

@@ -5416,7 +5416,8 @@ impl Runtime {
         );
         sweep_set!(
             discord_history_inflight, matrix_encrypted_rooms, matrix_backup_enabled,
-            emoji_unrestricted,
+            emoji_unrestricted, wants_connected, discord_gated, irc_monitor, replaying,
+            silenced, irc_rostered,
         );
         sweep_set_pairs!(matrix_previews_off);
     }
@@ -5444,8 +5445,12 @@ mod forget_tests {
                 // Sets as well as maps. Three of them escaped this test for
                 // as long as it only looked for HashMap - which is the
                 // failure it exists to catch, happening to itself.
-                let name = line.split(": Mutex<HashMap<").next()?.split(": Mutex<HashSet<").next()?;
-                ((line.contains(": Mutex<HashMap<") || line.contains(": Mutex<HashSet<")) && !name.contains(' '))
+                // Also the ones written `std::collections::HashSet<`, which
+                // the plain match above never saw: `wants_connected`,
+                // `irc_monitor`, `silenced` and others were outside the test,
+                // and outside the sweep with it.
+                let (name, ty) = line.split_once(": Mutex<")?;
+                ((ty.starts_with("HashMap<") || ty.starts_with("HashSet<") || ty.starts_with("std::collections::HashSet<")) && !name.contains(' '))
                     .then_some(name)
             })
             .collect();
@@ -5457,10 +5462,48 @@ mod forget_tests {
         let sweep = src.find("pub fn forget_account").expect("the sweep");
         let end = src[sweep..].find("\n    }\n}").expect("its end") + sweep;
         let sweep_body = &src[sweep..end];
-        let missing: Vec<&&str> = fields.iter().filter(|f| !sweep_body.contains(**f)).collect();
+        // As a whole word: `buffers` is inside `restored_buffers`, and
+        // `presence` inside `matrix_presence`, so a plain substring test
+        // passed with either of them missing from the sweep.
+        let is_word = |c: char| c.is_alphanumeric() || c == '_';
+        let swept = |name: &str| {
+            sweep_body.match_indices(name).any(|(at, _)| {
+                let before = sweep_body[..at].chars().next_back();
+                let after = sweep_body[at + name.len()..].chars().next();
+                !before.is_some_and(is_word) && !after.is_some_and(is_word)
+            })
+        };
+        let missing: Vec<&&str> = fields.iter().filter(|f| !swept(f)).collect();
         assert!(missing.is_empty(), "these maps would survive removing an account: {missing:?}");
     }
 
+    /// The sweep itself, run: the other test reads the list, this one checks
+    /// that what it names is actually emptied - for this account and its
+    /// buffers, and for nobody else's.
+    #[test]
+    fn forgetting_an_account_empties_it_and_only_it() {
+        let rt = super::Runtime::new();
+        rt.set_own_identity("irc:a", "alice");
+        rt.set_own_identity("irc:ab", "bob");
+        rt.matrix_rooms.lock().unwrap().insert("irc:a|#one".into(), "!one".into());
+        rt.matrix_rooms.lock().unwrap().insert("irc:ab|#one".into(), "!one".into());
+        rt.set_wants_connected("irc:a", true);
+        rt.set_wants_connected("irc:ab", true);
+        rt.silenced.lock().unwrap().insert("irc:a|#one".into());
+        rt.silenced.lock().unwrap().insert("irc:ab|#one".into());
+        rt.discord_members.lock().unwrap().insert(("irc:a|#one".into(), "u".into()), serde_json::json!({}));
+
+        rt.forget_account("irc:a");
+
+        assert!(rt.own_identity.lock().unwrap().get("irc:a").is_none());
+        assert!(rt.own_identity.lock().unwrap().get("irc:ab").is_some(), "a longer id that starts the same is another account");
+        assert_eq!(rt.matrix_rooms.lock().unwrap().keys().collect::<Vec<_>>(), ["irc:ab|#one"]);
+        assert!(rt.discord_members.lock().unwrap().is_empty());
+        assert!(!rt.wants_connected("irc:a"), "a removed account is not wanted online");
+        assert!(rt.wants_connected("irc:ab"));
+        assert_eq!(rt.silenced.lock().unwrap().len(), 1);
+    }
+
     /// The whole of forgetting an account rests on this one test, so it is
     /// worth being explicit about what it must and must not match.
     #[test]

src/store.rs

@@ -1606,7 +1606,7 @@ mod tests {
         let gone = s.forget_account("matrix:@a:server").expect("forgetting");
         assert_eq!(gone, 2, "should have taken exactly its own two");
 
-        let left = |buffer: &str| s.get_backlog(buffer, 50, 0).expect("backlog").len();
+        let left = |buffer: &str| s.get_backlog(buffer, 0, 50).expect("backlog").len();
         assert_eq!(left("matrix:@a:server|!room"), 0);
         assert_eq!(left("matrix:@a:server|!other"), 0);
         assert_eq!(left("matrix:@a:server.uk|!room"), 1, "a different account was emptied");