S11: make presence/discovery state honest on apply failure

SetPresenceMode and the Discoverable time-box auto-revert both committed
the new presence_mode to local state *before* apply_discovery and only
log_msg'd on failure, so a failed off-transition could leave the n0 DNS
PkarrPublisher running while the UI showed not-discoverable (privacy /
reality mismatch — security-open-handoff S11, from the W7 P7 review).

Fix (Codex, senior-reviewed):
- discovery.rs: pure resolve_presence_transition(prev, requested, apply_ok)
  -> (mode, Option<error>) seam — on failure keep the previous (truthful)
  mode and surface a message. +4 unit tests.
- apply_discovery now builds the replacement resolver/publisher services
  BEFORE clearing the service set, so a builder failure leaves the old
  posture fully intact (no partial state) — "keep previous mode" is then
  provably truthful.
- Both SetPresenceMode and the time-box revert apply discovery first, route
  through the seam, commit only the truthful mode, and surface failures via
  the existing PresenceModeReverted (corrects the picker) + UiEvent::Error.
  No new wire/event variant.
- A failed off-transition stays Discoverable and arms a 60s retry
  (DISCOVERY_REVERT_RETRY) so the beacon never stands stuck.
- P3 notes documented: relay-resolve exposes n0 query metadata (by design);
  no explicit iroh unpublish API exists, so the bounded ~30s pkarr TTL
  linger is documented, not behavior-changed; DirectOnly stays no-n0.

306 lib tests / clippy --all-targets / release all green (re-run by senior).
Runtime publish-stop behavior still wants a 2-machine / packet-capture check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-18 03:32:38 -04:00
co-authored by Claude Opus 4.8
parent d56c2c90b2
commit 319d0c5e29
4 changed files with 259 additions and 57 deletions
+96 -10
View File
@@ -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<String>) {
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)
);
}
}