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
created
modified
core/archipelago/src/federation/storage.rs
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
duration
completed
resumed/completed in this session; Task 1 was previously committed 2026-07-30
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).
