diff --git a/src/nostr/runtime.rs b/src/nostr/runtime.rs index 11b1800..4219a42 100644 --- a/src/nostr/runtime.rs +++ b/src/nostr/runtime.rs @@ -176,21 +176,51 @@ pub struct NostrRendezvous { } impl NostrRendezvous { - /// Whether the primary Nostr connection task has exited (runtime liveness). + /// Whether the Nostr subsystem has stopped being able to do its job + /// (runtime liveness). /// - /// "Nostr exited" is defined as the primary `connect_task` having finished. - /// It is `Some` for the engine's whole running life (installed in `start`); - /// `shutdown` takes it, leaving `None` — a taken handle means the engine has - /// been shut down, which counts as finished, so a `None` inner maps to - /// `true` (this lets the liveness poll monitor terminate after a stop rather - /// than spinning forever). `connect_task` is a `tokio::sync::Mutex`, so this - /// sync accessor uses the non-blocking `try_lock`: a momentarily-contended - /// lock (only start/stop hold it, briefly) reports "not finished", the safe - /// direction — the 2s liveness poll re-checks next tick and never spuriously - /// degrades a healthy node. + /// "Nostr exited" is defined as *any* of the three service loops that never + /// return by design having finished: + /// + /// - `notify_task` — the inbound receive loop. Without it no advert and no + /// traversal signal is ever observed again. + /// - `publish_task` — the advert publisher. Without it this node stops being + /// discoverable. + /// - `advertise_task` — the refresh ticker that drives the publisher. + /// + /// Each of these is an unconditional `loop` (the notify loop's only `break` + /// is the relay-pool broadcast channel closing, i.e. the pool itself is + /// gone), so a finished handle means the task panicked or was aborted: + /// unrecoverable, which matches the one-way `ChildExited` → `Degraded` + /// latch in the supervisor FSM. + /// + /// Deliberately *not* watched: `connect_task` and `relay_startup_task`. + /// `Client::connect()` only spawns a per-relay background connection task + /// and returns, so `connect_task` finishes moments after start on a + /// perfectly healthy node; `relay_startup_task` breaks out of its retry loop + /// on the first successful subscribe. Watching either reports Degraded on + /// every node forever. + /// + /// Each handle is `Some` for the engine's whole running life (installed in + /// `start`); `shutdown` takes them all, leaving `None` — a taken handle + /// means the engine has been shut down, which counts as finished, so a + /// `None` inner maps to `true` (this lets the liveness poll monitor + /// terminate after a stop rather than spinning forever). The slots are + /// `tokio::sync::Mutex`es, so this sync accessor uses the non-blocking + /// `try_lock`: a momentarily-contended lock (only start/stop hold it, + /// briefly) reports "not finished", the safe direction — the 2s liveness + /// poll re-checks next tick and never spuriously degrades a healthy node. pub fn is_finished(&self) -> bool { - self.connect_task - .try_lock() + Self::task_finished(&self.notify_task) + || Self::task_finished(&self.publish_task) + || Self::task_finished(&self.advertise_task) + } + + /// Liveness of one task slot: finished if the handle is gone (shut down) or + /// the task has completed; "not finished" when the slot is momentarily + /// locked by start/stop. + fn task_finished(slot: &Mutex>>) -> bool { + slot.try_lock() .map(|g| g.as_ref().is_none_or(|h| h.is_finished())) .unwrap_or(false) } @@ -1714,6 +1744,24 @@ impl NostrRendezvous { } } + /// Install the five background-task handles that `start` would install, so + /// liveness tests can drive `is_finished()` without live relays. Each + /// argument is the handle to place in the matching slot. + pub(crate) async fn install_tasks_for_test( + &self, + connect: JoinHandle<()>, + relay_startup: JoinHandle<()>, + notify: JoinHandle<()>, + publish: JoinHandle<()>, + advertise: JoinHandle<()>, + ) { + *self.connect_task.lock().await = Some(connect); + *self.relay_startup_task.lock().await = Some(relay_startup); + *self.notify_task.lock().await = Some(notify); + *self.publish_task.lock().await = Some(publish); + *self.advertise_task.lock().await = Some(advertise); + } + /// Build a `CachedOverlayAdvert` for tests with a single endpoint and /// a generous validity window (one hour from `now_ms()`). pub(crate) fn cached_advert_for_test( diff --git a/src/nostr/tests.rs b/src/nostr/tests.rs index 29b4cab..36fca49 100644 --- a/src/nostr/tests.rs +++ b/src/nostr/tests.rs @@ -857,3 +857,106 @@ fn signal_relays_without_an_advert_still_resolves() { "the responder path passes no advert and must still produce a target set" ); } + +/// A `JoinHandle` for a task that has definitely completed. Yields until the +/// runtime has polled the no-op task to completion, so `is_finished()` is +/// deterministically `true` on return. +async fn finished_handle() -> tokio::task::JoinHandle<()> { + let handle = tokio::spawn(async {}); + while !handle.is_finished() { + tokio::task::yield_now().await; + } + handle +} + +/// A `JoinHandle` for a task that never completes. +fn live_handle() -> tokio::task::JoinHandle<()> { + tokio::spawn(std::future::pending::<()>()) +} + +/// The regression case: `connect_task` and `relay_startup_task` both return by +/// design (`Client::connect()` only kicks off per-relay connection tasks; +/// the startup loop breaks on the first successful subscribe), so a healthy +/// node has two finished handles and must still report live. +#[tokio::test] +async fn nostr_liveness_ignores_the_tasks_that_return_by_design() { + let runtime = NostrRendezvous::new_for_test(); + runtime + .install_tasks_for_test( + finished_handle().await, + finished_handle().await, + live_handle(), + live_handle(), + live_handle(), + ) + .await; + assert!( + !runtime.is_finished(), + "a node whose connect/relay-startup tasks have returned normally is healthy" + ); +} + +#[tokio::test] +async fn nostr_liveness_fires_when_the_notify_loop_dies() { + let runtime = NostrRendezvous::new_for_test(); + runtime + .install_tasks_for_test( + live_handle(), + live_handle(), + finished_handle().await, + live_handle(), + live_handle(), + ) + .await; + assert!( + runtime.is_finished(), + "a dead inbound notify loop means no advert or signal is ever received again" + ); +} + +#[tokio::test] +async fn nostr_liveness_fires_when_the_publish_loop_dies() { + let runtime = NostrRendezvous::new_for_test(); + runtime + .install_tasks_for_test( + live_handle(), + live_handle(), + live_handle(), + finished_handle().await, + live_handle(), + ) + .await; + assert!( + runtime.is_finished(), + "a dead publish loop means this node stops being discoverable" + ); +} + +#[tokio::test] +async fn nostr_liveness_fires_when_the_advertise_loop_dies() { + let runtime = NostrRendezvous::new_for_test(); + runtime + .install_tasks_for_test( + live_handle(), + live_handle(), + live_handle(), + live_handle(), + finished_handle().await, + ) + .await; + assert!( + runtime.is_finished(), + "a dead advertise ticker means the advert is never refreshed" + ); +} + +/// `shutdown` takes every handle, leaving `None`. That must read as finished so +/// the 2s liveness poll monitor terminates instead of spinning after a stop. +#[tokio::test] +async fn nostr_liveness_reports_finished_once_the_handles_are_taken() { + let runtime = NostrRendezvous::new_for_test(); + assert!( + runtime.is_finished(), + "no installed handles (post-shutdown) reads as finished" + ); +}