docs(01-01): complete federation node-store lock plan

.planning/ROADMAP.md

@@ -53,13 +53,13 @@ signed/decentralized registry and a user installs it on their node.
   13. Genuinely slow opens show loader states (UIFIX-06, added 2026-07-30 — see `.planning/todos/pending/2026-07-30-missing-loader-states-on-slow-opens.md`; 02-08's flagged timing regressions are the starting inventory)
   NOTE for criteria 7–13: all were added after phase 1's 10 plans were written — before phase 1 execution completes, create gap plan(s) covering FED-07 + UIFIX-01..06 (existing desktop visuals must remain untouched per the standing visual-invisibility rule; UIFIX items themselves are user-approved visual changes)
 
-**Plans**: 18 plans
+**Plans**: 2/20 plans executed
 
 Plans:
 
 - [ ] 01-20-PLAN.md — URGENT wave 1: doctor stops restarting Tor every 5min (mesh Tor fallback) (FED-09)
 - [ ] 01-19-PLAN.md — URGENT wave 1: wallet invoices embed route hints so private-channel nodes can receive (FED-08)
-- [ ] 01-01-PLAN.md — Serialize the federation node store and make removal stick (FED-01)
+- [x] 01-01-PLAN.md — Serialize the federation node store and make removal stick (FED-01)
 - [ ] 01-02-PLAN.md — Demo mesh/federation RPC parity + automated parity harness (FED-04)
 - [ ] 01-03-PLAN.md — On-brand paid tick: ScreensaverRing badge variant on both success surfaces (FED-06)
 - [ ] 01-04-PLAN.md — Lightning identity: own-node URI + meshed Lightning peer discovery (FED-05)
@@ -73,7 +73,7 @@ Plans:
 **Wave 7** *(gap closure — criteria 7–13, added 2026-07-30 after the original 10 plans were written)*
 
 - [ ] 01-11-PLAN.md — No baked-in Fedimint gateway credential: per-install secret on every path (FED-07)
-- [ ] 01-12-PLAN.md — Connected-nodes list scrolls at row-matched height instead of growing (UIFIX-02)
+- [x] 01-12-PLAN.md — Connected-nodes list scrolls at row-matched height instead of growing (UIFIX-02)
 - [ ] 01-13-PLAN.md — On-brand scroll cue makes the onboarding tickbox findable on short screens (UIFIX-03)
 - [ ] 01-14-PLAN.md — Paid Files open in the app lightbox, with a visible wait and a real error path (UIFIX-04/06)
 - [ ] 01-15-PLAN.md — PiP hands off from the lightbox and survives tab changes and buffering (UIFIX-05)
@@ -235,7 +235,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8
 
 | Phase | Plans Complete | Status | Completed |
 |-------|----------------|--------|-----------|
-| 1. Federation & Mesh Hardening | 0/10 | Planned | - |
+| 1. Federation & Mesh Hardening | 2/20 | In Progress|  |
 | 2. UI Performance | 11/12 | Complete    | 2026-07-31 |
 | 3. Multinode Verification Pass | 0/TBD | Not started | - |
 | 4. Lifecycle Perfection & Quadlet Default | 0/TBD | Not started | - |

.planning/STATE.md

@@ -5,15 +5,15 @@ milestone_name: milestone
 current_phase: 09
 current_phase_name: BotFights Platform Upgrade
 status: planning
-stopped_at: Completed 02-11-PLAN.md — Phase 02 (ui-performance) final gap closure complete
-last_updated: "2026-07-31T15:58:19.908Z"
+stopped_at: Completed 01-01-PLAN.md (federation store lock, FED-01)
+last_updated: "2026-08-01T02:29:19.595Z"
 last_activity: 2026-07-31
 last_activity_desc: Phase 02 complete, transitioned to Phase 09
 progress:
   total_phases: 9
   completed_phases: 1
   total_plans: 39
-  completed_plans: 18
+  completed_plans: 20
   percent: 11
 ---
 
@@ -33,7 +33,7 @@ Plan: Not started
 Status: Ready to plan
 Last activity: 2026-07-31 — Phase 02 complete, transitioned to Phase 09
 
-Progress: [███████░░░] 67%
+Progress: [█████░░░░░] 51%
 
 ## Performance Metrics
 
@@ -63,6 +63,7 @@ Progress: [███████░░░] 67%
 | Phase 02 P09 | 130min | 3 tasks | 3 files |
 | Phase 02 P10 | 55min | 2 tasks | 3 files |
 | Phase 02 P11 | ~150min | 3 tasks | 8 files |
+| Phase 01 P01 | n/a-continuation | 2 tasks | 1 files |
 
 ## Accumulated Context
 
@@ -118,6 +119,7 @@ Decisions are logged in PROJECT.md (10 locked ADRs in the `<decisions>` block +
 - [Phase 2, gap closure 02-11]: Real cause of the six regressions was NOT compute-bound render cost (CPU profile: 86-99% idle/program, <10% JS self-time everywhere) — it was three background pollers (useFleetData.ts 60s, FipsNetworkCard.vue 15s, Web5Monitoring.vue 30s) armed in onMounted and never disarmed once their owning views joined KEEP_ALIVE_PATHS in 02-04, invisible to that audit because it grepped the top-level view files, not the child composables they delegate to. Gated to onActivated/onDeactivated, mirroring 02-04's own established pattern. Fixed: web5 275ms (was 566ms baseline/1329ms regressed), server 574ms (was 738/1239), fleet 790ms (was 330/2631)
 - [Phase 2, gap closure 02-11]: Discover (1389ms, worst remaining) has a SECOND, distinct cause: card-stagger/showStagger entrance-animation classes are baked into the DOM at first mount and never programmatically removed, so every KeepAlive detach/reattach cycle restarts the CSS animation on reactivation — replaying the full entrance cascade on every revisit. Confirmed via a diagnostic (DOM card count doubling transiently on every revisit) and an extended animation-event log. NOT fixed — blast radius spans 5+ files outside 02-11's scope (Apps.vue, Marketplace.vue, Home.vue, several Web5 sub-cards), needs its own real-device verification budget; recommended as a dedicated follow-up
 - [Phase 2, gap closure 02-11]: openwrt-gateway unmeasurable in the final re-measure (Chromium "Target crashed" cascading from an unrelated surface, cloud-folder, earlier in the same harness run) — recorded as not-measurable, not written in as data. Separately confirmed the prior baseline/after/remeasure numbers were measuring a real, substantive disconnected-state UI (OpenWrtGateway.vue's h1 is unconditional; a "No router configured" RPC error deterministically renders a real Connect-to-Router form, not a blank/error page) — the six-surface regression count is not retracted, but the numbers reflect one specific code branch (no OpenWrt device has ever been connected to archi-dev-box)
+- [Phase ?]: 01-01: record_peer_transport and update_node routed through FEDERATION_STORE_LOCK via *_inner; tombstone-write-failure test added; full-suite verify blocked by a concurrent agent's uncommitted install.rs edit (unrelated file, not fixed per scope boundary)
 
 ### Pending Todos
 
@@ -161,6 +163,6 @@ Decisions are logged in PROJECT.md (10 locked ADRs in the `<decisions>` block +
 
 ## Session Continuity
 
-Last session: 2026-07-31T13:45:00.000Z
-Stopped at: Completed 02-11-PLAN.md — Phase 02 (ui-performance) final gap closure complete
+Last session: 2026-08-01T02:28:42.155Z
+Stopped at: Completed 01-01-PLAN.md (federation store lock, FED-01)
 Resume file: None

.planning/phases/01-federation-mesh-hardening/01-01-SUMMARY.md

@@ -0,0 +1,142 @@
+---
+phase: 01-federation-mesh-hardening
+plan: 01
+subsystem: federation-storage
+tags: [federation, concurrency, tokio-mutex, atomic-write, FED-01]
+status: complete
+dependency-graph:
+  requires: []
+  provides:
+    - FEDERATION_STORE_LOCK
+    - atomic nodes.json write (temp+rename)
+  affects:
+    - core/archipelago/src/federation/sync.rs (caller, unchanged signatures)
+    - core/archipelago/src/api/rpc/federation/handlers.rs (caller, unchanged signatures)
+tech-stack:
+  added:
+    - tokio::sync::Mutex (module-level static, const_new)
+  patterns:
+    - "Async static lock for a racy on-disk resource (mirrors update.rs's UPDATE_OP_LOCK, but .lock().await not try_lock, since federation writes must queue not reject)"
+    - "Public fn = thin lock wrapper; private *_inner fn = lock-free body, so multi-step critical sections (tombstone + node-list save) don't self-deadlock on a non-reentrant Mutex"
+key-files:
+  created: []
+  modified:
+    - core/archipelago/src/federation/storage.rs
+decisions:
+  - "set_trust_level pulled forward into Task 1's commit (2f99db5e) rather than Task 2, because test_concurrent_writes_do_not_lose_updates (a Task 1 required-green test) races add_node against set_trust_level and needs it locked to pass"
+  - "record_peer_transport and update_node route through *_inner under a single held guard rather than calling the public load_nodes/save_nodes, closing the same unlocked-two-call race the whole plan exists to fix"
+  - "Tombstone-write failure test (test_remove_errors_when_tombstone_write_fails) forces failure by pre-creating removed-nodes.json as a directory, not by mocking I/O — no existing I/O mocking harness in this module, and this is the simplest deterministic failure induction available"
+metrics:
+  duration: "resumed/completed in this session; Task 1 was previously committed 2026-07-30"
+  completed: 2026-07-31
+---
+
+# Phase 01 Plan 01: Serialize the federation node store and make removal stick (FED-01) Summary
+
+Closed the concurrency race that let a removed federation node reappear: every load-mutate-save
+cycle in `federation/storage.rs` is now serialized behind one module-level `FEDERATION_STORE_LOCK`,
+the node-list write is atomic (temp file + same-directory rename), and a tombstone-write failure
+now surfaces as an `Err` instead of a silent no-op.
+
+## What Was Built
+
+- `FEDERATION_STORE_LOCK: tokio::sync::Mutex<()>` (module-level static, `const_new`), documented
+  with the concrete failure it prevents (a `federation.remove-node` RPC racing the 90s auto-sync
+  loop's stale pre-removal snapshot).
+- `load_nodes_inner` / `save_nodes_inner` / `tombstone_did_inner` / `untombstone_did_inner`:
+  lock-free bodies so `remove_node` (tombstone + save) and `add_node` (untombstone + save) can
+  each hold the guard across their whole multi-step critical section without self-deadlocking
+  (`tokio::sync::Mutex` is not re-entrant).
+- Every public mutator now routes through the lock: `load_nodes`, `save_nodes`, `add_node`,
+  `remove_node`, `tombstone_did`, `untombstone_did`, `set_trust_level`, `update_node`,
+  `update_node_state`, `record_peer_transport` — 10 `.lock().await` call sites total.
+- `save_nodes_inner` writes atomically: serialize to `nodes.json.tmp` in the same directory,
+  then `fs::rename` onto `nodes.json` — a crash mid-write can never leave a partial file for a
+  concurrent reader.
+- Four new regression tests in `federation::storage::tests`:
+  - `test_concurrent_writes_do_not_lose_updates` — races `add_node` against `set_trust_level`
+    via real `tokio::spawn` tasks, 40 iterations; asserts both writes persist.
+  - `test_remove_survives_concurrent_state_sync` — races `remove_node` against a 12-task burst
+    of `update_node_state` calls, 30 iterations; asserts the removed DID stays removed and
+    tombstoned.
+  - `test_remove_last_node_leaves_empty_list` — removing the sole federated node returns/loads
+    an empty `Vec`, not an error.
+  - `test_remove_errors_when_tombstone_write_fails` — pre-creates `removed-nodes.json` as a
+    directory so the tombstone write fails; asserts `remove_node` returns `Err` AND the node
+    list is untouched (no half-applied removal).
+
+## Task Execution Note (continuation)
+
+Task 1 (the tracer: lock + atomic write + first three tests) was already committed in a prior
+session (`2f99db5e`, 2026-07-30) and pushed. This session picked up as a continuation: verified
+Task 1's commit and tests were real and green, then completed Task 2 — routing
+`record_peer_transport` and `update_node` through the lock (they still called the public,
+separately-locked `load_nodes`/`save_nodes` instead of the `*_inner` pair under one guard) and
+adding the tombstone-failure test. No SUMMARY/STATE/ROADMAP update had been done for this plan
+before this session; that gap is closed by this document.
+
+## Pre-fix Failure Evidence (Task 1, historical)
+
+Task 1's commit message (`2f99db5e`) records that both `test_concurrent_writes_do_not_lose_updates`
+and `test_remove_survives_concurrent_state_sync` were proven fail-first before the lock existed:
+using real `tokio::spawn` tasks (not just `tokio::join!`, since `remove_node`'s extra tombstone
+I/O hop structurally biases a simple 2-task race toward the safe ordering) reliably reproduced
+both the lost concurrent write and the removed-node-reappears bug pre-fix. This session did not
+re-run the pre-fix reproduction (the fix and lock already exist on disk); the historical evidence
+is carried forward from the Task 1 commit message since no separate SUMMARY captured it at the
+time.
+
+## Verification
+
+- `cd core && cargo test -p archipelago federation::storage` — **14/14 passed, 0 failed**
+  (11 pre-existing + 3 new from Task 1's earlier commit + this session's
+  `test_remove_errors_when_tombstone_write_fails`).
+- `cd core && cargo build -p archipelago` — succeeds, no new warnings in `federation::storage`
+  (no dead-code warnings on any `*_inner` fn — confirms every mutator is wired through).
+- Acceptance-criteria greps: `FEDERATION_STORE_LOCK` count 16, `.lock().await` count 10 (≥9
+  required), `tokio::sync::Mutex::const_new` count 1, `fs::rename` count 1, all four new test
+  function names present, `flavor = "multi_thread"` present.
+- **`cd core && cargo test -p archipelago` (full suite) — NOT clean this session.** The
+  workspace test binary fails to *compile*, but the failure is in
+  `core/archipelago/src/api/rpc/package/install.rs:592` (a tuple-pattern-vs-`Result` mismatch on
+  a `.await?` line marked "Not Committed Yet" by `git blame` at the time of this run) — a file
+  this plan never touches, mid-edit by a different, concurrent agent session in this shared
+  checkout (`git status` at commit time showed `install.rs`, `config.rs`, `dependencies.rs`,
+  `secrets.rs`, and several `neode-ui` files dirty, none authored by this plan). This is the
+  exact shared-tree hazard the task's hard constraints warn about, not a regression from this
+  plan's change. `federation::storage`'s own test binary (scoped `cargo test -p archipelago
+  federation::storage`) compiles and passes clean, and `cargo build -p archipelago` (non-test)
+  also succeeds — the compile error is specific to the test-cfg path in `install.rs`, unrelated
+  to `storage.rs`. Recorded honestly per the task's own instruction rather than declared green;
+  re-run `cargo test -p archipelago` once the other in-flight session's `install.rs` edit lands
+  or is reverted.
+
+## Deviations from Plan
+
+### Auto-fixed Issues
+
+**1. [Rule 3 - blocking issue, scope-bounded] Full-suite verification blocked by a concurrent
+agent's uncommitted edit in an unrelated file**
+- **Found during:** final verification step (`cargo test -p archipelago`)
+- **Issue:** `install.rs:592` fails to compile (tuple destructure not wrapped in `Ok(...)`
+  against a function that now returns `Result`), per `git blame` an uncommitted, in-progress
+  edit by a different session.
+- **Fix:** None applied — out of this plan's scope per the SCOPE BOUNDARY rule (issue not caused
+  by this plan's changes, and touching a file another agent is actively editing risks clobbering
+  their work). Logged here and left for that session to resolve.
+- **Files modified:** none (no fix applied)
+
+No other deviations — the rest of this plan (routing `record_peer_transport`/`update_node`
+through the lock, adding the failure test) executed exactly as written in Task 2's `<action>`.
+
+## Known Stubs
+
+None.
+
+## Self-Check: PASSED
+
+- `core/archipelago/src/federation/storage.rs` — FOUND (modified, contains `FEDERATION_STORE_LOCK`,
+  `record_peer_transport`, `update_node`, `test_remove_errors_when_tombstone_write_fails`).
+- Commit `4b5367eb` — FOUND in `git log --oneline`.
+- Commit `2f99db5e` (Task 1, prior session) — FOUND in `git log --oneline`.
+- Push to `gitea-ai main` — confirmed (`bc9a210c..4b5367eb main -> main`).