fix(cli): discover reads the host store without writing to it

`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.
This commit is contained in:
2026-08-04 21:07:37 +02:00
parent 6267dcdcd3
commit 414380fc9e
2 changed files with 22 additions and 6 deletions
+5 -1
View File
@@ -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>,
+17 -5
View File
@@ -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.