Bring an IRC account back after the connection drops

The only backend that gave up. sockchat, Discord and Matrix have all had
retry loops for a while; IRC ran its event loop once and, when that
ended, reported "disconnected" and stopped. Recovering meant noticing and
reconnecting by hand, and on a client left running all day the noticing
is the part that does not happen.

The loop needs to tell a dead link from a deliberate disconnect, because
from inside they are identical: disconnect() on a connected account only
sends QUIT, and the session then ends exactly as a dropped one does. So
intent is now recorded, cleared before that QUIT is sent, and read on the
way out of every session - and cleared for everything by quit_all, or the
daemon would spend its shutdown reconnecting what it had just ended.

Attempts back off 3s to 60s on top of the existing per-account pacing. A
real network's anti-flood silently drops connections from a source that
retries too eagerly, which was already worth pacing for one reconnect and
is worth more now that they happen unattended.

Co-Authored-By: Claude Opus 5 <[email protected]>

src/backend/irc.rs

@@ -24,12 +24,27 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(20);
 // that - the actual fix is not re-triggering it, by pacing out our own
 // attempts per account.
 const MIN_RECONNECT_INTERVAL: Duration = Duration::from_secs(5);
+// Same shape the other three backends already use: start small so a blip
+// costs a couple of seconds, and give up ground quickly when a server is
+// genuinely down rather than hammering it.
+const RECONNECT_INITIAL_DELAY: Duration = Duration::from_secs(3);
+const RECONNECT_MAX_DELAY: Duration = Duration::from_secs(60);
 
-/// Spawns a background task that connects this account and runs its event
-/// loop for as long as the connection lasts. No auto-reconnect in this
-/// milestone (see project plan) - a failure just reports connectionState
-/// "disconnected" with an error and stops, same as killing/restarting the
-/// whole daemon would already require for other recovery paths.
+/// Spawns a background task that keeps this account connected: it runs the
+/// event loop, and when that ends it waits and connects again.
+///
+/// It used to stop instead, which made a momentary drop permanent - the one
+/// backend where that was true, since sockchat, Discord and Matrix have all
+/// had retry loops for a while. Recovering meant noticing and reconnecting
+/// by hand, and on a client left running all day the noticing is the part
+/// that does not happen.
+///
+/// Two things keep the loop from being a nuisance. Attempts back off 3s to
+/// 60s, on top of the existing per-account pacing, because a real network's
+/// anti-flood will silently drop connections from a source that retries too
+/// eagerly. And it stops when the account is switched off: disconnect()
+/// clears the intent flag before sending its QUIT, so an ending that was
+/// asked for is distinguishable from a link that died.
 ///
 /// The whole attempt is wrapped in catch_unwind - a bug anywhere in run()
 /// must still leave the account in a resolved (not stuck-"connecting")
@@ -53,6 +68,7 @@ pub fn spawn(state: AppState, config: IrcAccountConfig) {
     // can never blow away a newer, already-successful connection's state
     // just because they happen to share an account_id.
     let generation = state.runtime.next_generation(&account_id);
+    state.runtime.set_wants_connected(&account_id, true);
     state.runtime.set_conn_state(&state, &account_id, ConnState::Connecting, None);
     let delay = state.runtime.throttle_connect_attempt(&account_id, MIN_RECONNECT_INTERVAL);
     let join_handle = tokio::spawn({
@@ -63,18 +79,50 @@ pub fn spawn(state: AppState, config: IrcAccountConfig) {
                 state.runtime.report_progress(&state, &account_id, &format!("Waiting {}s before connecting (reconnected too recently)...", delay.as_secs().max(1)));
                 tokio::time::sleep(delay).await;
             }
-            let result = std::panic::AssertUnwindSafe(run(&state, &config))
-                .catch_unwind()
-                .await;
-            match result {
-                Ok(Ok(())) => state.runtime.finish_connection(&state, &account_id, generation, ConnState::Disconnected, None),
-                Ok(Err(e)) => {
-                    tracing::warn!("irc[{account_id}]: {e}");
-                    state.runtime.finish_connection(&state, &account_id, generation, ConnState::Disconnected, Some(&e.to_string()));
+            let mut backoff = RECONNECT_INITIAL_DELAY;
+            loop {
+                let result = std::panic::AssertUnwindSafe(run(&state, &config))
+                    .catch_unwind()
+                    .await;
+                let detail = match result {
+                    Ok(Ok(())) => None,
+                    Ok(Err(e)) => {
+                        tracing::warn!("irc[{account_id}]: {e}");
+                        Some(e.to_string())
+                    }
+                    Err(_) => {
+                        tracing::error!("irc[{account_id}]: connection task panicked");
+                        Some("internal error (see nobilis logs)".to_string())
+                    }
+                };
+
+                // Switched off while that session was running, or switched
+                // off *by* ending it - disconnect() sends QUIT, which is
+                // what brought us back here.
+                if !state.runtime.wants_connected(&account_id) {
+                    state.runtime.finish_connection(&state, &account_id, generation, ConnState::Disconnected, detail.as_deref());
+                    return;
                 }
-                Err(_) => {
-                    tracing::error!("irc[{account_id}]: connection task panicked");
-                    state.runtime.finish_connection(&state, &account_id, generation, ConnState::Disconnected, Some("internal error (see nobilis logs)"));
+
+                // A newer spawn for this account has taken over; that one
+                // owns the connection now and this loop must not race it.
+                if !state.runtime.is_current_generation(&account_id, generation) {
+                    return;
+                }
+
+                // The session is over, so the Sender that went with it is
+                // dead. Drop it before the wait, or a send during the gap
+                // goes into a socket nobody is reading.
+                state.runtime.remove_irc_handle(&account_id);
+                state.runtime.set_conn_state(&state, &account_id, ConnState::Connecting, None);
+                let because = detail.unwrap_or_else(|| "Connection closed".to_string());
+                state.runtime.report_progress(&state, &account_id, &format!("{because} - reconnecting in {}s...", backoff.as_secs()));
+                tokio::time::sleep(backoff).await;
+                backoff = (backoff * 2).min(RECONNECT_MAX_DELAY);
+
+                let pace = state.runtime.throttle_connect_attempt(&account_id, MIN_RECONNECT_INTERVAL);
+                if !pace.is_zero() {
+                    tokio::time::sleep(pace).await;
                 }
             }
         }

src/runtime.rs

@@ -123,6 +123,15 @@ pub struct Runtime {
     /// new. Each spawn() takes a fresh generation number, and cleanup
     /// only actually applies if its generation is still the current one.
     connect_generation: Mutex<HashMap<String, u64>>,
+    /// Accounts that are meant to be connected right now.
+    ///
+    /// IRC reconnects on its own, and a retry loop needs to tell "the
+    /// connection dropped" from "somebody switched this account off" -
+    /// which look identical from inside the loop, because disconnect() on a
+    /// connected account only sends QUIT and returns. The session then ends
+    /// the same way a dead link does, and without this the account would
+    /// come straight back up seconds after being turned off.
+    wants_connected: Mutex<std::collections::HashSet<String>>,
     /// Last-known member list per buffer (the same JSON shape presenceChange
     /// events carry) - presenceChange itself is only ever *pushed* on a
     /// join/part/namreply, so a client subscribing afterward (reopening a
@@ -325,6 +334,7 @@ impl Runtime {
             task_handles: Mutex::new(HashMap::new()),
             last_connect_attempt: Mutex::new(HashMap::new()),
             connect_generation: Mutex::new(HashMap::new()),
+            wants_connected: Mutex::new(std::collections::HashSet::new()),
             buffers: Mutex::new(HashMap::new()),
             presence: Mutex::new(HashMap::new()),
             own_identity: Mutex::new(HashMap::new()),
@@ -650,6 +660,15 @@ impl Runtime {
     /// connect_generation's doc comment). A no-op otherwise: some later
     /// spawn() has already superseded this attempt, and that attempt owns
     /// the account's state now, not this stale one.
+    /// Whether this attempt is still the one that owns the account.
+    ///
+    /// Same guard finish_connection applies, exposed so a retry loop can
+    /// stand down when a newer spawn has taken over rather than reconnecting
+    /// alongside it.
+    pub fn is_current_generation(&self, account_id: &str, generation: u64) -> bool {
+        self.connect_generation.lock().unwrap().get(account_id) == Some(&generation)
+    }
+
     pub fn finish_connection(&self, state: &AppState, account_id: &str, generation: u64, new_state: ConnState, error: Option<&str>) {
         if self.connect_generation.lock().unwrap().get(account_id) != Some(&generation) {
             return;
@@ -663,6 +682,21 @@ impl Runtime {
         self.task_handles.lock().unwrap().remove(account_id);
     }
 
+    /// Records that this account should be connected, and should come back
+    /// on its own if the link drops.
+    pub fn set_wants_connected(&self, account_id: &str, wants: bool) {
+        let mut w = self.wants_connected.lock().unwrap();
+        if wants {
+            w.insert(account_id.to_string());
+        } else {
+            w.remove(account_id);
+        }
+    }
+
+    pub fn wants_connected(&self, account_id: &str) -> bool {
+        self.wants_connected.lock().unwrap().contains(account_id)
+    }
+
     /// Forcibly tears down any previous connection attempt/session for this
     /// account before a new one starts - called at the very top of
     /// backend::irc::spawn(), before next_generation(). Without this, an old
@@ -1363,6 +1397,11 @@ impl Runtime {
     /// live against Libera: this is exactly what caused a real reconnect
     /// failure - "Nickname is already in use" - after an abrupt kill).
     pub fn quit_all(&self, message: &str) {
+        // Nothing should try to come back up on the way out. IRC reconnects
+        // by itself now, and its loop reads this on the way out of a
+        // session - a QUIT sent here ends one, so without clearing the
+        // intent first the daemon would spend its shutdown reconnecting.
+        self.wants_connected.lock().unwrap().clear();
         let senders: Vec<_> = self.irc_handles.lock().unwrap().values().map(|h| h.sender.clone()).collect();
         for sender in senders {
             let _ = sender.send_quit(message);
@@ -1375,6 +1414,10 @@ impl Runtime {
     /// still stuck in DNS/TCP/TLS/registration, which has no Sender yet.
     /// Returns false if this account has neither (nothing to stop).
     pub fn disconnect(&self, state: &AppState, account_id: &str) -> bool {
+        // Before the QUIT, not after: the session ends as a result of it,
+        // and an IRC retry loop reading this flag on the way out must see
+        // that the ending was asked for.
+        self.set_wants_connected(account_id, false);
         if let Some(sender) = self.irc_sender(account_id) {
             let _ = sender.send_quit("");
             return true;
@@ -1584,6 +1627,48 @@ impl Runtime {
 mod tests {
     use super::*;
 
+    /// The flag that keeps IRC's retry loop from undoing a deliberate
+    /// disconnect. A session ending looks the same from inside the loop
+    /// whether the link died or somebody switched the account off, and only
+    /// this tells them apart.
+    #[test]
+    fn connection_intent_survives_until_it_is_cleared() {
+        let rt = Runtime::new();
+        assert!(!rt.wants_connected("irc:[email protected]"), "nothing is wanted before a spawn");
+
+        rt.set_wants_connected("irc:[email protected]", true);
+        assert!(rt.wants_connected("irc:[email protected]"));
+        // One account being switched off must not stand the others down.
+        rt.set_wants_connected("irc:[email protected]", true);
+        rt.set_wants_connected("irc:[email protected]", false);
+        assert!(!rt.wants_connected("irc:[email protected]"));
+        assert!(rt.wants_connected("irc:[email protected]"));
+    }
+
+    /// Shutdown stands every account down at once, or the daemon would
+    /// spend its exit reconnecting the sessions quit_all just ended.
+    #[test]
+    fn quitting_everything_stands_every_account_down() {
+        let rt = Runtime::new();
+        rt.set_wants_connected("irc:[email protected]", true);
+        rt.set_wants_connected("irc:[email protected]", true);
+        rt.quit_all("bye");
+        assert!(!rt.wants_connected("irc:[email protected]"));
+        assert!(!rt.wants_connected("irc:[email protected]"));
+    }
+
+    /// A newer spawn owns the account; an older attempt still unwinding must
+    /// stand down rather than reconnect alongside it.
+    #[test]
+    fn only_the_newest_attempt_owns_the_account() {
+        let rt = Runtime::new();
+        let first = rt.next_generation("irc:[email protected]");
+        assert!(rt.is_current_generation("irc:[email protected]", first));
+        let second = rt.next_generation("irc:[email protected]");
+        assert!(!rt.is_current_generation("irc:[email protected]", first));
+        assert!(rt.is_current_generation("irc:[email protected]", second));
+    }
+
     /// Discord repeats CALL_UPDATE for every change to a live call - somebody
     /// muting, somebody starting video - and each one carries the same ringing
     /// list. Announcing all of them would restart a frontend's ringtone on