Count one expired session once, not once per package per tick

`autosync_login_required` fired on every backoff-due tick, for every package on
the deployment — so one expiry became as many events as the user had packages,
times as many ticks as they took to log back in. Bounded by backoff, unbounded in
meaning.

The episode is per **deployment**, not per package: one expired session blocks
every package on that host, and they reach the code one per loop iteration. So
`login_episode` asks whether any other namespace is already blocked on this host
— before recording the failing one, or it would look like a continuation of
itself — and only `Began` emits. The state it reads already existed;
`login_blocked` is keyed by namespace with the host as its value.

`LoginBlock` rather than a bool, because `report_login_required(host, true)` says
nothing at a call site. The UI and log reporters take it and ignore it: they
re-render the same affordance idempotently, so nothing changes about when anyone
is *told* — only about what is counted.

Tested on the classifier rather than through `run_once`, and the reason is worth
recording: the loop prunes `login_blocked` to currently-installed namespaces on
entry, so a sibling cannot be injected without a second installed package, and
installation is not the property under test. My first attempt at a loop-level
test failed exactly there. Three cases: fresh host begins, sibling on the same
host continues, a different host begins its own. Plus one for the unattributed
case — two unknowns collapse, but an unknown must never be taken for a known
host.

`Began` is still covered end-to-end by the existing login-required tick test.

just lint 0, cargo fmt --check 0, 312 quilt-sync tests, workspace green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: fiskus

quilt-sync/src-tauri/src/autopull/reporter.rs

@@ -15,6 +15,21 @@ use crate::telemetry::event::{
 };
 use crate::telemetry::prelude::*;
 
+/// Whether a deployment's session has *just* become unusable, or was already.
+///
+/// The distinction exists for telemetry: the loop rediscovers an expired session
+/// on every backoff-due tick, for every package on that deployment, so reporting
+/// each discovery would count one expiry many times over. The UI wants the
+/// opposite — it re-renders the same affordance idempotently and does not care —
+/// so this informs telemetry without changing when anyone is told.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum LoginBlock {
+    /// No package on this deployment was blocked before. The episode starts here.
+    Began,
+    /// Another package on the same deployment is already blocked, or this one was.
+    Continues,
+}
+
 /// Event names. Kept in lockstep with the UI's `listen(...)` calls.
 pub const STATUS_EVENT: &str = "package-status-changed";
 pub const LOGIN_REQUIRED_EVENT: &str = "autosync-login-required";
@@ -201,7 +216,10 @@ pub trait StatusReporter: Send + Sync + 'static {
     /// lineage has no origin before doing work, so an outcome it reports always
     /// concerns a known deployment.
     fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason);
-    fn report_login_required(&self, host: Option<&Host>);
+    /// `block` distinguishes the deployment's session *becoming* unusable from
+    /// the loop retrying while it stays that way. Only the transition is worth
+    /// counting; the retries are a log line.
+    fn report_login_required(&self, host: Option<&Host>, block: LoginBlock);
     fn report_subscriber_error(&self, event: SubscriberErrorEvent) {
         warn!(
             "fswatcher: kind={} namespace={:?} message={}",
@@ -233,11 +251,11 @@ impl StatusReporter for LogReporter {
         info!("autosync: paused namespace={namespace} host={host} reason={reason:?}");
     }
 
-    fn report_login_required(&self, host: Option<&Host>) {
+    fn report_login_required(&self, host: Option<&Host>, block: LoginBlock) {
         if let Some(h) = host {
-            warn!("autosync: login required for {h}");
+            warn!("autosync: login required for {h} ({block:?})");
         } else {
-            warn!("autosync: login required");
+            warn!("autosync: login required ({block:?})");
         }
     }
 }
@@ -298,11 +316,16 @@ impl StatusReporter for TelemetryReporter {
         self.inner.report_paused(namespace, host, reason);
     }
 
-    fn report_login_required(&self, host: Option<&Host>) {
-        self.emit(MixpanelEvent::AutosyncLoginRequired(AutosyncAuthEvent {
-            host: host.cloned(),
-        }));
-        self.inner.report_login_required(host);
+    fn report_login_required(&self, host: Option<&Host>, block: LoginBlock) {
+        // Once per deployment per episode. The loop rediscovers the same expired
+        // session on every backoff-due tick and for every package on that host, so
+        // counting discoveries would report one expiry as many.
+        if block == LoginBlock::Began {
+            self.emit(MixpanelEvent::AutosyncLoginRequired(AutosyncAuthEvent {
+                host: host.cloned(),
+            }));
+        }
+        self.inner.report_login_required(host, block);
     }
 
     fn report_subscriber_error(&self, event: SubscriberErrorEvent) {
@@ -356,11 +379,11 @@ impl StatusReporter for TauriEventReporter {
         }
     }
 
-    fn report_login_required(&self, host: Option<&Host>) {
+    fn report_login_required(&self, host: Option<&Host>, block: LoginBlock) {
         if let Some(h) = host {
-            warn!("autosync: login required for {h}");
+            warn!("autosync: login required for {h} ({block:?})");
         } else {
-            warn!("autosync: login required");
+            warn!("autosync: login required ({block:?})");
         }
         // TODO(autosync/03-merge-conflicts.md): no UI listener yet.
         let payload = LoginRequiredEvent {
@@ -611,6 +634,9 @@ pub(crate) mod test_support {
         pub statuses: Mutex<Vec<(Namespace, PackageStatusEvent)>>,
         pub paused: Mutex<Vec<(Namespace, PausedReason)>>,
         pub logins: Mutex<Vec<Option<Host>>>,
+        /// Whether each login report was the start of an episode or a repeat, so a
+        /// test can assert one expiry is counted once.
+        pub login_blocks: Mutex<Vec<LoginBlock>>,
         pub subscriber_errors: Mutex<Vec<SubscriberErrorEvent>>,
         pub published: Mutex<Vec<(Namespace, String)>>,
         /// Hosts seen on the outcomes that carry one, so a test can assert the
@@ -635,7 +661,8 @@ pub(crate) mod test_support {
                 .push((namespace.clone(), reason));
         }
 
-        fn report_login_required(&self, host: Option<&Host>) {
+        fn report_login_required(&self, host: Option<&Host>, block: LoginBlock) {
+            self.login_blocks.lock().unwrap().push(block);
             self.logins.lock().unwrap().push(host.cloned());
         }
 

quilt-sync/src-tauri/src/autopull/tick.rs

@@ -10,6 +10,7 @@ use quilt_uri::Namespace;
 use crate::Error;
 use crate::autopull::PausedReason;
 use crate::autopull::WatcherInner;
+use crate::autopull::reporter::LoginBlock;
 use crate::autopull::reporter::PackageStatusEvent;
 use crate::autopull::reporter::clean_uptodate_fingerprint;
 use crate::autopull::reporter::status_fingerprint;
@@ -53,6 +54,28 @@ impl RefreshOutcome {
     }
 }
 
+/// Whether a login failure *starts* an episode for its deployment, or joins one.
+///
+/// Per deployment rather than per package, and that is the whole point: one expired
+/// session blocks every package on the host, and they reach this code one per loop
+/// iteration. A per-package answer would report one expiry as many.
+///
+/// Asked *before* the failing namespace is recorded, so its own entry cannot make
+/// it look like a continuation of itself.
+fn login_episode(
+    blocked: &std::collections::BTreeMap<Namespace, Option<Host>>,
+    host: Option<&Host>,
+) -> LoginBlock {
+    if blocked
+        .values()
+        .any(|blocked_host| blocked_host.as_ref() == host)
+    {
+        LoginBlock::Continues
+    } else {
+        LoginBlock::Began
+    }
+}
+
 #[derive(Debug)]
 pub(crate) enum WatchError {
     Conflict(PausedReason),
@@ -515,12 +538,18 @@ pub(crate) async fn run_once(
             Err(WatchError::LoginRequired(host)) => {
                 // Backoff until the user re-auths; the Ok arm clears it.
                 bump_backoff(&mut *inner.backoff.write().await, &namespace, now);
-                inner
-                    .login_blocked
-                    .write()
-                    .await
-                    .insert(namespace.clone(), host.clone());
-                inner.reporter.report_login_required(host.as_ref());
+                // The episode is per *deployment*, not per package: one expired
+                // session blocks every package on that host, and they arrive one
+                // per loop iteration. Asking whether any other namespace is
+                // already blocked on this host — before inserting this one — is
+                // what makes it countable once.
+                let block = {
+                    let mut blocked = inner.login_blocked.write().await;
+                    let block = login_episode(&blocked, host.as_ref());
+                    blocked.insert(namespace.clone(), host.clone());
+                    block
+                };
+                inner.reporter.report_login_required(host.as_ref(), block);
                 inner.aggregator.note_login_required(&namespace, host);
             }
             Err(WatchError::Conflict(reason)) => {

quilt-sync/src-tauri/src/autopull/tick/tests.rs

@@ -1053,3 +1053,50 @@ async fn conflict_emit_carries_stable_fingerprint() -> Result<(), Error> {
     assert!(!event.fingerprint.is_empty());
     Ok(())
 }
+
+/// One expired session is **one** episode, however many packages it blocks.
+///
+/// Tested on the classifier rather than through `run_once`, because the loop
+/// prunes `login_blocked` to currently-installed namespaces on entry — so a
+/// sibling cannot be injected without a second installed package, and the
+/// property under test is not about installation.
+#[test]
+fn login_episode_counts_per_deployment_not_per_package() {
+    let host: Host = "catalog.dev".parse().unwrap();
+    let other: Host = "elsewhere.dev".parse().unwrap();
+
+    let mut blocked = BTreeMap::new();
+    assert_eq!(
+        login_episode(&blocked, Some(&host)),
+        LoginBlock::Began,
+        "nothing is blocked yet, so this failure starts the episode"
+    );
+
+    blocked.insert(("acme", "first").into(), Some(host.clone()));
+    assert_eq!(
+        login_episode(&blocked, Some(&host)),
+        LoginBlock::Continues,
+        "a sibling on the same deployment means the session was already known bad"
+    );
+    assert_eq!(
+        login_episode(&blocked, Some(&other)),
+        LoginBlock::Began,
+        "a different deployment's session expiring is its own episode"
+    );
+}
+
+/// An unattributed failure is its own episode, and does not merge with an
+/// attributed one — two unknowns are indistinguishable, so they collapse, but an
+/// unknown must never be taken for a known host.
+#[test]
+fn an_unattributed_login_failure_does_not_join_a_hosts_episode() {
+    let host: Host = "catalog.dev".parse().unwrap();
+    let mut blocked = BTreeMap::new();
+    blocked.insert(("acme", "first").into(), Some(host));
+
+    assert_eq!(
+        login_episode(&blocked, None),
+        LoginBlock::Began,
+        "a failure that could not name its deployment is not that deployment's"
+    );
+}

quilt-sync/src-tauri/src/autopull/tick/tests/publish.rs

@@ -682,6 +682,11 @@ async fn run_once_login_required_on_publish() -> Result<(), Error> {
     let logins = reporter.logins.lock().unwrap();
     assert_eq!(logins.len(), 1);
     assert_eq!(logins[0].as_ref(), Some(&host));
+    assert_eq!(
+        *reporter.login_blocks.lock().unwrap(),
+        vec![LoginBlock::Began],
+        "the first discovery starts the episode"
+    );
     Ok(())
 }