diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index dc1ee8c9..dda2ca2c 100644 --- a/.planning/ROADMAP.md +++ b/.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 | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 11e9d809..ebfdc11a 100644 --- a/.planning/STATE.md +++ b/.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 `` 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 `` 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 diff --git a/.planning/phases/01-federation-mesh-hardening/01-01-SUMMARY.md b/.planning/phases/01-federation-mesh-hardening/01-01-SUMMARY.md new file mode 100644 index 00000000..ccf8169f --- /dev/null +++ b/.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 ``. + +## 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`).