diff --git a/src/app/mod.rs b/src/app/mod.rs index abb79b8..8352bfc 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -955,13 +955,15 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { state.friend_presence.insert(id, presence); } UiEvent::PresenceModeReverted { mode } => { - // The Discoverable time-box elapsed; core dropped us back to - // `mode` (Normal) and stopped publishing. Mirror + persist so the - // presence picker reflects it, and tell the user why it changed. + // Core corrected the committed presence mode. Mirror + persist so + // the picker reflects the discovery state the endpoint actually has. state.config.presence_mode = mode; state.config.save(); - state.status_message = - "Discoverable timed out — back to Normal".to_string(); + state.status_message = if mode == PresenceMode::Normal { + "Discoverable timed out — back to Normal".to_string() + } else { + format!("Presence mode stayed {mode}") + }; } UiEvent::ShutdownComplete => { if state.closing { diff --git a/src/core/messages.rs b/src/core/messages.rs index 0833db6..b83fddc 100644 --- a/src/core/messages.rs +++ b/src/core/messages.rs @@ -124,11 +124,10 @@ pub enum UiEvent { /// joinable gathering (with a one-click ticket). Emitted by the outbound ping /// scheduler; absence of a recent event = treat as offline. FriendPresence { id: EndpointId, presence: FriendPresence }, - /// The Discoverable time-box elapsed (W7 P6): the core auto-reverted our presence - /// posture to the carried `mode` (always `Normal`) and stopped publishing. The - /// GUI must mirror + persist this so its presence picker stops showing - /// Discoverable. Distinct from a user-driven change so the GUI knows to update - /// without having issued the command itself. + /// Core corrected the committed presence posture. Usually the Discoverable + /// time-box elapsed and the core auto-reverted to `Normal`; on discovery apply + /// failure, this carries the previous truthful mode. The GUI must mirror + + /// persist this so its presence picker matches the endpoint's discovery state. PresenceModeReverted { mode: PresenceMode }, /// Core finished orderly app shutdown and the GUI can exit. ShutdownComplete, diff --git a/src/core/mod.rs b/src/core/mod.rs index 433e2b0..2ad052d 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -13,6 +13,7 @@ use crate::network::{ use crate::core::messages::{CoreCommand, UiEvent}; use crate::config::{NetworkMode, RecordingMode}; +use crate::presence::PresenceMode; use crate::audio::multitrack::MultitrackRecorder; use iroh::{Endpoint, EndpointAddr, EndpointId, RelayMode, SecretKey, endpoint::presets, protocol::Router}; use iroh_gossip::net::Gossip; @@ -69,10 +70,27 @@ const RECONNECT_GRACE: Duration = Duration::from_secs(45); /// 48 kHz / 60 ms frame) while bounding malicious datagram copy/decode churn. const MAX_OPUS_PAYLOAD: usize = 4000; +/// If the Discoverable time-box tries to revert but discovery service reconfiguration +/// fails, retry soon while keeping the UI in the still-possible publishing state. +const DISCOVERY_REVERT_RETRY: Duration = Duration::from_secs(60); + fn audio_datagram_len_ok(len: usize) -> bool { (4..=4 + MAX_OPUS_PAYLOAD).contains(&len) } +fn arm_discovery_retry( + discovery_deadline: &mut Option, + now: tokio::time::Instant, +) { + let retry_deadline = now + DISCOVERY_REVERT_RETRY; + if discovery_deadline + .map(|current| current > retry_deadline) + .unwrap_or(true) + { + *discovery_deadline = Some(retry_deadline); + } +} + /// Per-peer reconnect grace timers (see [`RECONNECT_GRACE`]). Shared between the /// room-event task (which arms one on a transient drop and cancels it on a /// gossip rejoin) and the conn-event task (which cancels it when the audio link @@ -466,12 +484,11 @@ impl NetStack { /// `DnsAddressLookup`, mirroring the `N0` preset) is added when `plan.resolver`; the /// n0 DNS *publisher* (`PkarrPublisher`) when `plan.publisher`. /// -/// Idempotent and reversible: it clears the whole service set and reinstalls exactly -/// what the plan wants, so flipping `publisher` off simply drops the publisher (its -/// republish task ends when the last clone is dropped, and the already-published -/// record TTL-expires within ~30s) without an endpoint rebuild and without disturbing -/// resolution. The brief clear→re-add window is a few synchronous calls; presence -/// toggles are rare, so a concurrent dial racing it is not a practical concern. +/// Idempotent and reversible: it builds the replacement services first, then clears +/// the service set and reinstalls exactly what the plan wants. Flipping `publisher` +/// off drops the publisher (its republish task ends when the last clone is dropped, +/// and the already-published record TTL-expires within ~30s) without an endpoint +/// rebuild and without disturbing resolution. fn apply_discovery( endpoint: &Endpoint, memory_lookup: &iroh::address_lookup::memory::MemoryLookup, @@ -482,16 +499,34 @@ fn apply_discovery( pkarr::{PkarrPublisher, PkarrResolver}, }; let services = endpoint.address_lookup()?; + let pkarr_resolver = if plan.resolver { + Some(PkarrResolver::n0_dns().into_address_lookup(endpoint)?) + } else { + None + }; + let dns_resolver = if plan.resolver { + Some(DnsAddressLookup::n0_dns().into_address_lookup(endpoint)?) + } else { + None + }; + let publisher = if plan.publisher { + Some(PkarrPublisher::n0_dns().into_address_lookup(endpoint)?) + } else { + None + }; + services.clear(); // Always keep the local, server-free lookup (this is what ticket/gossip dialing // depends on — it must survive every posture, including DirectOnly). services.add(memory_lookup.clone()); - if plan.resolver { - services.add(PkarrResolver::n0_dns().into_address_lookup(endpoint)?); - services.add(DnsAddressLookup::n0_dns().into_address_lookup(endpoint)?); + if let Some(pkarr_resolver) = pkarr_resolver { + services.add(pkarr_resolver); } - if plan.publisher { - services.add(PkarrPublisher::n0_dns().into_address_lookup(endpoint)?); + if let Some(dns_resolver) = dns_resolver { + services.add(dns_resolver); + } + if let Some(publisher) = publisher { + services.add(publisher); } Ok(()) } @@ -851,22 +886,64 @@ async fn run_core_loop( // W7 P6 time-box: Discoverable auto-reverts to Normal after DISCOVERY_TIMEBOX // so a publish beacon never stands indefinitely. The branch is disabled // (`if` guard) unless a deadline is armed; `unwrap_or_else` is unreachable - // belt-and-braces. On fire: stop publishing, drop to Normal, tell the GUI. + // belt-and-braces. On fire: stop publishing first, then commit Normal only + // if the endpoint's discovery services accepted the non-publishing plan. _ = tokio::time::sleep_until( discovery_deadline.unwrap_or_else(tokio::time::Instant::now), ), if discovery_deadline.is_some() => { - discovery_deadline = None; - *presence_mode.lock().unwrap() = crate::presence::PresenceMode::Normal; - let plan = crate::discovery::lookup_plan(network_mode, false); - if let Err(e) = apply_discovery(&net.endpoint, &net.memory_lookup, plan) { - crate::log_msg(&format!("discovery: time-box revert failed: {e:#}")); + let previous_mode = *presence_mode.lock().unwrap(); + if previous_mode != PresenceMode::Discoverable { + discovery_deadline = None; + continue; + } + + let requested_mode = PresenceMode::Normal; + let now = tokio::time::Instant::now(); + let plan = crate::discovery::lookup_plan( + network_mode, + requested_mode.publishes_to_discovery(), + ); + let apply_result = apply_discovery(&net.endpoint, &net.memory_lookup, plan); + let (committed_mode, transition_error) = + crate::discovery::resolve_presence_transition( + previous_mode, + requested_mode, + apply_result.is_ok(), + ); + *presence_mode.lock().unwrap() = committed_mode; + discovery_deadline = if committed_mode == PresenceMode::Discoverable { + Some(now + DISCOVERY_REVERT_RETRY) + } else { + None + }; + + match apply_result { + Ok(()) => { + crate::log_msg( + "discovery: Discoverable time-box elapsed → reverting to Normal", + ); + let _ = ui_tx + .send(UiEvent::PresenceModeReverted { + mode: PresenceMode::Normal, + }) + .await; + } + Err(e) => { + crate::log_msg(&format!("discovery: time-box revert failed: {e:#}")); + if committed_mode != requested_mode { + let _ = ui_tx + .send(UiEvent::PresenceModeReverted { + mode: committed_mode, + }) + .await; + } + if let Some(message) = transition_error { + let _ = ui_tx + .send(UiEvent::Error(format!("{message} ({e:#})"))) + .await; + } + } } - crate::log_msg("discovery: Discoverable time-box elapsed → reverting to Normal"); - let _ = ui_tx - .send(UiEvent::PresenceModeReverted { - mode: crate::presence::PresenceMode::Normal, - }) - .await; continue; } }; @@ -1798,22 +1875,60 @@ async fn run_core_loop( } CoreCommand::SetPresenceMode(mode) => { - *presence_mode.lock().unwrap() = mode; - // W7 P6: re-apply n0 DNS discovery for the new posture (publish on iff - // Discoverable). Runtime — no endpoint rebuild; clears + reinstalls the - // address-lookup services. The resolver stays on regardless so we can - // still look up moved friends. - let plan = crate::discovery::lookup_plan(network_mode, mode.publishes_to_discovery()); - if let Err(e) = apply_discovery(&net.endpoint, &net.memory_lookup, plan) { - crate::log_msg(&format!("discovery: apply failed: {e:#}")); + let previous_mode = *presence_mode.lock().unwrap(); + let now = tokio::time::Instant::now(); + + if previous_mode == mode { + // Same-mode requests are no-ops for discovery wiring, but keep the + // existing UX: re-selecting Discoverable restarts the clock. + discovery_deadline = if mode == PresenceMode::Discoverable { + Some(now + crate::discovery::DISCOVERY_TIMEBOX) + } else { + None + }; + continue; } - // Arm (Discoverable) or cancel (any other posture) the auto-revert - // time-box. Re-selecting Discoverable restarts the clock. - discovery_deadline = if mode == crate::presence::PresenceMode::Discoverable { - Some(tokio::time::Instant::now() + crate::discovery::DISCOVERY_TIMEBOX) + + // W7 P6/S11: re-apply n0 DNS discovery for the requested posture + // first, then commit the presence mode only if the endpoint accepted + // that discovery plan. This keeps the UI truthful when dropping the + // publisher fails. + let plan = + crate::discovery::lookup_plan(network_mode, mode.publishes_to_discovery()); + let apply_result = apply_discovery(&net.endpoint, &net.memory_lookup, plan); + let (committed_mode, transition_error) = + crate::discovery::resolve_presence_transition( + previous_mode, + mode, + apply_result.is_ok(), + ); + *presence_mode.lock().unwrap() = committed_mode; + + if committed_mode == PresenceMode::Discoverable { + if apply_result.is_ok() && mode == PresenceMode::Discoverable { + discovery_deadline = Some(now + crate::discovery::DISCOVERY_TIMEBOX); + } else { + arm_discovery_retry(&mut discovery_deadline, now); + } } else { - None - }; + discovery_deadline = None; + } + + if let Err(e) = apply_result { + crate::log_msg(&format!("discovery: apply failed: {e:#}")); + if committed_mode != mode { + let _ = ui_tx + .send(UiEvent::PresenceModeReverted { + mode: committed_mode, + }) + .await; + } + if let Some(message) = transition_error { + let _ = ui_tx + .send(UiEvent::Error(format!("{message} ({e:#})"))) + .await; + } + } } CoreCommand::SetRecordingMode(mode) => { diff --git a/src/discovery.rs b/src/discovery.rs index d87c00b..07326c5 100644 --- a/src/discovery.rs +++ b/src/discovery.rs @@ -8,14 +8,19 @@ //! The model (from `docs/contacts-plan.md` P6, decided 2026-06-16): //! - **Resolving is always allowed on relay-capable modes** — a stationary friend //! (typically in `Normal`) must be able to look up a friend who moved networks. A -//! resolve is a DNS query to n0 that publishes nothing; it only fires when a saved -//! address is stale and the dial falls through to discovery. +//! resolve is a DNS query to n0 that publishes nothing, but still exposes query +//! timing/source metadata to n0; it only fires when a saved address is stale and +//! the dial falls through to discovery. //! - **Publishing is gated on `Discoverable`** and asymmetric: only the mover //! publishes their address to n0 DNS; everyone else just looks it up. +//! - **Stopping publishing removes the local publisher service**; iroh does not +//! expose an explicit unpublish call here, so already-published pkarr records can +//! linger until their default ~30s TTL expires. //! - **`DirectOnly` is the explicit no-server posture** — neither resolve nor publish //! ever touches n0 there, regardless of the Discoverable toggle. use crate::config::NetworkMode; +use crate::presence::PresenceMode; use std::time::Duration; /// How long `Discoverable` stays on before auto-reverting to `Normal`. Discovery is @@ -46,12 +51,43 @@ pub fn lookup_plan(network_mode: NetworkMode, want_publish: bool) -> LookupPlan match network_mode { // The explicit serverless posture: no n0 contact at all, even to resolve. // A Discoverable toggle here is intentionally inert. - NetworkMode::DirectOnly => LookupPlan { resolver: false, publisher: false }, + NetworkMode::DirectOnly => LookupPlan { + resolver: false, + publisher: false, + }, // Relay-capable: always resolve (so a stationary friend can find a mover); // publish only when the user opted into Discoverable. - NetworkMode::RelayNoDiscovery | NetworkMode::N0Full => { - LookupPlan { resolver: true, publisher: want_publish } - } + NetworkMode::RelayNoDiscovery | NetworkMode::N0Full => LookupPlan { + resolver: true, + publisher: want_publish, + }, + } +} + +/// Decide which presence mode may be committed after attempting to apply discovery +/// services for `requested`. +/// +/// On failure, keep the previous mode: it is the only locally truthful state because +/// the endpoint's discovery services may still reflect the old posture. Same-mode +/// requests are no-ops from a presence-truth perspective and do not surface an error. +pub fn resolve_presence_transition( + previous: PresenceMode, + requested: PresenceMode, + apply_ok: bool, +) -> (PresenceMode, Option) { + if previous == requested { + return (previous, None); + } + + if apply_ok { + (requested, None) + } else { + ( + previous, + Some(format!( + "Couldn't update discovery mode; keeping {previous}." + )), + ) } } @@ -64,12 +100,18 @@ mod tests { for mode in [NetworkMode::RelayNoDiscovery, NetworkMode::N0Full] { assert_eq!( lookup_plan(mode, false), - LookupPlan { resolver: true, publisher: false }, + LookupPlan { + resolver: true, + publisher: false + }, "{mode:?}: resolve always on, no publish when not Discoverable" ); assert_eq!( lookup_plan(mode, true), - LookupPlan { resolver: true, publisher: true }, + LookupPlan { + resolver: true, + publisher: true + }, "{mode:?}: Discoverable adds publish on top of resolve" ); } @@ -79,12 +121,18 @@ mod tests { fn direct_only_never_touches_n0_even_when_discoverable() { assert_eq!( lookup_plan(NetworkMode::DirectOnly, false), - LookupPlan { resolver: false, publisher: false } + LookupPlan { + resolver: false, + publisher: false + } ); // The serverless posture overrides the Discoverable request entirely. assert_eq!( lookup_plan(NetworkMode::DirectOnly, true), - LookupPlan { resolver: false, publisher: false } + LookupPlan { + resolver: false, + publisher: false + } ); } @@ -92,4 +140,42 @@ mod tests { fn timebox_is_thirty_minutes() { assert_eq!(DISCOVERY_TIMEBOX, Duration::from_secs(1800)); } + + #[test] + fn presence_transition_commits_requested_mode_after_successful_apply() { + assert_eq!( + resolve_presence_transition(PresenceMode::Normal, PresenceMode::Discoverable, true), + (PresenceMode::Discoverable, None) + ); + } + + #[test] + fn presence_transition_keeps_previous_mode_when_apply_fails() { + let (mode, err) = + resolve_presence_transition(PresenceMode::Normal, PresenceMode::Discoverable, false); + + assert_eq!(mode, PresenceMode::Normal); + assert!(err.unwrap().contains("keeping Normal")); + } + + #[test] + fn presence_transition_keeps_discoverable_when_off_transition_fails() { + let (mode, err) = + resolve_presence_transition(PresenceMode::Discoverable, PresenceMode::Normal, false); + + assert_eq!(mode, PresenceMode::Discoverable); + assert!(err.unwrap().contains("keeping Discoverable")); + } + + #[test] + fn presence_transition_same_mode_is_noop_without_error() { + assert_eq!( + resolve_presence_transition( + PresenceMode::Discoverable, + PresenceMode::Discoverable, + false + ), + (PresenceMode::Discoverable, None) + ); + } }