From 48bb1769b4f6f6be0eeda5484f54a9b12e271049 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 20:26:59 +0200 Subject: [PATCH] =?UTF-8?q?feat(cli):=20punktfunk=20discover=20=E2=80=94?= =?UTF-8?q?=20browse=20the=20LAN,=20annotated=20against=20what=20you've=20?= =?UTF-8?q?saved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI could do everything with a host except FIND one, so every headless consumer grew its own mDNS: the Decky plugin parses ~120 lines of avahi TXT escaping in Python, which drifts from the host's advert every time a key is added and makes the plugin depend on Avahi being the resolver. `discovery::discover_for(timeout)` is the bounded collector beside the streaming `browse()` the UI uses — same service type, same TXT keys, folded to one row per host. A refreshed advert wins (it carries the newer address), a removal drops the row, and dropping the receiver on the way out stops the worker so a one-shot call can't leak a browse per invocation. The verb annotates each hit against the saved-hosts store rather than handing back two lists to join: `saved`/`paired` are answered by fingerprint first and address second — the same rule every other surface uses. That is what stops a host that moved DHCP lease from reading as new, and stops a different box that inherited the old address from reading as paired. punktfunk discover [--json] [--timeout SECS] Default 3 s, capped at 30 — this is called from a Quick Access panel, and a typo'd `--timeout 3000` would hang that panel with no way to cancel. An empty LAN exits 0: a caller branching on the code is asking whether the browse ran, and it did. --- clients/cli/src/main.rs | 117 ++++++++++++++++- clients/cli/tests/cli_smoke.rs | 15 +++ crates/pf-client-core/src/discovery.rs | 166 +++++++++++++++++++++++++ 3 files changed, 297 insertions(+), 1 deletion(-) diff --git a/clients/cli/src/main.rs b/clients/cli/src/main.rs index b359f7b1..4ed81607 100644 --- a/clients/cli/src/main.rs +++ b/clients/cli/src/main.rs @@ -44,6 +44,7 @@ mod cli { const USAGE: &str = "\ punktfunk — the Punktfunk client, headless + punktfunk discover [--json] [--timeout SECS] punktfunk pair [--pin N] [--name LABEL] punktfunk hosts list [--probe] [--json] punktfunk hosts add [--name LABEL] [--fp HEX] @@ -68,6 +69,24 @@ punktfunk:// link takes. Exit codes: 0 ok, 2 connect, 3 trust, 4 renderer, 5 not /// (what goes to stdout vs stderr, and which exit codes mean what). fn verb_help(verb: &str) -> Option<&'static str> { Some(match verb { + "discover" => { + "\ +punktfunk discover [--json] [--timeout SECS] — browse the LAN for hosts + +Listens for Punktfunk hosts advertising over mDNS and prints what answered: +name TAB addr:port TAB saved|new TAB paired|unpaired. `saved` means this +device already has a record for it, matched by fingerprint first and address +second — the same rule every other surface joins the two lists by. + + --timeout SECS how long to browse (default 3, capped at 30) — a bounded + call, so a panel can wait for it + --json {\"hosts\":[{\"name\",\"addr\",\"port\",\"fp\",\"pair\",\"id\",\"mgmt\", + \"os\",\"saved\",\"paired\"}]} + +Nothing answering is an answer, not a failure: an empty list exits 0. A host +mDNS never sees (Tailscale, another subnet) will not appear here — save it by +address with `punktfunk hosts add` and it shows in `hosts list --probe`." + } "pair" => { "\ punktfunk pair — enrol this device with a host (PIN ceremony) @@ -222,7 +241,7 @@ from the config directory for a true factory reset." fn flag_takes_value(flag: &str) -> bool { matches!( flag, - "--pin" | "--name" | "--fp" | "--game" | "--profile" | "--port" + "--pin" | "--name" | "--fp" | "--game" | "--profile" | "--port" | "--timeout" ) } @@ -269,6 +288,7 @@ from the config directory for a true factory reset." return OK; } match verb.as_str() { + "discover" => discover(&rest), "pair" => pair(&rest), "hosts" => hosts(&rest), "wake" => wake(&rest), @@ -306,6 +326,100 @@ from the config directory for a true factory reset." } } + /// How long `discover` browses when nobody says, and the ceiling on what they can ask for. + /// The cap is not politeness: this verb is called from a Quick Access panel, and a typo'd + /// `--timeout 3000` would hang that panel with no way to cancel it. + const DISCOVER_DEFAULT_SECS: f64 = 3.0; + const DISCOVER_MAX_SECS: f64 = 30.0; + + /// `discover [--json] [--timeout SECS]` — browse the LAN over mDNS and print what answered, + /// annotated against the saved-hosts store. + /// + /// The annotation is the point: a caller wants "can I stream this", which is a question + /// about BOTH lists, and joining them itself is how two surfaces end up disagreeing about + /// the same host. So the match rule lives here, once, and is the same one every other + /// surface uses — fingerprint first (survives a DHCP move), address second. + fn discover(args: &[String]) -> u8 { + let secs = value(args, "--timeout") + .and_then(|v| v.parse::().ok()) + .filter(|s| *s > 0.0) + .unwrap_or(DISCOVER_DEFAULT_SECS) + .min(DISCOVER_MAX_SECS); + let found = pf_client_core::discovery::discover_for(Duration::from_secs_f64(secs)); + let known = KnownHosts::load(); + let rows: Vec<( + &pf_client_core::discovery::DiscoveredHost, + Option<&KnownHost>, + )> = found.iter().map(|d| (d, match_saved(&known, d))).collect(); + if has(args, "--json") { + let hosts: Vec = rows + .iter() + .map(|(d, saved)| { + serde_json::json!({ + "name": d.name, + "addr": d.addr, + "port": d.port, + "fp": d.fp_hex, + "pair": d.pair, + "id": d.advertised_id(), + // 0 = not advertised, which is what a consumer's own "no mgmt port" + // already means — an older host simply omits the TXT. + "mgmt": d.mgmt_port.unwrap_or(0), + "os": d.os, + "saved": saved.is_some(), + "paired": saved.is_some_and(|h| h.paired), + }) + }) + .collect(); + println!("{}", serde_json::json!({ "hosts": hosts })); + } else { + for (d, saved) in &rows { + println!( + "{}\t{}:{}\t{}\t{}", + d.name, + d.addr, + d.port, + if saved.is_some() { "saved" } else { "new" }, + if saved.is_some_and(|h| h.paired) { + "paired" + } else { + "unpaired" + }, + ); + } + } + // An empty LAN is an answer, not a failure — a caller branching on the exit code is + // asking "did the browse run", and it did. + OK + } + + /// The saved record an advert belongs to, if any: fingerprint first, address second. + /// + /// Fingerprint FIRST is deliberate and load-bearing — a host that moved to a new DHCP lease + /// still matches its record, and a *different* host that inherited the old address does not + /// inherit its pairing. This is the rule the plugin's `mergeHosts` and the shells' hosts + /// pages already use; keeping one copy is what stops two surfaces disagreeing about whether + /// the box in front of you is paired. + fn match_saved<'a>( + known: &'a KnownHosts, + advert: &pf_client_core::discovery::DiscoveredHost, + ) -> Option<&'a KnownHost> { + known + .hosts + .iter() + .find(|h| { + !h.fp_hex.is_empty() + && !advert.fp_hex.is_empty() + && h.fp_hex.eq_ignore_ascii_case(&advert.fp_hex) + }) + .or_else(|| { + known + .hosts + .iter() + .find(|h| h.addr == advert.addr && h.port == advert.port) + }) + } + /// `pair [--pin N]` — the SPAKE2 ceremony. Without `--pin` it prompts, which /// is the interactive shape; with one it is scriptable. Refuses rather than prompting when /// stdin isn't a terminal and no PIN was given: a pairing that silently blocks a CI job @@ -967,6 +1081,7 @@ from the config directory for a true factory reset." #[test] fn every_usage_verb_has_help() { for verb in [ + "discover", "pair", "hosts", "wake", diff --git a/clients/cli/tests/cli_smoke.rs b/clients/cli/tests/cli_smoke.rs index 81a80347..90dd1db2 100644 --- a/clients/cli/tests/cli_smoke.rs +++ b/clients/cli/tests/cli_smoke.rs @@ -67,3 +67,18 @@ fn unknown_verbs_refuse_with_the_not_found_code() { let out = punktfunk(&["help", "frobnicate"]); assert_eq!(out.status.code(), Some(5), "unknown help topic exits 5"); } + +/// `discover` documents itself. Help only — the verb itself browses the LAN, which no runner +/// may be asked to do. +/// +/// The Decky panel detects a too-old client by exactly the signature the test above pins +/// (exit 5 + `unknown command`), so this is the other half of that contract: on a client new +/// enough, `discover` is a verb with help rather than an unknown word. +#[test] +fn discover_documents_itself() { + let out = punktfunk(&["help", "discover"]); + assert!(out.status.success(), "discover has its own help topic"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("--timeout"), "discover documents --timeout"); + assert!(stdout.contains("--json"), "discover documents --json"); +} diff --git a/crates/pf-client-core/src/discovery.rs b/crates/pf-client-core/src/discovery.rs index fdeb8fbe..d318b5c5 100644 --- a/crates/pf-client-core/src/discovery.rs +++ b/crates/pf-client-core/src/discovery.rs @@ -4,6 +4,8 @@ //! cards and flip a saved host's online pip when its advert disappears. use mdns_sd::{ServiceDaemon, ServiceEvent}; +use std::collections::BTreeMap; +use std::time::{Duration, Instant}; #[derive(Clone, Debug)] pub struct DiscoveredHost { @@ -31,6 +33,19 @@ pub struct DiscoveredHost { pub os: String, } +impl DiscoveredHost { + /// The host's advertised stable id (mDNS TXT `id`), or `""` when it doesn't advertise one. + /// [`DiscoveredHost::key`] falls back to the mDNS fullname in that case, so the two being + /// equal is exactly the "no id" signal — read it through here rather than re-deriving it. + pub fn advertised_id(&self) -> &str { + if self.key == self.fullname { + "" + } else { + &self.key + } + } +} + /// One discovery update for the UI's advert map. pub enum DiscoveryEvent { /// A host advert appeared or refreshed (new address, pairing flipped, …). @@ -117,3 +132,154 @@ pub fn browse() -> async_channel::Receiver { .expect("spawn mdns thread"); rx } + +/// The advert map one browse window folded down to. Kept separate from [`discover_for`] so the +/// fold — which is where dedupe and removal actually live — is testable without a network. +type Adverts = BTreeMap; + +/// Apply one event to the map. A refreshed advert WINS over the one already there (it carries +/// the newer address — a host that changed DHCP lease re-announces), and a removal drops +/// whichever entry that mDNS fullname produced, whatever it was keyed under. +fn fold(adverts: &mut Adverts, event: DiscoveryEvent) { + match event { + DiscoveryEvent::Resolved(host) => { + adverts.insert(host.key.clone(), host); + } + DiscoveryEvent::Removed { fullname } => { + adverts.retain(|_, h| h.fullname != fullname); + } + } +} + +/// Browse for `timeout`, then return what answered — deduped by `key`, address-sorted. +/// +/// Blocking; intended for one-shot consumers (the CLI's `discover` verb, a plugin backend that +/// wants one bounded call rather than a stream). The streaming [`browse`] stays the UI's door: +/// a live hosts page wants adverts as they land, not a snapshot taken `timeout` after it opened. +pub fn discover_for(timeout: Duration) -> Vec { + let rx = browse(); + let deadline = Instant::now() + timeout; + let mut adverts = Adverts::new(); + while Instant::now() < deadline { + while let Ok(event) = rx.try_recv() { + fold(&mut adverts, event); + } + // A short tick rather than a blocking recv with a deadline: `async_channel`'s blocking + // receive has no timeout, and the whole point of this call is that it is bounded. + std::thread::sleep(Duration::from_millis(50).min(timeout)); + } + while let Ok(event) = rx.try_recv() { + fold(&mut adverts, event); + } + // Dropping the receiver is what stops the worker: its next send fails and the thread exits, + // shutting the daemon down. Without this a one-shot consumer would leak a browse per call. + drop(rx); + sorted(adverts) +} + +/// The map as the list a caller gets: sorted by address, then port. IPv4 is compared +/// NUMERICALLY (a lexical sort puts `.10` before `.9`, which reads as scrambled in a host list). +fn sorted(adverts: Adverts) -> Vec { + let mut hosts: Vec = adverts.into_values().collect(); + hosts.sort_by_key(|h| { + ( + h.addr.parse::().ok().map(u32::from), + h.addr.clone(), + h.port, + ) + }); + hosts +} + +#[cfg(test)] +mod tests { + use super::*; + + fn host(key: &str, fullname: &str, addr: &str) -> DiscoveredHost { + DiscoveredHost { + key: key.into(), + fullname: fullname.into(), + name: fullname.split('.').next().unwrap_or("?").into(), + addr: addr.into(), + port: 9777, + fp_hex: "aa".into(), + pair: "required".into(), + mgmt_port: Some(47990), + mac: vec![], + os: String::new(), + } + } + + /// Two adverts for the same host collapse to one row, and the LATER one wins — that is how + /// a host that moved to a new address stops being listed at the stale one. + #[test] + fn refreshed_advert_supersedes_the_earlier_one() { + let mut adverts = Adverts::new(); + fold( + &mut adverts, + DiscoveryEvent::Resolved(host("id-1", "desk._punktfunk._udp.local.", "192.168.1.9")), + ); + fold( + &mut adverts, + DiscoveryEvent::Resolved(host("id-1", "desk._punktfunk._udp.local.", "192.168.1.20")), + ); + let out = sorted(adverts); + assert_eq!(out.len(), 1, "same key must not render twice"); + assert_eq!(out[0].addr, "192.168.1.20", "the newer address wins"); + } + + /// A host that goes away during the browse window is not in the answer. + #[test] + fn removal_drops_the_advert_it_names() { + let mut adverts = Adverts::new(); + fold( + &mut adverts, + DiscoveryEvent::Resolved(host("id-1", "desk._punktfunk._udp.local.", "192.168.1.9")), + ); + fold( + &mut adverts, + DiscoveryEvent::Resolved(host("id-2", "tv._punktfunk._udp.local.", "192.168.1.10")), + ); + fold( + &mut adverts, + DiscoveryEvent::Removed { + fullname: "desk._punktfunk._udp.local.".into(), + }, + ); + let out = sorted(adverts); + assert_eq!(out.len(), 1); + assert_eq!(out[0].key, "id-2"); + } + + /// A host with no `id` TXT is keyed by its fullname — and must not then report that + /// fullname as an id, which would send a caller launching against a nonexistent reference. + #[test] + fn advertised_id_is_empty_without_the_txt() { + let named = host("id-1", "desk._punktfunk._udp.local.", "10.0.0.1"); + assert_eq!(named.advertised_id(), "id-1"); + let anonymous = host( + "desk._punktfunk._udp.local.", + "desk._punktfunk._udp.local.", + "10.0.0.1", + ); + assert_eq!(anonymous.advertised_id(), ""); + } + + /// Addresses sort the way a person reads them, not the way strings compare. + #[test] + fn addresses_sort_numerically() { + let mut adverts = Adverts::new(); + for (i, addr) in ["192.168.1.20", "192.168.1.9", "192.168.1.100"] + .into_iter() + .enumerate() + { + fold( + &mut adverts, + DiscoveryEvent::Resolved(host(&format!("id-{i}"), &format!("h{i}."), addr)), + ); + } + let out = sorted(adverts); + let addrs: Vec<&str> = out.iter().map(|h| h.addr.as_str()).collect(); + assert_eq!(addrs, ["192.168.1.9", "192.168.1.20", "192.168.1.100"]); + } +}