archy/.planning/phases/01-federation-mesh-hardening/01-05-PLAN.md

---
phase: 01-federation-mesh-hardening
plan: 05
type: execute
wave: 2
depends_on: ["01-01"]
files_modified:
  - core/archipelago/src/federation/types.rs
  - core/archipelago/src/federation/storage.rs
  - core/archipelago/src/federation/sync.rs
  - core/archipelago/src/api/rpc/federation/handlers.rs
  - core/archipelago/src/server.rs
  - neode-ui/src/views/federation/types.ts
  - neode-ui/src/views/federation/NodeList.vue
autonomous: true
requirements: [FED-02]

must_haves:
  truths:
    - "A federation sync failure is recorded on the peer's node record and surfaced through federation.list-nodes, so the operator sees it in the UI instead of it existing only as a debug log line"
    - "A successful sync clears a previously recorded sync error for that peer — the badge does not persist after the peer recovers (FED-02 adjacency edge)"
    - "A periodic sync pass over zero federated nodes is a clean no-op: no error is recorded, nothing is written, and no error surfaces in the UI (FED-02 empty edge)"
    - "A state snapshot older than the one already stored for a peer does not overwrite the newer one — out-of-order sync responses cannot move a peer's status backwards (FED-02 ordering edge)"
    - "Exactly one periodic federation sync loop runs in the daemon; the redundant second loop is gone and every behavior unique to it is preserved in the surviving loop"
    - "Duplicate node entries do not accumulate across sync cycles — after sync settles the node list has one entry per federated node"
  prohibitions:
    - statement: "Making sync errors visible MUST NOT expose a peer's onion address, DID, or any transport secret in an error string rendered to a surface wider than the operator's own dashboard — a sync error message names what failed, never credential material"
      category: privacy
  artifacts:
    - path: core/archipelago/src/federation/types.rs
      provides: "last_sync_error / last_sync_error_at on FederatedNode"
      contains: "last_sync_error"
    - path: neode-ui/src/views/federation/NodeList.vue
      provides: "Operator-visible sync-error badge on a node row"
      contains: "last_sync_error"
  key_links:
    - from: core/archipelago/src/server.rs
      to: core/archipelago/src/federation/storage.rs
      via: "the periodic sync loop calls record_sync_result after each peer attempt instead of only debug-logging"
      pattern: "record_sync_result"
    - from: core/archipelago/src/api/rpc/federation/handlers.rs
      to: neode-ui/src/views/federation/NodeList.vue
      via: "federation.list-nodes emits last_sync_error, the node row renders it as a badge"
      pattern: "last_sync_error"
---

<objective>
Make federation sync converge and stop failing silently: one sync loop instead of two, a per-peer
sync error persisted and shown to the operator, and out-of-order snapshots unable to move a peer's
state backwards.

Purpose: FED-02. RESEARCH.md's anti-pattern list is explicit — both periodic sync loops in
`server.rs` log failures at `debug!` only, so a peer that has not synced in days looks identical to
one that synced a minute ago. The same section notes the two loops (90s at ~L497, 1800s at ~L840)
are redundant apart from one tail call, and that the redundancy doubles the write-race exposure that
plan 01-01 just locked down. Open Question 1 asks the reviewer to `git log -p` both loop-insertion
commits before deleting either — that check is a required step here, not an optional one.
Output: `last_sync_error` plumbed store → loop → RPC → UI badge, one surviving loop, and a
monotonicity guard on `update_node_state`.
</objective>

<execution_context>
@$HOME/.claude/gsd-core/workflows/execute-plan.md
@$HOME/.claude/gsd-core/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/STATE.md
@.planning/phases/01-federation-mesh-hardening/01-RESEARCH.md
@.planning/phases/01-federation-mesh-hardening/01-PATTERNS.md
@.planning/phases/01-federation-mesh-hardening/01-01-SUMMARY.md
@core/archipelago/src/federation/types.rs
</context>

## Artifacts this phase produces

Created or changed by **this plan**:

| Symbol | Kind | File |
|---|---|---|
| `FederatedNode.last_sync_error: Option<String>` | new optional field | `core/archipelago/src/federation/types.rs` |
| `FederatedNode.last_sync_error_at: Option<String>` | new optional field | same |
| `record_sync_result` | new pub async fn (records or clears a peer's sync error under the store lock) | `core/archipelago/src/federation/storage.rs` |
| `last_sync_error`, `last_sync_error_at` on `federation.list-nodes` | new response fields | `core/archipelago/src/api/rpc/federation/handlers.rs` |
| `FederatedNode.last_sync_error?` / `.last_sync_error_at?` | new TS interface fields | `neode-ui/src/views/federation/types.ts` |
| sync-error badge on a node row | Vue markup + class | `neode-ui/src/views/federation/NodeList.vue` |
| the 1800s periodic federation sync loop | **deleted** (its unique tail call moved into the 90s loop) | `core/archipelago/src/server.rs` |

<tasks>

<task type="tracer" tdd="true">
  <name>Task 1: End-to-end — a failed federation sync becomes visible to the operator</name>
  <files>core/archipelago/src/federation/types.rs, core/archipelago/src/federation/storage.rs, core/archipelago/src/api/rpc/federation/handlers.rs, core/archipelago/src/server.rs, neode-ui/src/views/federation/types.ts, neode-ui/src/views/federation/NodeList.vue</files>
  <read_first>
    - `core/archipelago/src/federation/types.rs` lines 50-146 — `FederatedNode`'s existing optional
      fields and the doc-comment convention on `last_transport` / `last_transport_at` (a result
      field written back after each attempt). The new pair mirrors that shape for the failure side.
    - `core/archipelago/src/federation/storage.rs` as left by plan 01-01 — `record_peer_transport`
      (the existing "write a result field back after an attempt" function) and the
      `FEDERATION_STORE_LOCK` wrapper + `*_inner` split convention the new function must follow.
    - `core/archipelago/src/api/rpc/federation/handlers.rs` `handle_federation_list_nodes`
      (from L220) — the `serde_json::json!` node object and the `if let Some(...)` conditional-field
      pattern the new fields must follow.
    - `core/archipelago/src/server.rs` lines 497-600 — the 90s periodic federation sync loop, its
      per-peer `sync_with_peer` call and the `debug!(peer = %node.did, error = %e, ...)` arm that
      currently swallows failures.
    - `neode-ui/src/views/federation/types.ts` lines 19-33 — the `FederatedNode` TS interface.
    - `neode-ui/src/views/federation/NodeList.vue` lines 40-140 — the trusted-node and peer rows,
      `transportBadge()` (L166) and `trustBadgeClass()` for the badge idiom to mirror, and the
      existing loading row.
    - `neode-ui/src/views/federation/__tests__/NodeList.test.ts` — the existing suite's mount
      conventions.
  </read_first>
  <behavior>
    - `record_sync_result(data_dir, did, Err("..."))` sets `last_sync_error` to the message and
      `last_sync_error_at` to an RFC 3339 timestamp on that node only.
    - `record_sync_result(data_dir, did, Ok(()))` clears both fields on that node.
    - `record_sync_result` for a DID that is not in the node list is a no-op returning Ok — a peer
      removed mid-pass must not be resurrected by an error write.
    - `federation.list-nodes` emits both fields when set and omits them when unset.
    - NodeList renders a sync-error badge on a node whose `last_sync_error` is set, and renders no
      such badge when it is unset.
  </behavior>
  <action>
    Write the Rust tests and the NodeList component test first and confirm they fail.

    Add `#[serde(default)] pub last_sync_error: Option<String>` and
    `#[serde(default)] pub last_sync_error_at: Option<String>` to `FederatedNode`, with a doc comment
    modelled on `last_transport`: these record the outcome of the most recent sync attempt so the
    operator can tell a stale peer from a healthy one, replacing a debug-only log line. Update the
    `make_node` test helper in `storage.rs`'s test module so the struct literal still compiles.

    Add `record_sync_result(data_dir: &Path, did: &str, outcome: Result<(), String>) -> Result<()>`
    to `storage.rs`, acquiring `FEDERATION_STORE_LOCK` and using the `*_inner` load/save functions
    established in 01-01. Missing DID is a silent Ok. Never create a node entry.

    In `server.rs`'s 90s loop, replace the debug-only failure arm with a call to `record_sync_result`
    carrying the error's display string, and call it with a success outcome on the success arm.
    Truncate the recorded message to a bounded length (256 characters) so a pathological error
    cannot bloat the node file. Keep the existing `debug!` line as well — persisting is additive,
    not a replacement for logs.

    In `handle_federation_list_nodes`, emit the two fields onto the node object using the same
    `if let Some(...)` conditional-insert pattern the existing optional fields use. Add the matching
    optional fields to the TS `FederatedNode` interface.

    In `NodeList.vue`, add a badge on the node row shown only when `last_sync_error` is set: red
    family (`alert-error`-adjacent classes already in the house style), short label, and the full
    message plus the timestamp in the element's `title` attribute — the row must stay single-line, so
    apply the same `truncate` + `:title` treatment the node-name span already uses. Place it beside
    the existing transport badge, not in place of it. Do not add a new nav entry, card, or view —
    only this badge inside the existing row.
  </action>
  <verify>
    <automated>cd core &amp;&amp; cargo test -p archipelago federation &amp;&amp; cd ../neode-ui &amp;&amp; test -f src/views/federation/__tests__/NodeList.test.ts &amp;&amp; npx vitest run src/views/federation/__tests__/NodeList.test.ts</automated>
  </verify>
  <acceptance_criteria>
    - `cd core && cargo test -p archipelago federation` exits 0 and includes a test named for the clear-on-success behavior and one for the missing-DID no-op.
    - `grep -c 'last_sync_error' core/archipelago/src/federation/types.rs` is at least 2.
    - `grep -c 'record_sync_result' core/archipelago/src/federation/storage.rs` is at least 1.
    - `grep -c 'record_sync_result' core/archipelago/src/server.rs` is at least 2 (the failure arm and the success arm).
    - `grep -c 'last_sync_error' core/archipelago/src/api/rpc/federation/handlers.rs` is at least 1.
    - `grep -c 'last_sync_error' neode-ui/src/views/federation/types.ts` is at least 1.
    - `grep -c 'last_sync_error' neode-ui/src/views/federation/NodeList.vue` is at least 1.
    - `cd neode-ui && npx vitest run src/views/federation/__tests__/NodeList.test.ts` exits 0 with a case asserting the badge is absent when the field is unset (the guard against a badge that always renders).
    - `cd neode-ui && npm run build` exits 0.
    - The SUMMARY records the pre-implementation failing output for both the Rust and the component test.
  </acceptance_criteria>
  <done>A sync failure is persisted per peer, travels through the RPC, and renders as a badge the operator can see — and clears when the peer recovers.</done>
</task>

<task type="auto">
  <name>Task 2: Collapse the two periodic sync loops into one</name>
  <reversibility rating="costly">Deleting a background loop changes daemon runtime behavior across
  the whole fleet on the next OTA; restoring it means re-deriving code that is gone from the tree
  rather than flipping a flag. Mitigated by moving — not discarding — the loop's unique tail call
  and by the required git-history check below.</reversibility>
  <files>core/archipelago/src/server.rs</files>
  <read_first>
    - `core/archipelago/src/server.rs` lines 497-600 (the 90s loop, including its asymmetry
      self-heal `notify_join` re-assertion) and lines 840-910 (the 1800s loop, whose unique tail
      call is `rpc.refresh_federation_mesh_peers()`).
    - The output of `git log -p -L 840,910:core/archipelago/src/server.rs` and
      `git log -p -L 497,600:core/archipelago/src/server.rs` — RESEARCH.md Assumption A2 flags that
      the 1800s loop may exist for an undocumented reason. Run this BEFORE deleting anything and
      record the finding in the SUMMARY.
    - `.planning/phases/01-federation-mesh-hardening/01-RESEARCH.md` — "Open Questions" item 1.
  </read_first>
  <action>
    First run the two `git log -p -L` commands above and write the answer to Open Question 1 into the
    SUMMARY: does the 1800s loop do anything the 90s loop does not, beyond
    `refresh_federation_mesh_peers()`? If the history shows a documented reason to keep it, STOP,
    do not delete it, and record that as a finding for the FED-03 review instead — the phase then
    keeps two loops and this task's remaining work is limited to routing both through
    `record_sync_result`.

    Otherwise: move the `refresh_federation_mesh_peers()` call to the tail of the 90s loop's
    completed pass (after the per-peer iteration, alongside the existing pass-complete log), thread
    whatever handle it needs into that task's captured state, and delete the entire 1800s
    `tokio::spawn` block. Keep the 90s loop's startup settle delay and its asymmetry self-heal
    unchanged.

    Make the surviving loop's zero-node case an explicit clean no-op: when `load_nodes` returns an
    empty list the pass continues to the next tick without writing anything and without recording a
    sync error against anyone.
  </action>
  <verify>
    <automated>cd core &amp;&amp; cargo build -p archipelago &amp;&amp; cargo test -p archipelago federation</automated>
  </verify>
  <acceptance_criteria>
    - `cd core && cargo build -p archipelago` exits 0.
    - `grep -v '^ *//' core/archipelago/src/server.rs | grep -c 'federation::sync_with_peer'` equals 1 (comment lines stripped so a doc comment cannot satisfy the gate).
    - `grep -v '^ *//' core/archipelago/src/server.rs | grep -c 'refresh_federation_mesh_peers'` equals 1.
    - `grep -c 'from_secs(1800)' core/archipelago/src/server.rs` equals 0 <!-- planner-discipline-allow: from_secs(1800) -->
    - `cd core && cargo test -p archipelago` exits 0.
    - The SUMMARY contains the `git log -p -L` finding answering RESEARCH.md Open Question 1, and states explicitly whether the loop was deleted or kept.
  </acceptance_criteria>
  <done>One periodic federation sync loop remains, its predecessor's unique behavior preserved, with the history check recorded.</done>
</task>

<task type="auto">
  <name>Task 3: Stop out-of-order snapshots and duplicates from breaking convergence</name>
  <files>core/archipelago/src/federation/storage.rs, core/archipelago/src/federation/sync.rs</files>
  <read_first>
    - `core/archipelago/src/federation/storage.rs` `update_node_state` (from L292 pre-01-01) — it
      currently overwrites `last_seen`, `name`, `fips_npub`, and `last_state` unconditionally from
      whatever snapshot arrives, with no comparison against what is already stored.
    - `core/archipelago/src/federation/types.rs` — `NodeStateSnapshot.timestamp` is an RFC 3339
      string; note that a lexicographic compare is only safe for same-offset RFC 3339, so parse it.
    - `core/archipelago/src/federation/storage.rs` — `dedup_nodes_by_onion` and its two existing
      tests, for the convergence behavior already present.
    - `core/archipelago/src/federation/sync.rs` — `merge_transitive_peers` (from L120) and its
      tombstone check, to confirm the guard added here does not conflict with it.
  </read_first>
  <action>
    Add a monotonicity guard to `update_node_state`: parse the incoming snapshot's timestamp and the
    stored `last_state`'s timestamp with `chrono::DateTime::parse_from_rfc3339`; if the incoming one
    is strictly older, return Ok without mutating the node — a slow sync response that lands after a
    newer one must not move the peer's status backwards. When either timestamp fails to parse, fall
    back to the current accept-newest behavior so a peer with a malformed clock is not frozen out,
    and log at `debug!`. Learning a peer's `fips_npub` is exempt: a stale snapshot may still carry
    the only copy of a FIPS key this node has, so apply that one field even on a rejected snapshot,
    and say so in a comment.

    Add tests: a strictly-older snapshot leaves `last_state` and `last_seen` unchanged; an equal
    timestamp is accepted (idempotent re-sync); a newer snapshot is accepted; a stale snapshot
    carrying a `fips_npub` this node lacks still populates it; an unparseable timestamp is accepted.

    Add a convergence test asserting that repeatedly applying the same peer's snapshot plus a
    transitive-peer merge does not grow the node list — one entry per federated node after N cycles.
  </action>
  <verify>
    <automated>cd core &amp;&amp; cargo test -p archipelago federation</automated>
  </verify>
  <acceptance_criteria>
    - `cd core && cargo test -p archipelago federation` exits 0 with the five snapshot-ordering cases and the convergence case present by name.
    - `grep -c 'parse_from_rfc3339' core/archipelago/src/federation/storage.rs` is at least 1.
    - `cd core && cargo test -p archipelago` exits 0.
    - `cd core && cargo clippy -p archipelago --all-targets` produces no new warnings in `federation`.
  </acceptance_criteria>
  <done>Out-of-order sync responses cannot regress a peer's state, and repeated sync cycles converge to one entry per node.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| federated peer → `sync_with_peer` → node record | A remote peer's snapshot and its timestamp drive local persisted state |
| daemon → operator dashboard | A sync error string crosses from the daemon into rendered UI |
| background loop → federation node store | The surviving periodic loop is now a writer of error state, not only a reader |

## STRIDE Threat Register

| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|-----------|----------|-----------|----------|-------------|-----------------|
| T-01-16 | Tampering | a peer replaying an old snapshot to roll a node's status backwards | high | mitigate | The `update_node_state` monotonicity guard rejects strictly-older snapshots (Task 3), with tests |
| T-01-17 | Information Disclosure | a sync error string carrying a peer onion address or transport credential into the UI | medium | mitigate | The recorded message is the error's display string truncated to 256 characters and rendered only on the operator's own dashboard; the prohibition above states the constraint and it is re-checked in the FED-03 review |
| T-01-18 | Denial of Service | an unbounded error message bloating `nodes.json` on every failed pass | medium | mitigate | 256-character truncation before persistence (Task 1) |
| T-01-19 | Repudiation | a silently-failing sync leaving no record of when a peer was last reachable | high | mitigate | `last_sync_error_at` is written on every attempt outcome; the badge makes staleness visible |
| T-01-20 | Denial of Service | deleting the 1800s loop dropping a behavior the fleet depends on | high | mitigate | Mandatory `git log -p -L` history check before deletion, the unique tail call moved rather than dropped, and an explicit STOP path if the history shows a documented reason |
</threat_model>

<verification>
- `cd core && cargo test -p archipelago` — green.
- `cd neode-ui && npx vitest run && npm run build` — green.
- The SUMMARY answers RESEARCH.md Open Question 1 with git evidence.
</verification>

<success_criteria>
- A sync failure is persisted, exposed over RPC, and rendered as an operator-visible badge that clears on recovery.
- Exactly one periodic federation sync loop remains, with the deleted loop's unique behavior preserved.
- Out-of-order snapshots cannot regress a peer's state; repeated cycles converge to one entry per node.
- A zero-node sync pass writes nothing and records no error.
</success_criteria>

<output>
Create `.planning/phases/01-federation-mesh-hardening/01-05-SUMMARY.md` when done.
Stage by explicit path, commit, and `git push gitea-ai main`.
</output>