From ceaaced941ee9cceff5f68759aed0951b9eca574 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Tue, 25 Aug 2026 11:43:50 +1000 Subject: [PATCH] fix(mesh): make open-relay mode consistent across discovery, reconcile, and probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the open-mesh change (d99cba5bb), closing three gaps found in review: 1. A stray membership snapshot on an open relay no longer gates routing. Admission runs TrustPolicy::Off there, but discovery still intersected status notes with any kind:13534 roster that happened to exist (added out of band), and the query layer applied the same authors filter — a hybrid where admission admits everyone while discovery hides non-roster peers. Both branches now check the relay mode: open relays ignore membership entirely and rely on freshness plus owner-signed endpoint binding. 2. Reconcile recovers a node isolated by the startup probe fallback. A transient NIP-11 probe failure at start falls back to the enforcing path and a self-only allowlist; treating Ok(None) as an unconditional Keep then pinned that node at "1 NODE" until an app restart — the original bug, reintroduced by one HTTP blip. A self-only roster admits nobody else, so restarting into open mesh relaxes nothing that protected anyone. An allowlist admitting other owners is still never relaxed mid-flight. 3. Successful NIP-11 probes are memoized per relay URL. The deployment mode does not flap at runtime, and discovery re-probed on every reconcile poll. Failures are not cached, so the fallback in (2) re-probes and recovers. Co-authored-by: Michael Neale Signed-off-by: Michael Neale --- desktop/src-tauri/src/commands/mesh_llm.rs | 31 +++++++++-- desktop/src-tauri/src/mesh_llm/coordinator.rs | 52 +++++++++++++++++-- desktop/src-tauri/src/mesh_llm/discovery.rs | 27 ++++++---- desktop/src-tauri/src/mesh_llm/mod_tests.rs | 39 ++++++++++++++ 4 files changed, 131 insertions(+), 18 deletions(-) diff --git a/desktop/src-tauri/src/commands/mesh_llm.rs b/desktop/src-tauri/src/commands/mesh_llm.rs index 3789121a52..8772bf819f 100644 --- a/desktop/src-tauri/src/commands/mesh_llm.rs +++ b/desktop/src-tauri/src/commands/mesh_llm.rs @@ -219,9 +219,29 @@ async fn query_mesh_discovery_events_at( /// /// A failed probe must not open the mesh up: an unreachable or malformed NIP-11 /// document leaves admission exactly as strict as it is today. +/// +/// Successful probes are memoized per relay URL: the deployment mode +/// (`BUZZ_REQUIRE_RELAY_MEMBERSHIP`) does not flap at runtime, and the +/// discovery/reconcile paths would otherwise re-issue this HTTP GET on every +/// poll. Failures are NOT cached, so a node that fell back to enforcing on a +/// transient blip re-probes on the next poll and can recover. async fn resolved_relay_mesh_mode(relay_url: &str) -> mesh_llm::MeshRelayMode { + static CACHE: std::sync::OnceLock< + std::sync::Mutex>, + > = std::sync::OnceLock::new(); + let cache = CACHE.get_or_init(Default::default); + if let Ok(guard) = cache.lock() { + if let Some(mode) = guard.get(relay_url).copied() { + return mode; + } + } match probe_relay_mesh_mode(relay_url).await { - Ok(mode) => mode, + Ok(mode) => { + if let Ok(mut guard) = cache.lock() { + guard.insert(relay_url.to_string(), mode); + } + mode + } Err(error) => { eprintln!("buzz-mesh: NIP-11 probe failed; assuming membership is enforced: {error}"); mesh_llm::MeshRelayMode::ClosedMembershipEnforced @@ -260,9 +280,12 @@ async fn query_mesh_discovery_events_for_mode( } } let mut status_filter = mesh_llm::mesh_status_filter(); - // On an enforcing relay, scope status reads to current members. Without a - // roster there is nobody to scope to, so read every reporter's note. - if !member_pubkeys.is_empty() { + // On an enforcing relay, scope status reads to current members. On an open + // relay the snapshot is not a trust boundary even when one exists (added + // out of band) — admission runs unenforced there, so scoping reads to a + // stray roster would hide peers the mesh admits. Read every reporter's + // note and let freshness + endpoint-binding checks gate routing. + if !mode.is_open() && !member_pubkeys.is_empty() { status_filter["authors"] = serde_json::json!(member_pubkeys); } let mut previous_cursor: Option<(u64, String)> = None; diff --git a/desktop/src-tauri/src/mesh_llm/coordinator.rs b/desktop/src-tauri/src/mesh_llm/coordinator.rs index 1cf0751656..4bf870172b 100644 --- a/desktop/src-tauri/src/mesh_llm/coordinator.rs +++ b/desktop/src-tauri/src/mesh_llm/coordinator.rs @@ -234,17 +234,24 @@ fn roster_shrinks(current: &[String], fresh: &[String]) -> bool { /// /// Rules: /// - query failed (`Err`) → `Keep` (never de-admit on a relay blip) -/// - relay enforces no membership (`Ok(None)`) → `Keep` (see below) +/// - relay enforces no membership (`Ok(None)`), roster beyond self → `Keep` (see below) +/// - relay enforces no membership (`Ok(None)`), self-only roster → `RestartProcess` (see below) /// - resolved roster == current → `Keep` (no-op) /// - grows (only additions) → `RestartProcess` immediately (fast admission) /// - shrinks/empties, first observation → `AwaitConfirm` (hold, re-check next poll) /// - shrinks/empties, confirmed → `RestartProcess` (same reduced roster twice) /// /// `Ok(None)` means the relay does not enforce NIP-43 membership, so there is -/// no roster to reconcile against. A node that is *already* enforcing an -/// allowlist keeps it: relaxing a live node's admission on the strength of a -/// NIP-11 read would let a relay downgrade (or a spoofed document) de-restrict -/// a running mesh. The open-relay path is chosen at start, not mid-flight. +/// no roster to reconcile against. A node enforcing an allowlist that admits +/// *other* owners keeps it: relaxing a live multi-party allowlist on the +/// strength of a NIP-11 read would let a relay downgrade (or a spoofed +/// document) de-restrict a running mesh. A **self-only** allowlist is +/// different: it admits nobody else, so it protects nothing — and it is +/// exactly the fallback a transient NIP-11 probe failure at startup produces +/// on an open relay. Without recovery here that node stays at "1 NODE" until +/// an app restart (the original bug, reintroduced by a single HTTP blip). +/// Restarting resolves the roster afresh, which on an open relay yields +/// `None` → `TrustPolicy::Off`. fn roster_reconcile_action( current_owners: &[String], pending_shrink: Option<&[String]>, @@ -257,6 +264,13 @@ fn roster_reconcile_action( ); return RosterReconcileAction::Keep; } + Ok(None) if current_owners.is_empty() => { + eprintln!( + "buzz-mesh: relay does not enforce membership and this node is isolated by the \ + startup fallback; restarting to run the mesh unenforced" + ); + return RosterReconcileAction::RestartProcess; + } Ok(None) => { eprintln!( "buzz-mesh: relay does not enforce membership; keeping this node's current allowlist" @@ -571,6 +585,34 @@ mod tests { ); } + // Regression: a transient NIP-11 probe failure at startup on an OPEN relay + // falls back to the enforcing path and starts the node with a self-only + // allowlist. If reconcile then treated `Ok(None)` as an unconditional + // `Keep`, that node would sit at "1 NODE" until an app restart — the + // original open-relay bug, reintroduced by a single HTTP blip. A self-only + // roster admits nobody else, so restarting to run unenforced relaxes + // nothing that was protecting anyone. + #[test] + fn open_relay_recovers_a_fallback_isolated_node() { + let self_only: Vec = Vec::new(); + let action = roster_reconcile_action(&self_only, None, Ok(None)); + assert_eq!( + action, + RosterReconcileAction::RestartProcess, + "a fallback-isolated node on an open relay must restart into open mesh" + ); + } + + // The other side of that rule: an allowlist that admits OTHER owners is + // never relaxed on the strength of a NIP-11 read. A relay downgrade (or a + // spoofed document) must not de-restrict a running mesh. + #[test] + fn open_relay_report_keeps_a_multi_party_allowlist() { + let current = vec!["owner-a".to_string(), "owner-b".to_string()]; + let action = roster_reconcile_action(¤t, None, Ok(None)); + assert_eq!(action, RosterReconcileAction::Keep); + } + #[test] fn unchanged_roster_is_a_noop() { let current = vec!["owner-a".to_string()]; diff --git a/desktop/src-tauri/src/mesh_llm/discovery.rs b/desktop/src-tauri/src/mesh_llm/discovery.rs index e917229a6d..e226c7f652 100644 --- a/desktop/src-tauri/src/mesh_llm/discovery.rs +++ b/desktop/src-tauri/src/mesh_llm/discovery.rs @@ -240,16 +240,25 @@ pub fn availability_from_events_for_mode( if events.is_empty() { return MeshAvailability::unavailable("Buzz shared compute status is not published yet"); } - let members = match latest_membership_list(&events) { - Some(members) => Some(members), - // An enforcing relay always publishes a snapshot (even a zero-member - // one), so its absence there means an incomplete read: stay closed. - None if !mode.is_open() => { - return MeshAvailability::unavailable( - "Buzz shared compute is waiting for the current member roster", - ); + // On an open relay the snapshot is not a trust boundary even when one + // happens to exist (added out of band): admission runs `TrustPolicy::Off` + // there, so intersecting routing with a stray roster would hide peers the + // mesh happily admits. Ignore membership entirely and accept status notes + // on their own merits. + let members = if mode.is_open() { + None + } else { + match latest_membership_list(&events) { + Some(members) => Some(members), + // An enforcing relay always publishes a snapshot (even a + // zero-member one), so its absence means an incomplete read: + // stay closed. + None => { + return MeshAvailability::unavailable( + "Buzz shared compute is waiting for the current member roster", + ); + } } - None => None, }; // Status is replaceable per member pubkey, so a query returns multiple diff --git a/desktop/src-tauri/src/mesh_llm/mod_tests.rs b/desktop/src-tauri/src/mesh_llm/mod_tests.rs index 97ae2290e8..e28569c5fd 100644 --- a/desktop/src-tauri/src/mesh_llm/mod_tests.rs +++ b/desktop/src-tauri/src/mesh_llm/mod_tests.rs @@ -725,6 +725,45 @@ fn open_relay_routes_without_a_membership_snapshot() { assert_eq!(open.serve_targets[0].endpoint_addr, endpoint); } +#[test] +fn open_relay_ignores_a_stray_membership_snapshot() { + use super::MeshRelayMode; + + // On an open relay a kind:13534 snapshot can still exist (members added + // out of band), but it is not a trust boundary there: admission runs + // `TrustPolicy::Off` regardless. Intersecting routing with that stray + // roster would hide peers the mesh happily admits — a hybrid state where + // admission and discovery disagree. Routing must ignore it entirely. + let reporter_secret = "a".repeat(64); + let reporter_pubkey = nostr::Keys::parse(&reporter_secret) + .unwrap() + .public_key() + .to_hex(); + let endpoint = test_endpoint_token(); + let events = vec![ + // Snapshot deliberately lists someone else, NOT the reporter. + signed_membership_event(&["f".repeat(64)]), + signed_reporter_target(&reporter_secret, "model-open", &endpoint), + ]; + + let closed = super::availability_from_events_for_mode( + events.clone(), + MeshRelayMode::ClosedMembershipEnforced, + ); + assert!( + closed.serve_targets.is_empty(), + "an enforcing relay still intersects routing with the roster" + ); + + let open = super::availability_from_events_for_mode(events, MeshRelayMode::OpenNoMembership); + assert_eq!( + open.serve_targets.len(), + 1, + "an open relay accepts a non-roster reporter's status: {reporter_pubkey} was hidden" + ); + assert_eq!(open.serve_targets[0].endpoint_addr, endpoint); +} + #[test] fn closed_relay_without_snapshot_never_silently_opens_up() { use super::MeshRelayMode;