diff --git a/core/archipelago/src/api/rpc/federation/handlers.rs b/core/archipelago/src/api/rpc/federation/handlers.rs index 5e4679e8..7a60257d 100644 --- a/core/archipelago/src/api/rpc/federation/handlers.rs +++ b/core/archipelago/src/api/rpc/federation/handlers.rs @@ -262,6 +262,16 @@ impl RpcHandler { if let Some(state) = &n.last_state { obj["last_state"] = serde_json::to_value(state).unwrap_or_default(); } + // FED-02: surface the most recent sync failure so the operator + // sees a stale peer in the UI instead of it living only in the + // node's debug log. Omitted (not null) when the last attempt + // succeeded, so a recovered peer's badge disappears. + if let Some(err) = &n.last_sync_error { + obj["last_sync_error"] = serde_json::json!(err); + } + if let Some(at) = &n.last_sync_error_at { + obj["last_sync_error_at"] = serde_json::json!(at); + } obj }) .collect(); @@ -659,6 +669,8 @@ impl RpcHandler { fips_npub, last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }; federation::add_node(&self.config.data_dir, node).await?; diff --git a/core/archipelago/src/federation/invites.rs b/core/archipelago/src/federation/invites.rs index d05a13dd..51ad2692 100644 --- a/core/archipelago/src/federation/invites.rs +++ b/core/archipelago/src/federation/invites.rs @@ -199,6 +199,8 @@ pub async fn accept_invite( fips_npub: fips_npub.clone(), last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }; add_node(data_dir, node.clone()).await?; diff --git a/core/archipelago/src/federation/mod.rs b/core/archipelago/src/federation/mod.rs index cb99fcdc..7be272e3 100644 --- a/core/archipelago/src/federation/mod.rs +++ b/core/archipelago/src/federation/mod.rs @@ -21,7 +21,7 @@ pub(crate) use storage::load_invites; #[allow(unused_imports)] pub use storage::{ add_node, fips_npub_for_onion, load_nodes, load_removed_dids, record_peer_transport, - remove_node, save_nodes, set_trust_level, update_node, + record_sync_result, remove_node, save_nodes, set_trust_level, update_node, }; pub use sync::{build_local_state, deploy_to_peer, sync_with_peer, sync_with_peer_by_did}; pub use types::{AppStatus, FederatedNode, NodeStateSnapshot, TrustLevel}; diff --git a/core/archipelago/src/federation/storage.rs b/core/archipelago/src/federation/storage.rs index e6c07d58..434524e4 100644 --- a/core/archipelago/src/federation/storage.rs +++ b/core/archipelago/src/federation/storage.rs @@ -174,6 +174,69 @@ pub async fn record_peer_transport( Ok(()) } +/// Upper bound on a persisted `last_sync_error` message, in characters. +/// +/// T-01-18: the periodic loop records an outcome for every peer on every +/// pass, so an unbounded error string (a peer echoing a huge body, a deep +/// `anyhow` chain) would be rewritten into `nodes.json` every 90 seconds. +/// Counted in `char`s, not bytes, so truncation can never split a UTF-8 +/// sequence and produce a file that fails to deserialize. +pub(crate) const MAX_SYNC_ERROR_CHARS: usize = 256; + +/// Record the outcome of the most recent federation sync attempt with a peer. +/// +/// `Err(msg)` stores the message (truncated to `MAX_SYNC_ERROR_CHARS`) plus +/// the current time; `Ok(())` clears both fields so a recovered peer stops +/// showing a stale error badge. Only the named DID is touched. +/// +/// Why this exists: both periodic sync loops previously logged failures at +/// `debug!` and nothing else, so a peer that had not synced in days looked +/// identical in the UI to one that synced a minute ago (FED-02). +/// +/// A DID that isn't in the node list is a silent `Ok` and writes nothing — +/// a peer the operator removed while a sync was in flight must not be +/// resurrected by that sync's error write. This function never creates a +/// node entry. +pub async fn record_sync_result( + data_dir: &Path, + did: &str, + outcome: Result<(), String>, +) -> Result<()> { + let _guard = FEDERATION_STORE_LOCK.lock().await; + let mut nodes = load_nodes_inner(data_dir).await?; + + let Some(node) = nodes.iter_mut().find(|n| n.did == did) else { + // Unknown/removed peer: nothing to record against. Not an error. + return Ok(()); + }; + + let changed = match outcome { + Err(msg) => { + let truncated: String = msg.chars().take(MAX_SYNC_ERROR_CHARS).collect(); + node.last_sync_error = Some(truncated); + node.last_sync_error_at = Some(chrono::Utc::now().to_rfc3339()); + true + } + Ok(()) => { + // Peer recovered — drop the badge rather than leaving a stale one. + let had_error = node.last_sync_error.is_some() || node.last_sync_error_at.is_some(); + node.last_sync_error = None; + node.last_sync_error_at = None; + had_error + } + }; + + // A healthy peer stays healthy on most passes, and the surviving loop + // calls this for every peer every 90s. Skipping the write when nothing + // actually changed keeps the steady state read-only, so this failure + // surfacing doesn't add a rewrite of nodes.json (and lock contention + // with `federation.remove-node`) every single pass. + if changed { + save_nodes_inner(data_dir, &nodes).await?; + } + Ok(()) +} + pub async fn save_nodes(data_dir: &Path, nodes: &[FederatedNode]) -> Result<()> { let _guard = FEDERATION_STORE_LOCK.lock().await; save_nodes_inner(data_dir, nodes).await @@ -435,6 +498,8 @@ mod tests { fips_npub: None, last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, } } @@ -750,6 +815,122 @@ mod tests { ); } + /// FED-02: a failed sync must leave a durable, per-peer record instead of + /// only a `debug!` line, so the operator can tell a peer that hasn't + /// synced in days from one that synced a minute ago. + #[tokio::test] + async fn test_record_sync_result_persists_error() { + let dir = tempfile::tempdir().unwrap(); + add_node(dir.path(), make_node("did:key:z1", "a.onion")) + .await + .unwrap(); + add_node(dir.path(), make_node("did:key:z2", "b.onion")) + .await + .unwrap(); + + record_sync_result(dir.path(), "did:key:z1", Err("peer unreachable".to_string())) + .await + .unwrap(); + + let nodes = load_nodes(dir.path()).await.unwrap(); + let n1 = nodes.iter().find(|n| n.did == "did:key:z1").unwrap(); + assert_eq!(n1.last_sync_error.as_deref(), Some("peer unreachable")); + assert!( + n1.last_sync_error_at.is_some(), + "an error must carry the time it happened" + ); + + let n2 = nodes.iter().find(|n| n.did == "did:key:z2").unwrap(); + assert!( + n2.last_sync_error.is_none(), + "only the failing peer may be marked" + ); + } + + /// FED-02 adjacency edge: the badge must not outlive the failure. A + /// successful sync clears the previously recorded error for that peer. + #[tokio::test] + async fn test_record_sync_result_success_clears_error() { + let dir = tempfile::tempdir().unwrap(); + add_node(dir.path(), make_node("did:key:z1", "a.onion")) + .await + .unwrap(); + + record_sync_result(dir.path(), "did:key:z1", Err("timed out".to_string())) + .await + .unwrap(); + assert!(load_nodes(dir.path()).await.unwrap()[0] + .last_sync_error + .is_some()); + + record_sync_result(dir.path(), "did:key:z1", Ok(())).await.unwrap(); + + let nodes = load_nodes(dir.path()).await.unwrap(); + assert!( + nodes[0].last_sync_error.is_none(), + "a recovered peer must not keep its stale error badge" + ); + assert!( + nodes[0].last_sync_error_at.is_none(), + "the error timestamp must clear with the error" + ); + } + + /// FED-02: a peer removed mid-pass must not be resurrected by the + /// in-flight sync attempt's error write. Missing DID is a silent Ok. + #[tokio::test] + async fn test_record_sync_result_missing_did_is_noop() { + let dir = tempfile::tempdir().unwrap(); + add_node(dir.path(), make_node("did:key:z1", "a.onion")) + .await + .unwrap(); + + record_sync_result(dir.path(), "did:key:zGONE", Err("unreachable".to_string())) + .await + .expect("recording against an unknown DID must be a silent Ok, not an error"); + + let nodes = load_nodes(dir.path()).await.unwrap(); + assert_eq!(nodes.len(), 1, "a removed peer must not be resurrected"); + assert_eq!(nodes[0].did, "did:key:z1"); + assert!(nodes[0].last_sync_error.is_none()); + } + + /// FED-02 empty edge: recording against an empty node store writes + /// nothing and errors on nobody (the zero-federated-node sync pass). + #[tokio::test] + async fn test_record_sync_result_on_empty_store_is_noop() { + let dir = tempfile::tempdir().unwrap(); + record_sync_result(dir.path(), "did:key:zAny", Err("boom".to_string())) + .await + .unwrap(); + assert!(load_nodes(dir.path()).await.unwrap().is_empty()); + assert!( + !dir.path().join(FEDERATION_DIR).join(NODES_FILE).exists(), + "a no-op must not create the node file" + ); + } + + /// T-01-18: an unbounded error string must not bloat nodes.json on every + /// failed pass. The recorded message is truncated before persistence. + #[tokio::test] + async fn test_record_sync_result_truncates_long_error() { + let dir = tempfile::tempdir().unwrap(); + add_node(dir.path(), make_node("did:key:z1", "a.onion")) + .await + .unwrap(); + + let huge = "x".repeat(5000); + record_sync_result(dir.path(), "did:key:z1", Err(huge)).await.unwrap(); + + let nodes = load_nodes(dir.path()).await.unwrap(); + let msg = nodes[0].last_sync_error.as_deref().unwrap(); + assert!( + msg.chars().count() <= MAX_SYNC_ERROR_CHARS, + "recorded error must be truncated to {MAX_SYNC_ERROR_CHARS} chars, got {}", + msg.chars().count() + ); + } + #[tokio::test] async fn test_update_node_state() { let dir = tempfile::tempdir().unwrap(); diff --git a/core/archipelago/src/federation/sync.rs b/core/archipelago/src/federation/sync.rs index 278eb8b4..6d699551 100644 --- a/core/archipelago/src/federation/sync.rs +++ b/core/archipelago/src/federation/sync.rs @@ -187,6 +187,8 @@ async fn merge_transitive_peers( fips_npub: hint.fips_npub.clone(), last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }); added += 1; } @@ -375,6 +377,8 @@ mod tests { fips_npub: Some("npub1a".into()), last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }, FederatedNode { did: "did:key:zObserver".into(), @@ -388,6 +392,8 @@ mod tests { fips_npub: Some("npub1b".into()), last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }, FederatedNode { did: "did:key:zUntrusted".into(), @@ -401,6 +407,8 @@ mod tests { fips_npub: None, last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }, ]; let state = build_local_state( @@ -443,6 +451,8 @@ mod tests { fips_npub: None, last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }], ) .await diff --git a/core/archipelago/src/federation/types.rs b/core/archipelago/src/federation/types.rs index 0e861d92..0758643e 100644 --- a/core/archipelago/src/federation/types.rs +++ b/core/archipelago/src/federation/types.rs @@ -77,6 +77,23 @@ pub struct FederatedNode { /// RFC 3339 timestamp of the last_transport value. #[serde(default)] pub last_transport_at: Option, + /// Error from the most recent federation sync attempt with this peer, + /// `None` when that attempt succeeded. Written back after every attempt + /// (same shape as `last_transport`, for the failure side) so the + /// operator can tell a peer that hasn't synced in days from one that + /// synced a minute ago — previously a failed sync existed only as a + /// `debug!` log line, making the two indistinguishable in the UI. + /// + /// Truncated to `storage::MAX_SYNC_ERROR_CHARS` before it is persisted + /// so a pathological error can't bloat `nodes.json` on every pass, and + /// carries only the error's own display string — never credential + /// material (see FED-02's privacy prohibition). + #[serde(default)] + pub last_sync_error: Option, + /// RFC 3339 timestamp of the last_sync_error value. Cleared together + /// with `last_sync_error` when the peer recovers. + #[serde(default)] + pub last_sync_error_at: Option, } /// State snapshot received from a federated peer during sync. @@ -204,6 +221,8 @@ mod tests { fips_npub: None, last_transport: None, last_transport_at: None, + last_sync_error: None, + last_sync_error_at: None, }; let json = serde_json::to_string(&node).unwrap(); let parsed: FederatedNode = serde_json::from_str(&json).unwrap(); diff --git a/core/archipelago/src/server.rs b/core/archipelago/src/server.rs index 82c4cbf4..d6f215e5 100644 --- a/core/archipelago/src/server.rs +++ b/core/archipelago/src/server.rs @@ -544,6 +544,15 @@ impl Server { { Ok(state) => { ok += 1; + // FED-02: clear any error this peer accumulated + // while it was unreachable, so the operator's + // sync-error badge disappears on recovery + // instead of sticking around forever. + crate::federation::record_sync_result( + &data_dir, &node.did, Ok(()), + ) + .await + .ok(); // Asymmetry self-heal: if this peer's exported // trusted list doesn't include us, our original // peer-joined never landed (e.g. it was sent @@ -576,7 +585,22 @@ impl Server { } } Err(e) => { - debug!(peer = %node.did, error = %e, "federation auto-sync (non-fatal)") + debug!(peer = %node.did, error = %e, "federation auto-sync (non-fatal)"); + // FED-02: persist the failure on the peer's own + // record too. The debug! line above is kept — + // persisting is additive, not a replacement for + // logs — but on its own it left a peer that + // hadn't synced in days looking identical in the + // UI to one that synced a minute ago. The stored + // message is the error's display string, bounded + // by record_sync_result to MAX_SYNC_ERROR_CHARS. + crate::federation::record_sync_result( + &data_dir, + &node.did, + Err(format!("{e:#}")), + ) + .await + .ok(); } } } diff --git a/neode-ui/src/views/federation/NodeList.vue b/neode-ui/src/views/federation/NodeList.vue index d881a6b5..a200c992 100644 --- a/neode-ui/src/views/federation/NodeList.vue +++ b/neode-ui/src/views/federation/NodeList.vue @@ -58,6 +58,12 @@ :class="transportBadge(node)!.cls" :title="transportBadge(node)!.title" >{{ transportBadge(node)!.label }} + SYNC {{ transportBadge(node)!.label }} + SYNC {{ node.trust_level }} @@ -195,4 +207,20 @@ function transportBadge(node: FederatedNode): { label: string; cls: string; titl return null } } + +// FED-02: the most recent sync attempt with this peer failed. Before this, +// the failure existed only as a `debug!` line on the node, so a peer that +// hadn't synced in days looked identical on this screen to one that synced a +// minute ago. Returns null when the peer's last attempt succeeded — the +// backend clears last_sync_error on success, so the badge disappears on +// recovery rather than sticking around. +// +// The row must stay single-line, so the badge itself is a fixed short label +// and the daemon's message plus when it happened go in the tooltip (the same +// truncate + :title treatment the node-name span uses). +function syncErrorBadge(node: FederatedNode): { title: string } | null { + if (!node.last_sync_error) return null + const age = node.last_sync_error_at ? timeAgo(node.last_sync_error_at) : 'unknown' + return { title: `Last sync failed · ${age} · ${node.last_sync_error}` } +} diff --git a/neode-ui/src/views/federation/__tests__/NodeList.test.ts b/neode-ui/src/views/federation/__tests__/NodeList.test.ts index 859244c2..9700fb5c 100644 --- a/neode-ui/src/views/federation/__tests__/NodeList.test.ts +++ b/neode-ui/src/views/federation/__tests__/NodeList.test.ts @@ -29,4 +29,75 @@ describe('NodeList', () => { expect(wrapper.text()).toContain('Trusted Node') expect(wrapper.text()).not.toContain('Loading nodes...') }) + + // FED-02: a sync failure used to exist only as a debug! log line on the + // node, so a peer that hadn't synced in days looked identical to one that + // synced a minute ago. It must now be visible on the node's own row. + it('shows a sync-error badge on a node whose last sync failed', () => { + const failing: FederatedNode = { + ...trustedNode, + last_sync_error: 'Failed to reach federated peer', + last_sync_error_at: '2026-06-11T00:00:00Z', + } + const wrapper = mount(NodeList, { + props: { + nodes: [failing], + loading: false, + error: '', + syncResults: [], + dwnSyncDotClass: 'bg-green-400', + cleaningNodes: false, + }, + }) + + const badge = wrapper.find('[data-testid="sync-error-badge"]') + expect(badge.exists()).toBe(true) + expect(badge.text()).toBe('SYNC') + // The full message + when it happened live in the tooltip, so the row + // stays single-line (the node-name truncate idiom). + expect(badge.attributes('title')).toContain('Failed to reach federated peer') + }) + + // The guard against a badge that always renders: a healthy peer (and a + // peer that has recovered, since record_sync_result clears the field) must + // show no sync-error badge at all. + it('renders no sync-error badge when last_sync_error is unset', () => { + const wrapper = mount(NodeList, { + props: { + nodes: [trustedNode], + loading: false, + error: '', + syncResults: [], + dwnSyncDotClass: 'bg-green-400', + cleaningNodes: false, + }, + }) + + expect(wrapper.find('[data-testid="sync-error-badge"]').exists()).toBe(false) + }) + + // Peers (Observer level) render in a separate column — the badge must be + // on that row too, or a failing peer stays invisible. + it('shows the sync-error badge on an observer peer row', () => { + const peer: FederatedNode = { + ...trustedNode, + did: 'did:key:z6MkPeerNode', + trust_level: 'observer', + name: 'Peer Node', + last_sync_error: 'Peer returned 502 (via tor)', + last_sync_error_at: '2026-06-11T00:00:00Z', + } + const wrapper = mount(NodeList, { + props: { + nodes: [peer], + loading: false, + error: '', + syncResults: [], + dwnSyncDotClass: 'bg-green-400', + cleaningNodes: false, + }, + }) + + expect(wrapper.find('[data-testid="sync-error-badge"]').exists()).toBe(true) + }) }) diff --git a/neode-ui/src/views/federation/types.ts b/neode-ui/src/views/federation/types.ts index b6aa24c6..33e76fa4 100644 --- a/neode-ui/src/views/federation/types.ts +++ b/neode-ui/src/views/federation/types.ts @@ -31,6 +31,15 @@ export interface FederatedNode { last_transport?: 'fips' | 'tor' | 'mesh' | 'lan' /** RFC 3339 timestamp of last_transport. */ last_transport_at?: string + /** + * Error from the most recent federation sync attempt with this peer, or + * absent when the last attempt succeeded. Persisted per peer so a stale + * peer is visibly different from a healthy one instead of the failure + * living only in the node's debug log (FED-02). + */ + last_sync_error?: string + /** RFC 3339 timestamp of last_sync_error. */ + last_sync_error_at?: string } export interface DwnStatus {