From 754afd56217a8e2b7836a5a0f618c7b8c1a6172d Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 16 May 2026 18:51:54 +0200 Subject: [PATCH] refactor(peer): drop --no-mdns toggle, mDNS is always on The peer runtime previously accepted an `enable_mdns: bool` flag, plumbed through `PeerStartOptions`, `spawn_peer_runtime`, `run_peer`, `Ctx`, and `PeerCtx`. The lanspread-peer-cli harness exposed the toggle as `--no-mdns` so test scenarios could fall back to explicit `connect` commands when mDNS could not be relied on, in particular when multiple peers ran inside `--network host` containers and could not advertise independently. That host-networking workaround no longer exists: the previous commit moves harness containers onto a macvlan network, where each peer is a real LAN device and mDNS just works between them. There is no scenario left in the codebase where disabling mDNS is desirable. Per the project's protocol policy in CLAUDE.md ("there is only one wire version, no compatibility shims, no fallback paths"), an opt-out path with no current caller is exactly the kind of dead code we should not carry. Remove the flag and every plumbing point that exists only to support it: - `PeerStartOptions::enable_mdns` and the custom `Default` impl that set it to `true`; the struct now derives `Default` and just carries `state_dir`. - The `enable_mdns` parameter on `start_peer_with_options`, `spawn_peer_runtime`, `run_peer`, and `Ctx::new`. - The `enable_mdns` fields on `Ctx` and `PeerCtx` and the propagation through `to_peer_ctx`. - The `if ctx.enable_mdns` guard in `spawn_startup_services`; `spawn_peer_discovery_service` is now always spawned. - The `if ctx.enable_mdns { ... } else { ... }` branch in `run_server_component`: the mDNS advertiser and event monitor are now unconditionally started, and the no-mDNS-fallback log line that read "mDNS disabled; direct peer address is ..." is gone. The `direct_connect_addr` helper is kept because the mDNS-on branch still uses it as a fallback when `local_peer_addr` has not yet been populated. - The internal test helpers in `handlers.rs`, `services/local_monitor.rs`, and `services/stream.rs` that passed `true` as the trailing `enable_mdns` arg to `Ctx::new`. - In `lanspread-peer-cli`: the `--no-mdns` arg parsing, the `Args::enable_mdns` field, the `mdns` key on the `cli-started` event payload, and the `--no-mdns` mention in the help text and the crate README. The `Args::name` field is wired to the harness identity but is otherwise untouched. The macvlan network created by `just peer-cli-net` is the runtime prerequisite for this change to be observable across containers; on a single workstation, two harness binaries on `127.0.0.1` discover each other through mDNS on the loopback interface as before. Test Plan: - `just fmt` - `just clippy` - `just test` - `just peer-cli-build` - Two peers on macvlan: `just peer-cli-run alpha` and `just peer-cli-run beta`; check that each emits `peer-discovered` and `peer-connected` events without an explicit `connect` JSONL command. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/lanspread-peer-cli/README.md | 1 - crates/lanspread-peer-cli/src/main.rs | 7 +----- crates/lanspread-peer/src/context.rs | 5 ---- crates/lanspread-peer/src/handlers.rs | 1 - crates/lanspread-peer/src/lib.rs | 21 ++-------------- .../src/services/local_monitor.rs | 1 - crates/lanspread-peer/src/services/server.rs | 25 +++++++------------ crates/lanspread-peer/src/services/stream.rs | 1 - crates/lanspread-peer/src/startup.rs | 6 +---- 9 files changed, 13 insertions(+), 55 deletions(-) diff --git a/crates/lanspread-peer-cli/README.md b/crates/lanspread-peer-cli/README.md index a1f87b9..36761c9 100644 --- a/crates/lanspread-peer-cli/README.md +++ b/crates/lanspread-peer-cli/README.md @@ -17,7 +17,6 @@ Useful flags: - `--games-dir PATH` stores local archives and installs. - `--state-dir PATH` stores the generated peer identity. - `--fixture GAME_ID` seeds a tiny archive that the fixture unpacker can install. -- `--no-mdns` disables mDNS so tests can use explicit `connect` commands. ## Commands diff --git a/crates/lanspread-peer-cli/src/main.rs b/crates/lanspread-peer-cli/src/main.rs index 130fa45..edb00da 100644 --- a/crates/lanspread-peer-cli/src/main.rs +++ b/crates/lanspread-peer-cli/src/main.rs @@ -52,7 +52,6 @@ struct Args { catalog_db: Option, fixtures: Vec, unrar: Option, - enable_mdns: bool, } #[derive(Clone)] @@ -134,7 +133,6 @@ async fn main() -> eyre::Result<()> { catalog, PeerStartOptions { state_dir: Some(args.state_dir.clone()), - enable_mdns: args.enable_mdns, }, )?; let sender = handle.sender(); @@ -155,7 +153,6 @@ async fn main() -> eyre::Result<()> { "games_dir": args.games_dir, "state_dir": args.state_dir, "fixtures": fixture_seeds, - "mdns": args.enable_mdns, }), )); @@ -488,7 +485,6 @@ fn parse_args() -> eyre::Result { catalog_db: default_catalog_db(), fixtures: Vec::new(), unrar: None, - enable_mdns: true, }; while let Some(arg) = args.next() { @@ -503,7 +499,6 @@ fn parse_args() -> eyre::Result { Some("--catalog-db") => parsed.catalog_db = Some(next_path(&mut args, "--catalog-db")?), Some("--fixture") => parsed.fixtures.push(next_string(&mut args, "--fixture")?), Some("--unrar") => parsed.unrar = Some(next_path(&mut args, "--unrar")?), - Some("--no-mdns") => parsed.enable_mdns = false, Some(other) => eyre::bail!("unknown argument: {other}"), None => eyre::bail!("argument is not valid UTF-8: {arg:?}"), } @@ -535,7 +530,7 @@ fn next_path(args: &mut impl Iterator, flag: &str) -> eyre::Res fn print_help() { eprintln!( "usage: lanspread-peer-cli [--name NAME] [--games-dir PATH] [--state-dir PATH] \\ - [--catalog-db PATH] [--fixture GAME_ID] [--unrar PATH] [--no-mdns]\n\ + [--catalog-db PATH] [--fixture GAME_ID] [--unrar PATH]\n\ Reads JSONL commands on stdin and writes result/event/error JSONL on stdout." ); } diff --git a/crates/lanspread-peer/src/context.rs b/crates/lanspread-peer/src/context.rs index da4f92f..63eb582 100644 --- a/crates/lanspread-peer/src/context.rs +++ b/crates/lanspread-peer/src/context.rs @@ -39,7 +39,6 @@ pub struct Ctx { pub unpacker: Arc, pub catalog: Arc>>, pub peer_id: Arc, - pub enable_mdns: bool, pub shutdown: CancellationToken, pub task_tracker: TaskTracker, } @@ -55,7 +54,6 @@ pub struct PeerCtx { pub peer_game_db: Arc>, pub catalog: Arc>>, pub peer_id: Arc, - pub enable_mdns: bool, pub tx_notify_ui: tokio::sync::mpsc::UnboundedSender, pub shutdown: CancellationToken, pub task_tracker: TaskTracker, @@ -83,7 +81,6 @@ impl Ctx { shutdown: CancellationToken, task_tracker: TaskTracker, catalog: Arc>>, - enable_mdns: bool, ) -> Self { Self { game_dir: Arc::new(RwLock::new(game_dir)), @@ -96,7 +93,6 @@ impl Ctx { unpacker, catalog, peer_id: Arc::new(peer_id), - enable_mdns, shutdown, task_tracker, } @@ -116,7 +112,6 @@ impl Ctx { peer_game_db: self.peer_game_db.clone(), catalog: self.catalog.clone(), peer_id: self.peer_id.clone(), - enable_mdns: self.enable_mdns, tx_notify_ui, shutdown: self.shutdown.clone(), task_tracker: self.task_tracker.clone(), diff --git a/crates/lanspread-peer/src/handlers.rs b/crates/lanspread-peer/src/handlers.rs index 15cb145..a758d7c 100644 --- a/crates/lanspread-peer/src/handlers.rs +++ b/crates/lanspread-peer/src/handlers.rs @@ -897,7 +897,6 @@ mod tests { CancellationToken::new(), TaskTracker::new(), Arc::new(RwLock::new(HashSet::from(["game".to_string()]))), - true, ) } diff --git a/crates/lanspread-peer/src/lib.rs b/crates/lanspread-peer/src/lib.rs index ab97312..39dc925 100644 --- a/crates/lanspread-peer/src/lib.rs +++ b/crates/lanspread-peer/src/lib.rs @@ -213,21 +213,10 @@ pub enum PeerCommand { } /// Optional startup settings for non-GUI callers and tests. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct PeerStartOptions { /// Directory used for peer identity and other state. pub state_dir: Option, - /// Whether to advertise and discover peers via mDNS. - pub enable_mdns: bool, -} - -impl Default for PeerStartOptions { - fn default() -> Self { - Self { - state_dir: None, - enable_mdns: true, - } - } } // ============================================================================= @@ -274,10 +263,7 @@ pub fn start_peer_with_options( catalog: Arc>>, options: PeerStartOptions, ) -> eyre::Result { - let PeerStartOptions { - state_dir, - enable_mdns, - } = options; + let PeerStartOptions { state_dir } = options; let game_dir = game_dir.into(); log::info!( "Starting peer system with game directory: {}", @@ -296,7 +282,6 @@ pub fn start_peer_with_options( game_dir, unpacker, catalog, - enable_mdns, )) } @@ -312,7 +297,6 @@ async fn run_peer( shutdown: CancellationToken, task_tracker: TaskTracker, catalog: Arc>>, - enable_mdns: bool, ) -> eyre::Result<()> { let ctx = Ctx::new( peer_game_db, @@ -322,7 +306,6 @@ async fn run_peer( shutdown, task_tracker, catalog, - enable_mdns, ); if let Err(err) = load_local_library(&ctx, &tx_notify_ui).await { log::error!("Failed to load initial local game database: {err}"); diff --git a/crates/lanspread-peer/src/services/local_monitor.rs b/crates/lanspread-peer/src/services/local_monitor.rs index f3be000..f1ef263 100644 --- a/crates/lanspread-peer/src/services/local_monitor.rs +++ b/crates/lanspread-peer/src/services/local_monitor.rs @@ -375,7 +375,6 @@ mod tests { CancellationToken::new(), TaskTracker::new(), Arc::new(RwLock::new(catalog)), - true, ) } diff --git a/crates/lanspread-peer/src/services/server.rs b/crates/lanspread-peer/src/services/server.rs index 3eb64af..4b20795 100644 --- a/crates/lanspread-peer/src/services/server.rs +++ b/crates/lanspread-peer/src/services/server.rs @@ -35,22 +35,15 @@ pub async fn run_server_component( let server_addr = server.local_addr()?; log::info!("Peer server listening on {server_addr}"); - let (ready_addr, _mdns_advertiser) = if ctx.enable_mdns { - let mdns_advertiser = start_mdns_advertiser(&ctx, server_addr).await?; - let mdns_monitor = mdns_advertiser.monitor.clone(); - let mdns_shutdown = ctx.shutdown.clone(); - ctx.task_tracker.spawn(async move { - monitor_mdns_events(mdns_monitor, mdns_shutdown).await; - }); - let ready_addr = - (*ctx.local_peer_addr.read().await).unwrap_or_else(|| direct_connect_addr(server_addr)); - (ready_addr, Some(mdns_advertiser)) - } else { - let addr = direct_connect_addr(server_addr); - *ctx.local_peer_addr.write().await = Some(addr); - log::info!("mDNS disabled; direct peer address is {addr}"); - (addr, None) - }; + let mdns_advertiser = start_mdns_advertiser(&ctx, server_addr).await?; + let mdns_monitor = mdns_advertiser.monitor.clone(); + let mdns_shutdown = ctx.shutdown.clone(); + ctx.task_tracker.spawn(async move { + monitor_mdns_events(mdns_monitor, mdns_shutdown).await; + }); + let ready_addr = + (*ctx.local_peer_addr.read().await).unwrap_or_else(|| direct_connect_addr(server_addr)); + let _mdns_advertiser = mdns_advertiser; events::send( &tx_notify_ui, diff --git a/crates/lanspread-peer/src/services/stream.rs b/crates/lanspread-peer/src/services/stream.rs index 5502026..52527e7 100644 --- a/crates/lanspread-peer/src/services/stream.rs +++ b/crates/lanspread-peer/src/services/stream.rs @@ -421,7 +421,6 @@ mod tests { CancellationToken::new(), TaskTracker::new(), Arc::new(RwLock::new(catalog)), - true, ) .to_peer_ctx(tx_notify_ui) } diff --git a/crates/lanspread-peer/src/startup.rs b/crates/lanspread-peer/src/startup.rs index d44b991..a6c95f9 100644 --- a/crates/lanspread-peer/src/startup.rs +++ b/crates/lanspread-peer/src/startup.rs @@ -84,7 +84,6 @@ pub(crate) fn spawn_peer_runtime( game_dir: PathBuf, unpacker: Arc, catalog: Arc>>, - enable_mdns: bool, ) -> PeerRuntimeHandle { let shutdown = CancellationToken::new(); let task_tracker = TaskTracker::new(); @@ -103,7 +102,6 @@ pub(crate) fn spawn_peer_runtime( runtime_shutdown.clone(), runtime_tracker.clone(), catalog, - enable_mdns, ) .await { @@ -127,9 +125,7 @@ pub(crate) fn spawn_peer_runtime( pub(crate) fn spawn_startup_services(ctx: &Ctx, tx_notify_ui: &UnboundedSender) { spawn_quic_server(ctx, tx_notify_ui); - if ctx.enable_mdns { - spawn_peer_discovery_service(ctx, tx_notify_ui); - } + spawn_peer_discovery_service(ctx, tx_notify_ui); spawn_peer_liveness_service(ctx, tx_notify_ui); spawn_local_library_monitor(ctx, tx_notify_ui); }