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

phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves

phase	plan	type	wave	depends_on	files_modified	autonomous	requirements	must_haves
01-federation-mesh-hardening	01	execute	1		core/archipelago/src/federation/storage.rs	true	FED-01	truths prohibitions artifacts key_links A federation removal issued while an auto-sync pass is in flight leaves the peer removed — a sync's pre-removal node-list snapshot can no longer re-save the removed peer (FED-01 adjacency edge) Two concurrent federation node writes both persist — neither silently loses the other's update (the lost-update class behind the reported 'removed nodes reappear' symptom) Removing the last remaining federated node succeeds, leaves an empty node list, and returns Ok with an empty Vec (FED-01 empty edge) A removal whose tombstone write fails returns Err to the caller instead of reporting success (FED-01 failure-surfacing) The tombstone is durably written before the filtered node list is saved, so an interruption between the two never resurrects the removed peer (FED-01 ordering edge) A partially-written federation node list can never be observed by a concurrent reader — the list is written to a sibling temp file and renamed into place statement category Removing a federation node MUST NOT delete or destroy that peer's local data — no app data directory under /var/lib/archipelago, no mesh message history, no credential store is erased by unfederating; removal revokes trust, it never destroys operator data safety path provides contains core/archipelago/src/federation/storage.rs Serialized, crash-safe federation node store FEDERATION_STORE_LOCK from to via pattern core/archipelago/src/federation/storage.rs core/archipelago/src/federation/sync.rs update_node_state acquires FEDERATION_STORE_LOCK for its whole load-mutate-save cycle, so a sync pass cannot interleave with remove_node FEDERATION_STORE_LOCK

Close the concurrency race that lets a removed federation node come back: serialize every read-modify-write against `federation/nodes.json` behind one async lock, and make the node-list write atomic.

Purpose: FED-01 — "removing a federation node sticks" is the reason this phase exists. RESEARCH.md identifies an unlocked read-modify-write on federation/nodes.json as the primary suspect: the 90s auto-sync loop, the 1800s auto-sync loop, federation.sync-state, and federation.remove-node all load → mutate → save the same file with zero coordination, so a sync task holding a pre-removal snapshot silently re-saves the peer the operator just removed — with no error logged anywhere. Output: federation/storage.rs with a module-level FEDERATION_STORE_LOCK, inner/outer function split to avoid re-entrancy deadlock, an atomic temp-file+rename node-list write, and three new regression tests that fail without the lock.

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

@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/01-federation-mesh-hardening/01-RESEARCH.md @.planning/phases/01-federation-mesh-hardening/01-PATTERNS.md @core/archipelago/src/federation/storage.rs @core/archipelago/src/update.rs

Artifacts this phase produces

Created or changed by this plan:

Symbol	Kind	File
FEDERATION_STORE_LOCK	static tokio::sync::Mutex<()>	core/archipelago/src/federation/storage.rs
save_nodes_inner	private async fn (no lock; atomic temp+rename write)	same
load_nodes_inner	private async fn (no lock)	same
tombstone_did_inner / untombstone_did_inner	private async fns (no lock)	same
test_concurrent_writes_do_not_lose_updates	#[tokio::test]	same (mod tests)
test_remove_survives_concurrent_state_sync	#[tokio::test]	same (mod tests)
test_remove_last_node_leaves_empty_list	#[tokio::test]	same (mod tests)
test_remove_errors_when_tombstone_write_fails	#[tokio::test]	same (mod tests)

Public function signatures of load_nodes, save_nodes, add_node, remove_node, set_trust_level, update_node, update_node_state, record_peer_transport, tombstone_did, untombstone_did, load_removed_dids are unchanged — callers in sync.rs, handlers.rs, server.rs, and mesh/mod.rs compile untouched.

Task 1: End-to-end — a removal survives an in-flight sync (lock + atomic write) A module-private lock and a temp+rename write are internal to storage.rs behind unchanged public signatures; reverting is a single-file change with no on-disk format change. core/archipelago/src/federation/storage.rs - `core/archipelago/src/federation/storage.rs` — the whole file (532 lines). Note in particular: `load_nodes` (L51), `record_peer_transport` (L120), `save_nodes` (L149), `add_node` (L161, calls `untombstone_did`), `remove_node` (L180, calls `tombstone_did`), `tombstone_did` (L214), `untombstone_did` (L237), `set_trust_level` (L256), `update_node` (L272), `update_node_state` (L292), and the existing `#[cfg(test)] mod tests` (L341) with its `make_node(did, onion)` helper and `tempfile::tempdir()` convention. - `core/archipelago/src/update.rs` lines 25-40 — `UPDATE_OP_LOCK`, the in-repo precedent for "two async call sites race on one on-disk resource". Copy its doc-comment style (name the concrete historical incident, then state the acquisition policy). - `.planning/phases/01-federation-mesh-hardening/01-PATTERNS.md` — section "Async static lock for a racy on-disk resource" and "core/archipelago/src/federation/storage.rs (locking fix)". - `test_concurrent_writes_do_not_lose_updates`: under `#[tokio::test(flavor = "multi_thread", worker_threads = 4)]`, seed one node, then `tokio::join!` an `add_node(B)` with a `set_trust_level(A, Observer)`; afterwards `load_nodes` returns 2 nodes AND node A's trust level is Observer. Without the lock one of the two writes is lost. - `test_remove_survives_concurrent_state_sync`: under the multi-thread flavor, loop 50 times: fresh tempdir, seed nodes A and B, `tokio::join!(remove_node(A), update_node_state(A, snapshot))`, then assert `load_nodes` contains no entry whose `did` is A and `load_removed_dids` contains A. Without the lock this reliably fails within 50 iterations. - `test_remove_last_node_leaves_empty_list`: seed exactly one node, remove it, assert the returned Vec is empty, `load_nodes` returns an empty Vec (not an error), and the DID is tombstoned. Write the three tests FIRST in the existing `mod tests` block and confirm they fail (run the verify command and capture the failure) before writing the fix.

Then add at module scope, immediately after the existing `use` block:
`static FEDERATION_STORE_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());`
with a doc comment that names the failure it prevents — a `federation.remove-node` RPC racing
the 90s auto-sync pass, whose pre-removal `load_nodes` snapshot re-saves the removed peer with
no error logged — and states the acquisition policy: acquire with `.lock().await`, never
`try_lock`. Reject-on-contention is wrong here: a rejected federation write reproduces the very
lost-write symptom the lock exists to stop, unlike `update.rs` where rejecting a second
concurrent download is the desired UX.

Restructure so no public function can deadlock on itself. For each function that both takes the
lock and calls another locking function, extract the body into a private `*_inner` fn that does
NOT acquire, and make the public fn a thin wrapper: acquire the guard, call the inner(s), drop.
Required inner fns for this task: `load_nodes_inner`, `save_nodes_inner`, `tombstone_did_inner`,
`untombstone_did_inner`. `remove_node` (which calls `tombstone_did`) and `add_node` (which calls
`untombstone_did`) must call the `*_inner` variants under a single held guard so tombstone +
node-list save are one critical section.

Convert the node-list write in `save_nodes_inner` to atomic replace: serialize to a sibling path
formed by appending a `.tmp` suffix to the resolved nodes file path in the same directory, write
it with `tokio::fs::write`, then `tokio::fs::rename` it onto the real path. Keep the existing
`.context(...)` error strings so callers' messages are unchanged. Same-directory rename is
required — a cross-filesystem rename is not atomic.

In THIS task route `load_nodes`, `save_nodes`, `remove_node`, `tombstone_did`,
`untombstone_did`, and `update_node_state` through the lock. The remaining mutators are Task 2.

Preserve `remove_node`'s existing ordering exactly: the retain/`bail!`-on-not-found check, then
the tombstone write with its failure propagated via `.context("persist removal tombstone")?`,
then the node-list save. Do not weaken that ordering or its error propagation.

Build gotcha from CLAUDE.md: if the build hits `rust-lld: undefined hidden symbol`, that is
incremental-cache corruption — re-run with `CARGO_INCREMENTAL=0`.

cd core && cargo test -p archipelago federation::storage - `cd core && cargo test -p archipelago federation::storage` exits 0. - `grep -c 'FEDERATION_STORE_LOCK' core/archipelago/src/federation/storage.rs` is at least 7. - `grep -c 'tokio::sync::Mutex::const_new' core/archipelago/src/federation/storage.rs` equals 1. - `grep -c 'fs::rename' core/archipelago/src/federation/storage.rs` is at least 1. - `grep -Eq 'async fn test_remove_survives_concurrent_state_sync' core/archipelago/src/federation/storage.rs` succeeds. - `grep -Eq 'async fn test_concurrent_writes_do_not_lose_updates' core/archipelago/src/federation/storage.rs` succeeds. - `grep -Eq 'async fn test_remove_last_node_leaves_empty_list' core/archipelago/src/federation/storage.rs` succeeds. - `grep -Eq 'flavor = "multi_thread"' core/archipelago/src/federation/storage.rs` succeeds (the race tests are useless on the single-threaded default runtime). - The SUMMARY records the captured pre-fix failure output for at least one of the three tests. The removal-vs-sync race is closed at the storage layer and proven by a test that fails without the lock; the node list is written atomically. Task 2: Bring every remaining federation mutator under the lock + surface tombstone-write failure core/archipelago/src/federation/storage.rs - `core/archipelago/src/federation/storage.rs` as left by Task 1 — specifically the four mutators not yet routed through the lock: `record_peer_transport` (L120 pre-change), `add_node`, `set_trust_level`, `update_node`. - The existing test `test_remove_nonexistent_node_errors` (L445 pre-change) — mirror its assertion style for the new failure test. Route `add_node`, `set_trust_level`, `update_node`, and `record_peer_transport` through `FEDERATION_STORE_LOCK` using the same wrapper + `*_inner` split established in Task 1. Every public function in this module that performs a load → mutate → save cycle must hold the guard for the whole cycle; none may call another lock-acquiring public function while holding it.

Add `test_remove_errors_when_tombstone_write_fails`: seed a node, then make the tombstone write
fail by pre-creating the removed-nodes path as a directory (a directory cannot be replaced by a
file write), call `remove_node`, and assert the result is `Err` AND that `load_nodes` still
contains the node — a removal whose tombstone never landed must not have half-applied. This is
the FED-01 "a failed removal surfaces an error instead of silently no-opping" criterion at the
storage layer.

Do not change any public signature and do not touch `load_invites`/`save_invites` (a separate
file with no cross-writer).

cd core && cargo test -p archipelago federation - `cd core && cargo test -p archipelago federation` exits 0. - `grep -Eq 'async fn test_remove_errors_when_tombstone_write_fails' core/archipelago/src/federation/storage.rs` succeeds. - `grep -c 'FEDERATION_STORE_LOCK.lock().await' core/archipelago/src/federation/storage.rs` is at least 9. - `cd core && cargo build -p archipelago` exits 0 with no new warnings in `federation::storage` (dead-code warnings on unused `*_inner` fns mean a mutator was missed). - `cd core && cargo test -p archipelago` exits 0 — no caller in `sync.rs`, `handlers.rs`, `server.rs`, or `mesh/mod.rs` was broken by the refactor. Every federation node-store mutator is serialized; a tombstone-write failure is proven to surface as an error with no half-applied removal.

<threat_model>

Trust Boundaries

Boundary	Description
federated peer → federation::sync → node store	A remote peer's state snapshot crosses into local persisted trust state
operator RPC (federation.remove-node) → node store	An authenticated local operator action mutates trust membership
process → federation/nodes.json on disk	Multiple concurrent async tasks write one file; a crash can leave it partial

STRIDE Threat Register

Threat ID	Category	Component	Severity	Disposition	Mitigation Plan
T-01-01	Tampering	federation::storage concurrent read-modify-write	high	mitigate	FEDERATION_STORE_LOCK held across the whole load-mutate-save cycle in every mutator (Tasks 1-2); regression test proves a removal survives a concurrent sync
T-01-02	Elevation of Privilege	a removed (untrusted) peer regaining federation membership via the race	high	mitigate	Same lock; plus the pre-existing tombstone check in merge_transitive_peers and handle_federation_peer_joined is left intact and re-verified by cargo test -p archipelago federation
T-01-03	Denial of Service	a partial nodes.json write on crash making the node list unreadable	medium	mitigate	Atomic temp-file + same-directory fs::rename in save_nodes_inner (Task 1)
T-01-04	Denial of Service	lock contention stalling the federation RPC surface	low	accept	Federation writes are infrequent (90s loop + operator actions); .lock().await queues rather than rejects, and every critical section is a bounded file read+write
</threat_model>

- `cd core && cargo test -p archipelago federation` — green. - `cd core && cargo test -p archipelago` — green (no caller regressions). - The pre-fix failure of `test_remove_survives_concurrent_state_sync` is recorded in the SUMMARY as evidence the test is fail-first and not vacuous.

<success_criteria>

• Every read-modify-write in federation/storage.rs is serialized behind one module-level async mutex with no re-entrancy path.
• The node list is written atomically (temp file + same-directory rename).
• Four new tests exist and pass; at least one is demonstrated to fail without the lock.
• Public signatures unchanged; the full archipelago test suite is green. </success_criteria>

Create `.planning/phases/01-federation-mesh-hardening/01-01-SUMMARY.md` when done. Commit with `git add` by explicit path (another agent shares this tree — never `git add -A`), then push per CLAUDE.md: `git push gitea-ai main`.