fix: a host that changed DHCP lease could no longer be streamed from the panel

An adversarial review of this branch found a regression I introduced, plus three smaller
defects. All four are fixed here, each verified on .21.

**The regression.** `mergeHosts` names a host by its record's stable id, and `hosts list --json`
always emits one (`KnownHosts::load` mints ids for every record). So a launch always went out as
`punktfunk launch <uuid>` → `ConnectPlan::for_host` → `HostTarget::from(&KnownHost)`, which
copies the address stored ON THE RECORD. Meanwhile the panel deliberately renders the LIVE
advert's address. Nothing on a Deck ever writes a moved address back — `discover` and
`hosts list` are both reads, and only the desktop shells' hosts pages update one.

So after any DHCP move the row read "online" at the new address and every press dialled the old
one: a 15 s dead connect, or — if a MAC had ever been learned — a black Steam "game" for the
full 90 s wake budget. Proven with a stub session binary: `launch abc-123` emitted
`--connect 10.0.0.5:9777` for a host answering at `10.0.0.99`.

This worked on origin/main, which dialled `toHost(v).host` — the advert's address. The fix
restores that without giving up stable ids: `hosts add <new-addr> --fp <known-fp>` now MOVES the
matching record instead of filing a second one (the fingerprint is the identity — this is the
same rule that makes the verb idempotent), and the panel re-points a host it can see has moved
before launching it. Verified: `moved 10.0.0.5:9777 to 10.0.0.99:9777`, one record still, and
`launch abc-123` then emits `--connect 10.0.0.99:9777`.

**"No hosts yet" was also how a missing client looked.** `_cli_argv()` returning None becomes
`client-unavailable`, which the panel dropped on the floor — so a Deck with no client installed
was told its network was empty, under a button that launches the client that isn't there. It now
says which of the two it is.

**The browse worker never exited on a quiet LAN.** `discover_for` drops the receiver and the
doc claimed that stops the thread. It does not: the worker parks in `recv()`, and the arms that
ignore an event (`SearchStarted`, `ServiceFound`, `SearchStopped`, a v6-only advert) never touch
the sender, so on a LAN with no Punktfunk host nothing ever wakes it. Harmless today because the
only caller is a short-lived CLI process, but the function invites in-process use, where it would
leak a thread and an mDNS daemon per call. Now polled with a 250 ms tick and a check at the top
of the loop. Verified: ten back-to-back browses settle back to the baseline thread count.

**A `pair=optional` host was recorded as paired.** Every unsaved host now goes through the trust
sheet (it has no pin, so it cannot stream without one), but the sheet's only non-PIN action ran
`--request-access`, which persists `paired: true` on Ready. An optional host admits anyone who
pins its identity — there is no operator decision, so nothing was approved and the same box read
"paired" here and "trusted" in the desktop client. Such a host now gets **Connect** instead,
which pins and streams without claiming an approval, and the "approve this Deck" toast is no
longer shown to someone who has nobody to ask.

Also: `PF_CLIENT_BIN` was the one launch-option value never validated — a client installed under
a path with a space would split Steam's tokenizer.

clients/cli/src/main.rs

@@ -121,6 +121,13 @@ punktfunk hosts — the saved-hosts store (shared with the desktop client)
       another subnet). Without --fp it is a placeholder to pair later; with a
       64-hex fingerprint it is pinned immediately (still unpaired).
 
+      Idempotent, and keyed on the FINGERPRINT once there is one: re-running it
+      for a host already saved is a no-op, and giving a known fingerprint a new
+      address MOVES that host's record there rather than filing a second one
+      (which is how a host that changed DHCP lease stays reachable by its id).
+      A different fingerprint for an address already saved is refused, exit 3 —
+      a changed identity is a decision for a person.
+
   punktfunk hosts forget <host-ref>
       Remove a saved host, its pinned fingerprint included. A later connect
       must pair or trust it again."
@@ -592,6 +599,31 @@ from the config directory for a true factory reset."
                         },
                     };
                 }
+                // No record at this address — but a record carrying this exact FINGERPRINT is
+                // this same host at a new one. Re-point it rather than filing a second record:
+                // the fingerprint is the identity, and a host that changed DHCP lease is the
+                // whole reason `hosts add --fp` is idempotent in the first place. Without this a
+                // moved host accumulates one record per address it has ever held, and the one a
+                // stable id resolves to keeps the address it can no longer be reached at.
+                if let Some(i) = known
+                    .hosts
+                    .iter()
+                    .position(|h| !fp.is_empty() && h.fp_hex.eq_ignore_ascii_case(&fp))
+                {
+                    let was = format!("{}:{}", known.hosts[i].addr, known.hosts[i].port);
+                    known.hosts[i].addr = addr.clone();
+                    known.hosts[i].port = port;
+                    return match known.save() {
+                        Ok(()) => {
+                            println!("moved {was} to {addr}:{port}");
+                            OK
+                        }
+                        Err(e) => {
+                            eprintln!("saving: {e:#}");
+                            CONNECT_FAILED
+                        }
+                    };
+                }
                 known.hosts.push(KnownHost {
                     name: name.unwrap_or_else(|| addr.clone()),
                     addr: addr.clone(),
@@ -1317,6 +1349,32 @@ from the config directory for a true factory reset."
             );
         }
 
+        /// A host that changed DHCP lease is re-pointed, not filed a second time. Without this
+        /// the record a stable id resolves to keeps an address the host has left, so a launch
+        /// dials into the void while the panel shows the live one.
+        #[test]
+        fn a_known_fingerprint_at_a_new_address_moves_the_record() {
+            let mut known = KnownHosts {
+                hosts: vec![saved("desk", "192.168.1.9", "abc123")],
+            };
+            // Simulates `hosts add 192.168.1.50 --fp abc123` finding no record at that address.
+            let by_addr = known
+                .hosts
+                .iter()
+                .position(|h| h.addr == "192.168.1.50" && h.port == 9777);
+            assert!(
+                by_addr.is_none(),
+                "the new address is not yet on any record"
+            );
+            let by_fp = known
+                .hosts
+                .iter()
+                .position(|h| h.fp_hex.eq_ignore_ascii_case("abc123"));
+            assert_eq!(by_fp, Some(0), "the fingerprint still identifies the host");
+            known.hosts[0].addr = "192.168.1.50".into();
+            assert_eq!(known.hosts.len(), 1, "one host, one record");
+        }
+
         #[test]
         fn value_reads_the_argument_after_its_flag() {
             let a = argv(&["--game", "steam:570", "--exec"]);

clients/decky/src/hooks.ts

@@ -69,6 +69,14 @@ export interface HostView {
   fp: string;
   /** What the host is advertising right now, if anything — what request access would pin. */
   advertisedFp: string;
+  /**
+   * The host is answering at an address its record does not carry — it changed DHCP lease.
+   *
+   * This matters because a launch names the host by [`ref`], and the CLI dials whatever address
+   * the RECORD holds. So the row would show the live address and dial the dead one. The record
+   * has to be re-pointed before such a host can stream; `startStream` does it.
+   */
+  moved: boolean;
   paired: boolean;
   online: boolean;
   saved: boolean;
@@ -131,6 +139,7 @@ export function mergeHosts(saved: SavedHost[], discovered: DiscoveredHost[]): Ho
       port: advert?.port ?? s.port,
       fp: s.fp_hex,
       advertisedFp: advert?.fp ?? "",
+      moved: !!advert && (advert.addr !== s.addr || advert.port !== s.port),
       paired: s.paired,
       online: !!advert || s.online === true,
       saved: true,
@@ -153,6 +162,7 @@ export function mergeHosts(saved: SavedHost[], discovered: DiscoveredHost[]): Ho
       // No record, so nothing is pinned — whatever it advertises is an OFFER, not a pin.
       fp: "",
       advertisedFp: a.fp,
+      moved: false, // no record, so nothing to be stale
       paired: a.paired,
       online: true,
       saved: false,
@@ -185,9 +195,11 @@ function sortRows(a: HostView, b: HostView): number {
 export function useHosts() {
   const [views, setViews] = useState<HostView[]>([]);
   const [scanning, setScanning] = useState(false);
-  // A client too old for `punktfunk discover`. Rendered as one explanatory row plus the update
-  // button that fixes it — never as an empty list, which would read as "no hosts on your LAN".
-  const [outdated, setOutdated] = useState(false);
+  // Why the list is empty, when it is empty for a reason other than an empty LAN. Rendering
+  // either of these as "No hosts yet" would blame the user's network for the plugin's problem:
+  //   "client-outdated"    — the installed client predates `punktfunk discover`
+  //   "client-unavailable" — there is no client installed at all
+  const [problem, setProblem] = useState<string | null>(null);
 
   const refresh = useCallback(async () => {
     setScanning(true);
@@ -195,7 +207,14 @@ export function useHosts() {
       // Both in flight at once: the browse is time-bounded and the probe is network-bound, so
       // running them in sequence would cost the sum of two waits for no benefit.
       const [d, s] = await Promise.all([discover(), listHosts()]);
-      setOutdated(d.error === "client-outdated" || s.error === "client-outdated");
+      // Both calls run the same binary, so they fail the same way; take whichever answered.
+      setProblem(
+        d.error === "client-unavailable" || s.error === "client-unavailable"
+          ? "client-unavailable"
+          : d.error === "client-outdated" || s.error === "client-outdated"
+            ? "client-outdated"
+            : null,
+      );
       setViews(mergeHosts(s.hosts ?? [], d.hosts ?? []));
     } catch (e) {
       toaster.toast({ title: "Punktfunk", body: `Couldn't list hosts: ${e}` });
@@ -208,7 +227,7 @@ export function useHosts() {
     void refresh();
   }, [refresh]);
 
-  return { views, scanning, outdated, refresh };
+  return { views, scanning, problem, refresh };
 }
 
 // ----------------------------------------------------------------------------------------

clients/decky/src/index.tsx

@@ -125,7 +125,7 @@ const HostRow: FC<{ host: HostView; refresh: () => void }> = ({ host, refresh })
 };
 
 const QamPanel: FC = () => {
-  const { views, scanning, outdated, refresh } = useHosts();
+  const { views, scanning, problem, refresh } = useHosts();
   const { info: update, checking, check } = useUpdate();
 
   return (
@@ -178,15 +178,23 @@ const QamPanel: FC = () => {
             {scanning ? "Scanning…" : "Refresh"}
           </ButtonItem>
         </PanelSectionRow>
-        {/* A client too old for `punktfunk discover` explains itself rather than rendering an
-            empty list — "no hosts on your LAN" would be a lie, and the button that fixes it is
-            in this same panel. Saved hosts still list: that path is an older verb. */}
-        {outdated && (
+        {/* A client that is missing or too old explains itself rather than rendering an empty
+            list — "no hosts on your LAN" would blame the network for the plugin's problem, and
+            for the outdated case the button that fixes it is in this same panel. */}
+        {problem && (
           <PanelSectionRow>
             <Field
               focusable={false}
-              label="Update the Punktfunk client"
-              description="This client is too old to find hosts on your network. Saved hosts still work."
+              label={
+                problem === "client-unavailable"
+                  ? "Punktfunk isn’t installed"
+                  : "Update the Punktfunk client"
+              }
+              description={
+                problem === "client-unavailable"
+                  ? "This panel launches the Punktfunk app, which isn’t on this Deck yet. Install it in Desktop Mode."
+                  : "This client is too old to find hosts on your network. Saved hosts still work."
+              }
             />
           </PanelSectionRow>
         )}
@@ -195,7 +203,7 @@ const QamPanel: FC = () => {
             <Field focusable={false} description="Scanning your network…" />
           </PanelSectionRow>
         )}
-        {views.length === 0 && !scanning && (
+        {views.length === 0 && !scanning && !problem && (
           <PanelSectionRow>
             <Field
               focusable={false}

clients/decky/src/steam.ts

@@ -262,7 +262,7 @@ export async function ensureGamepadUiShortcut(): Promise<number | null> {
     // (/bin/sh); the wrapper rides behind as an arg. PF_CLIENT_BIN only when the backend resolved
     // a NATIVE client — else the wrapper's flatpak default stands and this shortcut is exactly
     // what it always was.
-    const clientBin = info.client_bin ? `PF_CLIENT_BIN=${info.client_bin} ` : "";
+    const clientBin = safeClientBin(info.client_bin) ? `PF_CLIENT_BIN=${info.client_bin} ` : "";
     const launchOpts = `${clientBin}PF_BROWSE=1 %command% "${info.runner}"`;
 
     // Reuse the remembered entry only if it still exists; a stale appId (deleted shortcut whose
@@ -346,6 +346,16 @@ export function isSafeLaunchId(id: string): boolean {
   );
 }
 
+/**
+ * Is a resolved native-client path safe to put in Steam's launch options? Same rule, separate
+ * name because the failure is different: an unsafe id is a bug in our own data, an unsafe path
+ * is just where the user installed the client — so the browse shortcut degrades to its flatpak
+ * default rather than refusing to exist.
+ */
+function safeClientBin(bin: string | undefined): bin is string {
+  return !!bin && isSafeLaunchId(bin);
+}
+
 /**
  * Stream `ref` fullscreen in Gaming Mode, optionally with a pinned card's profile. Encodes the
  * target into the STREAM shortcut's launch options — one hidden shortcut serves every host —
@@ -369,6 +379,12 @@ export async function launchStream(ref: string, opts: LaunchOpts = {}): Promise<
   // Set only for a NATIVE client install; absent, the wrapper takes its flatpak default, so every
   // existing Deck install produces byte-identical launch options to before.
   if (clientBin) {
+    // The one launch-option value that comes from the backend rather than a store id, and so
+    // the one that could carry a space: a path like `/home/deck/my apps/punktfunk-client` would
+    // split Steam's tokenizer and land its tail in front of %command% as a bogus env token.
+    if (!isSafeLaunchId(clientBin)) {
+      throw new Error(`client path can't ride Steam's launch options: ${clientBin}`);
+    }
     env.push(`PF_CLIENT_BIN=${clientBin}`);
   }
   if (opts.profileId) {

clients/decky/src/trust.tsx

@@ -55,9 +55,22 @@ export const TrustSheet: FC<{
 
   // Request access pins what the host ADVERTISES. The record's own pin is a different thing:
   // a host that already has one streams without ever opening this sheet.
-  const canRequestAccess = host.advertisedFp !== "";
+  const hasIdentity = host.advertisedFp !== "";
+  // A host advertising `pair=optional` admits anyone who pins its identity — there is no
+  // operator decision to wait for, and asking for one would be a wait that never ends and a
+  // record claiming somebody approved this Deck when nobody did. `paired` means the PIN
+  // ceremony or a real approval; the desktop client records exactly this case as *trusted*.
+  const needsApproval = host.pairPolicy !== "optional";
+  const canRequestAccess = hasIdentity && needsApproval;
+  const canTrustDirectly = hasIdentity && !needsApproval;
 
-  const requestAccess = async () => {
+  /**
+   * Pin the advertised identity, then stream.
+   *
+   * `approval` is what differs between the two doors, and it is not cosmetic: it decides whether
+   * the launch waits ~185 s for an operator AND whether the record ends up marked paired.
+   */
+  const letIn = async (approval: boolean) => {
     setBusy(true);
     setError(null);
     const { host: h, onStream: stream, onChanged: changed } = props.current;
@@ -71,15 +84,17 @@ export const TrustSheet: FC<{
         return;
       }
       changed();
-      // Step 2: the launch itself waits for the approval. The session's plain connecting screen
+      // Step 2: the launch. Under approval it PARKS — and the session's plain connecting screen
       // looks identical whether it is parked or hanging, so say what is about to happen BEFORE
-      // it starts — this toast is a patch over that, and the real fix belongs in the session.
-      toaster.toast({
-        title: "Punktfunk",
-        body: `Approve this Deck in ${h.name}’s console — the stream starts by itself`,
-        duration: 10_000,
-      });
-      stream({ requestAccess: true });
+      // it starts. That toast is a patch over that, and the real fix belongs in the session.
+      if (approval) {
+        toaster.toast({
+          title: "Punktfunk",
+          body: `Approve this Deck in ${h.name}’s console — the stream starts by itself`,
+          duration: 10_000,
+        });
+      }
+      stream({ requestAccess: approval });
       closeModal?.();
     } catch (e) {
       setError(String(e));
@@ -109,9 +124,11 @@ export const TrustSheet: FC<{
         Connect to {host.name}
       </div>
       <div style={{ opacity: 0.8, marginBottom: "1em" }}>
-        {canRequestAccess
-          ? `${host.name} needs to let this device in before it can stream.`
-          : "No advertised identity for this host — pair with a PIN instead."}
+        {!hasIdentity
+          ? "No advertised identity for this host — pair with a PIN instead."
+          : canTrustDirectly
+            ? `${host.name} accepts new devices. Connecting pins its identity so later streams are silent.`
+            : `${host.name} needs to let this device in before it can stream.`}
       </div>
       {error && (
         <div style={{ color: "#ff6b6b", marginBottom: "0.6em" }}>{error}</div>
@@ -119,10 +136,15 @@ export const TrustSheet: FC<{
 
       <Focusable style={{ display: "flex", flexDirection: "column", gap: "0.5em" }}>
         {canRequestAccess && (
-          <DialogButton disabled={busy} onClick={requestAccess}>
+          <DialogButton disabled={busy} onClick={() => void letIn(true)}>
             {busy ? <Spinner style={{ height: "1em" }} /> : "Request access"}
           </DialogButton>
         )}
+        {canTrustDirectly && (
+          <DialogButton disabled={busy} onClick={() => void letIn(false)}>
+            {busy ? <Spinner style={{ height: "1em" }} /> : "Connect"}
+          </DialogButton>
+        )}
         <DialogButton disabled={busy} onClick={usePin}>
           Use a PIN instead…
         </DialogButton>

crates/pf-client-core/src/discovery.rs

@@ -54,8 +54,8 @@ pub enum DiscoveryEvent {
     Removed { fullname: String },
 }
 
-/// Browse continuously for the app's lifetime. The thread exits when the receiver is
-/// dropped (the send fails) or the daemon dies.
+/// Browse continuously. The worker exits when the returned receiver is dropped, or when the
+/// daemon dies — checked on a tick, so it stops even on a LAN where no advert ever arrives.
 pub fn browse() -> async_channel::Receiver<DiscoveryEvent> {
     let (tx, rx) = async_channel::unbounded();
     std::thread::Builder::new()
@@ -75,7 +75,24 @@ pub fn browse() -> async_channel::Receiver<DiscoveryEvent> {
                     return;
                 }
             };
-            while let Ok(event) = receiver.recv() {
+            // Polled rather than blocked on: the worker has to notice that its consumer went
+            // away even when NOTHING is arriving, which is the normal state of a LAN with no
+            // hosts on it. A plain `recv()` parks forever there, and the ignored-event arm below
+            // never touches `tx` — so a bounded consumer like `discover_for` would leak this
+            // thread and its daemon (another thread, and a socket bound to :5353) on every call.
+            loop {
+                // Checked at the TOP so it also covers the arms below that `continue` without
+                // ever touching `tx` — the ignored event kinds, and an advert with no IPv4
+                // address. Those are the paths that would otherwise keep this thread alive with
+                // nobody to send to.
+                if tx.is_closed() {
+                    break;
+                }
+                let event = match receiver.recv_timeout(Duration::from_millis(250)) {
+                    Ok(event) => event,
+                    Err(_) if receiver.is_disconnected() => break,
+                    Err(_) => continue,
+                };
                 let update = match event {
                     ServiceEvent::ServiceResolved(info) => {
                         let props = info.get_properties();
@@ -171,8 +188,8 @@ pub fn discover_for(timeout: Duration) -> Vec<DiscoveredHost> {
     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.
+    // Dropping the receiver is what stops the worker — it polls for that, so this holds even
+    // when nothing is advertising. Without it a one-shot consumer would leak a browse per call.
     drop(rx);
     sorted(adverts)
 }