From 414380fc9eb56cf337863bc0ecf557999eff05d2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 4 Aug 2026 21:07:37 +0200 Subject: [PATCH] fix(cli): discover reads the host store without writing to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `KnownHosts::load()` mints a stable id for any record that lacks one and SAVES it — which makes it a write, and `discover` was calling it purely to annotate what the browse found with saved/paired. It never hands those ids back to anyone. That matters because the Decky panel issues `discover` and `hosts list` together, in parallel. Against a store written before ids existed, both processes read it, both mint DIFFERENT ids for the same record, and both save. Whichever loses the race has already handed its ids to its caller — so the panel could draw a row whose host reference no longer resolves, and pressing it would exit 5 ("no saved host matches") until the next refresh settled things. `KnownHosts::read()` is `load` without the mint: the store exactly as it is on disk. `discover` uses it; every caller that dials a host by id still uses `load`, so ids are still minted the first time anything needs one. Verified on a fixture store with no ids: `punktfunk discover` leaves it byte-identical, and a following `punktfunk hosts list` mints as before. --- clients/cli/src/main.rs | 6 +++++- crates/pf-client-core/src/trust.rs | 22 +++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/clients/cli/src/main.rs b/clients/cli/src/main.rs index bf993d0c..98d4a62c 100644 --- a/clients/cli/src/main.rs +++ b/clients/cli/src/main.rs @@ -363,7 +363,11 @@ from the config directory for a true factory reset." .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(); + // `read`, not `load`: this verb only LOOKS at the records to annotate what it found, and + // never hands their ids back. `load` would mint ids for a pre-mint store and save them — + // a write from a read-only verb, and one that races the `hosts list` a caller is very + // likely running at the same moment (the Decky panel issues both together). + let known = KnownHosts::read(); let rows: Vec<( &pf_client_core::discovery::DiscoveredHost, Option<&KnownHost>, diff --git a/crates/pf-client-core/src/trust.rs b/crates/pf-client-core/src/trust.rs index 929e245c..9ab18cf2 100644 --- a/crates/pf-client-core/src/trust.rs +++ b/crates/pf-client-core/src/trust.rs @@ -232,17 +232,29 @@ impl KnownHosts { /// A read-only config dir just keeps re-minting in memory, which harms nothing: no lookup /// is keyed by the id yet (design §4.5). pub fn load() -> KnownHosts { - let mut k: KnownHosts = Self::path() - .and_then(|p| Ok(std::fs::read_to_string(p)?)) - .ok() - .and_then(|s| serde_json::from_str(&s).ok()) - .unwrap_or_default(); + let mut k = Self::read(); if k.mint_missing_ids() { let _ = k.save(); } k } + /// The store exactly as it is on disk — no mint, and so no write. + /// + /// For a consumer that only needs to LOOK at the records (annotating a discovery result + /// against them, say) and never dials one by id. [`KnownHosts::load`]'s mint is a write, and + /// two processes started together against a pre-mint store will each mint a *different* id + /// for the same record and race to save it — after which whichever one already handed its + /// ids to a caller has handed out references that no longer resolve. A read that stays a + /// read cannot take part in that. + pub fn read() -> KnownHosts { + Self::path() + .and_then(|p| Ok(std::fs::read_to_string(p)?)) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()) + .unwrap_or_default() + } + /// Give every record still missing one a stable id; returns true if anything changed /// (i.e. whether this needs persisting). Idempotent — a store that has been through it /// once is left byte-identical.