mcpmesh-node 0.20.5

//! Keeping the persisted dial hint honest (#124).
//!
//! `PeerEntry::last_addr` is the pairing-proven dial-back hint: the address we attach to a dial so
//! a just-paired peer is reachable without waiting on discovery (#27). Until now it was written in
//! exactly ONE place — the pairing rendezvous — and never updated again.
//!
//! That made it permanently wrong the moment a peer changed networks. `stored_dial_addr` REPLACES
//! the bare-id dial with the hint, so a stale entry means the direct-candidate set is exactly one
//! dead address: every dial and every probe offers iroh that address, fails, and falls back to the
//! relay. Forever, because nothing rewrote it. An embedder measured it as a whole fleet stuck on
//! relay at ~800ms while a freshly-paired identity on the same two machines punched direct at
//! 11-23ms, and it survived daemon restarts because the hint lives in redb (#124).
//!
//! The fix is to refresh the hint from a peer we are demonstrably talking to. A live authenticated
//! connection knows the remote's ACTUAL address; that is ground truth, and writing it back makes
//! the hint self-healing across network changes. A stale hint then costs one failed attempt rather
//! than being permanent.

use std::sync::Arc;

use super::MeshState;

/// Refresh `last_addr` for `endpoint_id` from a LIVE connection, if it has changed (#124).
///
/// Best-effort and deliberately quiet: this is cache maintenance on a path whose real job is
/// already done, so every failure — an unknown peer, an empty path snapshot, a serialize error, a
/// redb write error — leaves the stored hint exactly as it was. A stale hint is a slow dial; a
/// *wrong* write could be worse, so the bar for replacing one is a connection that is up.
///
/// **Only writes on a CHANGE**, decided inside the store's write transaction. The value is not
/// static — it flips as a session's open IP paths change, and across a restart — so this is a real
/// reduction, not the "changes approximately never" an earlier version claimed.
///
/// **Never downgrades `Some` to `None`**, matching the pairing path's rule: an empty path snapshot
/// means we learned nothing, not that the peer has no address.
pub(crate) fn refresh(mesh: &Arc<MeshState>, endpoint_id: [u8; 32], observed: String) {
    let mesh = mesh.clone();
    // Detached, and taking NO lock. The atomicity lives in the store's single write transaction —
    // a lock here would exclude only `rename_peer`, since the pairing writes and `add_peer` take
    // none. The caller is #92's path watcher, whose own job is to push a frame WHEN IT HAPPENS, so
    // it must not queue behind cache maintenance. The redb work blocks: blocking pool (fs rule).
    tokio::task::spawn_blocking(move || write_if_changed(&mesh, endpoint_id, observed));
}

/// The store half of [`refresh`], separated so the rules are testable without a live connection:
/// replace a changed hint, skip an unchanged one, and never CREATE a peer.
fn write_if_changed(mesh: &Arc<MeshState>, endpoint_id: [u8; 32], observed: String) {
    // ONE redb write transaction does the read, the change-check and the write. A lock cannot
    // substitute: the pairing writes and `add_peer` take none, so a `reload_lock`-guarded
    // resolve-then-add excludes only `rename_peer` and still lost a verified `user_id` at a
    // measured 33% under a concurrent re-pair (#124 review).
    match mesh.store.set_last_addr(&endpoint_id, &observed) {
        Ok(true) => tracing::debug!("refreshed the dial hint from a live connection (#124)"),
        Ok(false) => {} // unchanged, or not a stored peer — both are no-ops by design
        // Cache maintenance: the session is already up and unaffected.
        Err(e) => tracing::debug!(%e, "could not persist refreshed dial hint"),
    }
}

/// The remote's DIRECT address as this connection sees it, serialized the way
/// [`crate::allowlist::PeerEntry::last_addr`] stores them.
///
/// **Filters to IP paths, and returns `None` when only a relay path is open (#124 review).** The
/// first version took every path unfiltered, which is actively harmful: a hint is *replacing* the
/// bare-id dial, so persisting a relay URL leaves the direct-candidate set EMPTY. Measured — a
/// seeded direct hint was overwritten with a relay URL and stayed that way for the rest of the
/// session — and it manufactures in production exactly the state this repo's own tests seed
/// deliberately to force a relayed session. A relay URL must never become a dial hint.
///
/// `None` (empty snapshot, no IP path, or a serialize failure) means "learned nothing", which the
/// caller treats as leave-alone rather than clear.
pub(crate) fn observed_for(conn: &iroh::endpoint::Connection) -> Option<String> {
    let paths = conn.paths();
    // EVERY open IP path, with no preference expressed — deliberately. An earlier version ordered
    // selected-first, which reads like #64's rule but is dead code: `EndpointAddr::from_parts`
    // collects into a `BTreeSet`, so ordering is discarded and the two forms serialize
    // byte-identically (#124 review). Each open QUIC path is a validated 4-tuple we reached this
    // peer at, so storing them all is correct for a dial hint; the canonical set ordering is also
    // what makes the unchanged-check stable.
    let addrs: Vec<iroh::TransportAddr> = paths
        .iter()
        .filter(|p| p.is_ip())
        .map(|p| p.remote_addr().clone())
        .collect();
    if addrs.is_empty() {
        return None; // relay-only, or nothing open: keep whatever we already had
    }
    serde_json::to_string(&iroh::EndpointAddr::from_parts(conn.remote_id(), addrs)).ok()
}

#[cfg(test)]
mod tests {
    use crate::allowlist::PeerEntry;

    /// #124: a hint that has gone stale must be REPLACED by what a live connection observes.
    ///
    /// This is the bug the embedder measured: after a network change both roots held each other's
    /// pre-change address, `stored_dial_addr` handed iroh that one dead address as the entire
    /// direct-candidate set, and every session fell back to the relay — permanently, because
    /// nothing rewrote the hint. A freshly-paired identity on the same hardware punched direct.
    ///
    /// Driven through the same store the daemon uses, with the observed address supplied directly:
    /// pinning it through a real network change would need two machines and a hotspot.
    #[tokio::test(flavor = "multi_thread")]
    async fn a_stale_hint_is_replaced_and_an_unchanged_one_is_not_rewritten() {
        let dir = tempfile::tempdir().unwrap();
        let cfg = dir.path().join("config.toml");
        std::fs::write(&cfg, "").unwrap();
        let mesh = crate::daemon::testutil::hermetic_mesh(cfg).await;
        let eid = [3u8; 32];

        let stale = r#"{"id":"stale","addrs":[]}"#.to_string();
        mesh.store
            .add(PeerEntry {
                endpoint_id: eid,
                nickname: "bob".into(),
                services: vec!["notes".into(), "kb".into()],
                // NON-EMPTY deliberately (#124 third review): the earlier fixture seeded these
                // empty, so mutations that clobbered `user_id`/`services`/`paired_at` all
                // SURVIVED — including the loss of a verified `user_id`, which is the entire
                // reason this code takes the care it does.
                paired_at: Some("2026-07-29T00:00:00Z".into()),
                user_id: Some("b64u:someuser".into()),
                last_addr: Some(stale.clone()),
            })
            .unwrap();

        // What a live connection would observe now — a DIFFERENT address.
        let fresh = r#"{"id":"fresh","addrs":[]}"#.to_string();
        super::write_if_changed(&mesh, eid, fresh.clone());
        assert_eq!(
            mesh.store.resolve(&eid).unwrap().unwrap().last_addr,
            Some(fresh.clone()),
            "a live observation must overwrite a stale hint — leaving it is #124: the peer is \
             dialed at a dead address forever and every session falls back to the relay"
        );

        // Re-observing the SAME address must SKIP the write. Asserting the value is unchanged
        // would be vacuous — a rewrite stores an identical value (#124 review caught exactly that)
        // — so assert on the store's own report of whether it wrote.
        assert!(
            !mesh
                .store
                .set_last_addr(&eid, &fresh)
                .expect("store readable"),
            "an unchanged observation must not write: every Selected event would otherwise cost a \
             blocking redb write"
        );
        assert!(
            mesh.store
                .set_last_addr(&eid, r#"{"id":"newer","addrs":[]}"#)
                .expect("store readable"),
            "and a CHANGED observation must write"
        );

        // An unknown peer is not invented — refresh is maintenance on a stored row, not a source
        // of trust. A bare-eid probe must never create an allowlist entry.
        super::write_if_changed(&mesh, [9u8; 32], fresh);
        assert!(
            mesh.store.resolve(&[9u8; 32]).unwrap().is_none(),
            "refreshing must never CREATE a peer — that would be an authorization path"
        );

        // EVERY identity field the pairing path protects must survive a hint refresh. `user_id`
        // above all: a resolve-then-add refresh downgraded a verified one to `None` at a measured
        // 33% under a concurrent re-pair, which is why the write is a single transaction.
        let row = mesh.store.resolve(&eid).unwrap().unwrap();
        assert_eq!(row.nickname, "bob");
        assert_eq!(row.endpoint_id, eid);
        assert_eq!(
            row.user_id.as_deref(),
            Some("b64u:someuser"),
            "a verified user_id must NEVER be downgraded by a dial-hint refresh"
        );
        assert_eq!(
            row.services,
            vec!["notes".to_string(), "kb".to_string()],
            "the dial directory must survive — clobbering it is the reverse-pairing bug"
        );
        assert_eq!(
            row.paired_at.as_deref(),
            Some("2026-07-29T00:00:00Z"),
            "the original pairing stamp stands"
        );
    }
}