fix(net): room creator can rejoin a room they left (A8)

The gossip bootstrap list was derived solely from the ticket: a client
dialed the host, but "we are the host" produced an EMPTY list. So when the
room CREATOR rejoined their own room (their ticket names themselves as host)
they dialed nobody and never re-entered the swarm — the remaining peer stayed
stuck until it too left and rejoined. (First 2-human field test, 2026-06-14;
user-confirmed call-breaking, P1.)

Fix: retain the peers seen in the current room across leave (core
`known_peers`, updated by the event task; reset only when the joined ticket
changes) and pass them to `RoomState::join` as extra bootstrap targets. The
new pure `compute_bootstrap` seam unions the ticket host + retained peers,
drops self, and de-dups; address resolution rides the persistent lookup.

- `RoomState::join` gains `extra_bootstrap: Vec<EndpointAddr>` (test_net
  callers pass vec![]).
- +4 unit tests on `compute_bootstrap`, incl. the host-rejoin regression case.

Tests-green (208 lib + 6 + 4 integration), clippy clean. NOT yet 2-machine
field-verified — needs a live host leave→rejoin call.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-14 14:57:37 -04:00
co-authored by Claude Opus 4.8
parent 88e0adec5d
commit 7c8f0114f7
4 changed files with 138 additions and 19 deletions
+2 -2
View File
@@ -63,7 +63,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
addr: endpoint_a.addr(), addr: endpoint_a.addr(),
sharing: None, sharing: None,
}; };
room_a.join(&ticket_str, state_a).await?; room_a.join(&ticket_str, state_a, vec![]).await?;
println!("Node A joined topic."); println!("Node A joined topic.");
// Subscribe to events on Node A // Subscribe to events on Node A
@@ -81,7 +81,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
addr: endpoint_b.addr(), addr: endpoint_b.addr(),
sharing: None, sharing: None,
}; };
room_b.join(&ticket_str, state_b).await?; room_b.join(&ticket_str, state_b, vec![]).await?;
println!("Node B joined topic."); println!("Node B joined topic.");
// Subscribe to events on Node B // Subscribe to events on Node B
+34 -3
View File
@@ -13,7 +13,7 @@ use crate::core::messages::{CoreCommand, UiEvent};
use crate::config::{NetworkMode, RecordingMode}; use crate::config::{NetworkMode, RecordingMode};
use crate::audio::multitrack::MultitrackRecorder; use crate::audio::multitrack::MultitrackRecorder;
use iroh::{Endpoint, EndpointId, RelayMode, endpoint::presets, protocol::Router}; use iroh::{Endpoint, EndpointAddr, EndpointId, RelayMode, endpoint::presets, protocol::Router};
use iroh_gossip::net::Gossip; use iroh_gossip::net::Gossip;
use tokio::sync::{mpsc, Mutex}; use tokio::sync::{mpsc, Mutex};
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
@@ -415,6 +415,15 @@ async fn run_core_loop(
) -> Result<(), anyhow::Error> { ) -> Result<(), anyhow::Error> {
let memory_lookup = iroh::address_lookup::memory::MemoryLookup::new(); let memory_lookup = iroh::address_lookup::memory::MemoryLookup::new();
let secret_key = iroh::SecretKey::generate(); let secret_key = iroh::SecretKey::generate();
// Peers seen in the current/most-recent room, retained ACROSS leave so a
// rejoin can bootstrap to them. This is the fix for A8: the room creator's own
// ticket lists only themselves as host, so on rejoin these retained peers are
// their only dial targets. Reset when the joined ticket changes (different /
// brand-new room — see the Join handler). Keyed by id so updates refresh addr.
let known_peers: Arc<std::sync::Mutex<HashMap<EndpointId, EndpointAddr>>> =
Arc::new(std::sync::Mutex::new(HashMap::new()));
// The ticket of the room we last (re)joined, to detect room changes above.
let mut last_join_ticket: Option<String> = None;
let audio_backend = Arc::new(PipeWireBackend::new()); let audio_backend = Arc::new(PipeWireBackend::new());
@@ -554,8 +563,21 @@ async fn run_core_loop(
sharing: None, sharing: None,
}; };
crate::log_msg(&format!("Attempting room_state.join with self_state={:?}", self_state)); // Retain peers across leave so a rejoin can dial them (A8). A
if let Err(e) = room_state.join(&ticket_str, self_state.clone()).await { // different ticket (or a fresh "create", which mints a random
// ticket) shares no peers with the last room, so reset; rejoining
// the SAME ticket keeps the set — that's what gives the room
// creator a dial target on rejoin. Then snapshot it as the extra
// bootstrap set (resolution rides the persistent address book).
if last_join_ticket.as_deref() != Some(ticket_str.as_str()) {
known_peers.lock().unwrap().clear();
}
last_join_ticket = Some(ticket_str.clone());
let extra_bootstrap: Vec<EndpointAddr> =
known_peers.lock().unwrap().values().cloned().collect();
crate::log_msg(&format!("Attempting room_state.join with self_state={:?}, extra_bootstrap={:?}", self_state, extra_bootstrap.iter().map(|a| a.id).collect::<Vec<_>>()));
if let Err(e) = room_state.join(&ticket_str, self_state.clone(), extra_bootstrap).await {
crate::log_msg(&format!("Error room_state.join failed: {:?}", e)); crate::log_msg(&format!("Error room_state.join failed: {:?}", e));
let _ = ui_tx.send(UiEvent::Error(format!("Failed to join room: {}", e))).await; let _ = ui_tx.send(UiEvent::Error(format!("Failed to join room: {}", e))).await;
let _ = router.shutdown().await; let _ = router.shutdown().await;
@@ -900,6 +922,7 @@ async fn run_core_loop(
let seen_connected_events = seen_connected.clone(); let seen_connected_events = seen_connected.clone();
let multitrack_events = multitrack.clone(); let multitrack_events = multitrack.clone();
let is_multitrack_events = is_multitrack.clone(); let is_multitrack_events = is_multitrack.clone();
let known_peers_events = known_peers.clone();
let event_task = tokio::spawn(async move { let event_task = tokio::spawn(async move {
while let Some(event) = room_events.recv().await { while let Some(event) = room_events.recv().await {
match event { match event {
@@ -912,6 +935,8 @@ async fn run_core_loop(
// Hand over the full address so reconnects can dial // Hand over the full address so reconnects can dial
// it directly rather than via the gossip lookup. // it directly rather than via the gossip lookup.
transport_events.connect_peer(state.addr.clone()).await; transport_events.connect_peer(state.addr.clone()).await;
// Retain this peer as a future rejoin bootstrap target (A8).
known_peers_events.lock().unwrap().insert(peer_id, state.addr.clone());
// If a multitrack recording is live, give this peer // If a multitrack recording is live, give this peer
// its own stem track (silence-padded back to t=0). // its own stem track (silence-padded back to t=0).
if is_multitrack_events.load(Ordering::Relaxed) if is_multitrack_events.load(Ordering::Relaxed)
@@ -926,6 +951,10 @@ async fn run_core_loop(
// Graceful leave — evict immediately. // Graceful leave — evict immediately.
cancel_grace_timer(&grace_timers_events, &peer_id); cancel_grace_timer(&grace_timers_events, &peer_id);
seen_connected_events.lock().unwrap().remove(&peer_id); seen_connected_events.lock().unwrap().remove(&peer_id);
// Graceful leave: drop them as a rejoin dial target
// (a transient PeerConnectionLost deliberately does
// NOT, so we can still re-dial a peer who's still up).
known_peers_events.lock().unwrap().remove(&peer_id);
transport_events.disconnect_peer(peer_id).await; transport_events.disconnect_peer(peer_id).await;
jitter_events.lock().await.remove(&peer_id); jitter_events.lock().await.remove(&peer_id);
let _ = ui_tx_events.send(UiEvent::PeerLeft { id: peer_id }).await; let _ = ui_tx_events.send(UiEvent::PeerLeft { id: peer_id }).await;
@@ -939,6 +968,8 @@ async fn run_core_loop(
// re-records the same address. // re-records the same address.
cancel_grace_timer(&grace_timers_events, &peer_id); cancel_grace_timer(&grace_timers_events, &peer_id);
transport_events.connect_peer(state.addr.clone()).await; transport_events.connect_peer(state.addr.clone()).await;
// Refresh the retained rejoin target with the fresh addr (A8).
known_peers_events.lock().unwrap().insert(peer_id, state.addr.clone());
let _ = ui_tx_events.send(UiEvent::PeerUpdated { id: peer_id, state }).await; let _ = ui_tx_events.send(UiEvent::PeerUpdated { id: peer_id, state }).await;
} }
RoomEvent::ChatMessage { name, text, .. } => { RoomEvent::ChatMessage { name, text, .. } => {
+89 -12
View File
@@ -1,5 +1,5 @@
use crate::network::{RoomState, NetError, PeerState, RoomEvent, PeerSpeakTicket}; use crate::network::{RoomState, NetError, PeerState, RoomEvent, PeerSpeakTicket};
use iroh::{Endpoint, EndpointId, SecretKey, Signature}; use iroh::{Endpoint, EndpointAddr, EndpointId, SecretKey, Signature};
use iroh_gossip::net::Gossip; use iroh_gossip::net::Gossip;
use iroh_gossip::proto::TopicId; use iroh_gossip::proto::TopicId;
use tokio::sync::mpsc; use tokio::sync::mpsc;
@@ -102,6 +102,28 @@ fn verify_gossip(
Ok(()) Ok(())
} }
/// Compute the gossip bootstrap peer set for a (re)join: the ticket's host plus
/// every peer we already knew about (`extra`), with self removed and ids
/// de-duplicated. Without the retained `extra` peers, the ROOM CREATOR rejoining
/// their own room would have an empty list (their ticket names only themselves as
/// host) and so never re-enter the swarm — that was bug A8. Pure → unit-testable.
fn compute_bootstrap(
self_id: EndpointId,
ticket_host: EndpointId,
extra: &[EndpointAddr],
) -> Vec<EndpointId> {
let mut out: Vec<EndpointId> = Vec::new();
if ticket_host != self_id {
out.push(ticket_host);
}
for addr in extra {
if addr.id != self_id && !out.contains(&addr.id) {
out.push(addr.id);
}
}
out
}
#[derive(Serialize, Deserialize, Clone, Debug)] #[derive(Serialize, Deserialize, Clone, Debug)]
pub enum GossipMessage { pub enum GossipMessage {
Announce(PeerState), Announce(PeerState),
@@ -157,7 +179,12 @@ impl IrohGossipState {
#[async_trait] #[async_trait]
impl RoomState for IrohGossipState { impl RoomState for IrohGossipState {
async fn join(&self, ticket_str: &str, self_state: PeerState) -> Result<(), NetError> { async fn join(
&self,
ticket_str: &str,
self_state: PeerState,
extra_bootstrap: Vec<EndpointAddr>,
) -> Result<(), NetError> {
crate::log_msg(&format!("RoomState::join: self_id={:?}, self_name={:?}, ticket={}", self_state.addr.id, self_state.name, ticket_str)); crate::log_msg(&format!("RoomState::join: self_id={:?}, self_name={:?}, ticket={}", self_state.addr.id, self_state.name, ticket_str));
let ticket = ticket_str.parse::<PeerSpeakTicket>()?; let ticket = ticket_str.parse::<PeerSpeakTicket>()?;
let topic_id = TopicId::from_bytes(ticket.topic_id); let topic_id = TopicId::from_bytes(ticket.topic_id);
@@ -167,18 +194,20 @@ impl RoomState for IrohGossipState {
// Stop any currently running topic // Stop any currently running topic
let _ = self.leave().await; let _ = self.leave().await;
// Add the host to the address book // Make every dial target resolvable: the ticket host plus any retained
// peers handed in (their addresses may have aged out of a fresh endpoint's
// book even though the persistent lookup usually still holds them).
self.address_lookup.add_endpoint_info(ticket.host_addr.clone()); self.address_lookup.add_endpoint_info(ticket.host_addr.clone());
for addr in &extra_bootstrap {
self.address_lookup.add_endpoint_info(addr.clone());
}
// Join the gossip topic. If we are the host, bootstrap list will be empty // Bootstrap to the ticket host AND every retained peer (minus self). The
// or contain ourselves (which is fine), but let's bootstrap to the ticket host. // retained peers are what let the room creator rejoin a room they left —
let bootstrap_peers = if ticket.host_addr.id == self_state.addr.id { // their own ticket names only themselves as host (bug A8).
crate::log_msg("We are the host. Bootstrap peers list is empty."); let bootstrap_peers =
vec![] compute_bootstrap(self_state.addr.id, ticket.host_addr.id, &extra_bootstrap);
} else { crate::log_msg(&format!("Bootstrap peers for join: {:?}", bootstrap_peers));
crate::log_msg(&format!("We are a client. Bootstrapping to host ID={:?}", ticket.host_addr.id));
vec![ticket.host_addr.id]
};
let gossip_topic = self.gossip.subscribe(topic_id, bootstrap_peers).await let gossip_topic = self.gossip.subscribe(topic_id, bootstrap_peers).await
.map_err(|e| { .map_err(|e| {
@@ -467,6 +496,54 @@ mod tests {
} }
} }
fn fresh_id() -> EndpointId {
SecretKey::generate().public()
}
fn addr_of(id: EndpointId) -> EndpointAddr {
EndpointAddr::from(id)
}
#[test]
fn bootstrap_client_dials_host() {
// A non-host (client) with no retained peers dials just the ticket host.
let me = fresh_id();
let host = fresh_id();
assert_eq!(compute_bootstrap(me, host, &[]), vec![host]);
}
#[test]
fn bootstrap_host_rejoin_dials_retained_peers() {
// The A8 regression: the room creator's ticket names themselves as host,
// so the ticket-host contributes nothing — the retained peer is the only
// (and essential) dial target. An empty result here would reproduce A8.
let me = fresh_id();
let peer = fresh_id();
let bootstrap = compute_bootstrap(me, /* ticket_host = */ me, &[addr_of(peer)]);
assert_eq!(bootstrap, vec![peer]);
}
#[test]
fn bootstrap_host_rejoin_with_no_retained_peers_is_empty() {
// Host rejoin before anyone was ever seen: nothing to dial (unavoidable),
// but it must not contain self.
let me = fresh_id();
assert!(compute_bootstrap(me, me, &[]).is_empty());
}
#[test]
fn bootstrap_excludes_self_and_dedups_host() {
// Self is never dialed (even if it shows up among retained peers), and a
// peer that equals the ticket host isn't listed twice.
let me = fresh_id();
let host = fresh_id();
let other = fresh_id();
let extra = [addr_of(me), addr_of(host), addr_of(other)];
let bootstrap = compute_bootstrap(me, host, &extra);
assert_eq!(bootstrap, vec![host, other]);
assert!(!bootstrap.contains(&me));
}
#[test] #[test]
fn test_gossip_message_leave_round_trip() { fn test_gossip_message_leave_round_trip() {
let original = GossipMessage::Leave; let original = GossipMessage::Leave;
+13 -2
View File
@@ -1,4 +1,4 @@
use iroh::EndpointId; use iroh::{EndpointId, EndpointAddr};
use bytes::Bytes; use bytes::Bytes;
use thiserror::Error; use thiserror::Error;
use tokio::sync::mpsc::Receiver; use tokio::sync::mpsc::Receiver;
@@ -133,7 +133,18 @@ pub trait NetworkTransport: Send + Sync {
#[async_trait] #[async_trait]
pub trait RoomState: Send + Sync { pub trait RoomState: Send + Sync {
/// Joins a room using a gossip ticket string and announces our state. /// Joins a room using a gossip ticket string and announces our state.
async fn join(&self, ticket: &str, self_state: PeerState) -> Result<(), NetError>; ///
/// `extra_bootstrap` are additional peer addresses to dial when entering the
/// gossip swarm, on top of the ticket's host. This is what lets the ROOM
/// CREATOR rejoin a room they left: their own ticket lists only themselves as
/// host, so without retained peers they'd have no dial target and never
/// re-enter the swarm (bug A8). Callers with no prior peers pass `vec![]`.
async fn join(
&self,
ticket: &str,
self_state: PeerState,
extra_bootstrap: Vec<EndpointAddr>,
) -> Result<(), NetError>;
/// Updates our local state (e.g. when user mutes/unmutes) and broadcasts it. /// Updates our local state (e.g. when user mutes/unmutes) and broadcasts it.
async fn update_self_state(&self, self_state: PeerState) -> Result<(), NetError>; async fn update_self_state(&self, self_state: PeerState) -> Result<(), NetError>;