fix(auth): serialize token refresh and make config/session writes atomic

[eddietejeda]

Problem

Concurrent hotdata invocations race on the shared state in ~/.hotdata. Found while load-testing databases create with N parallel processes:

1. Torn config writes — every databases create calls save_current_database, an unlocked read-modify-write of config.yml that ends in a truncating fs::write. Parallel commands read half-written YAML (error parsing config file., exit 1) and silently drop each other's entries.
2. Refresh-token stampede — when the 5-minute access token expires mid-burst, every process spends the same refresh token against /o/token/. The server treats reuse of a rotated refresh token as compromise and revokes the session family; the losing processes then call clear_session(), deleting the winner's freshly saved session. Escalation observed live: 261 session expired or revoked errors at 32-way concurrency, then total session death — a 128-way burst went 0 for 35,146, and the login session was permanently revoked (re-auth required).

Fix (commit 1)

• Atomic writes: config.yml, session.json, and database_session.json are written via tempfile-in-same-dir + rename, so readers never observe a partial file.
• Config lock: a blocking exclusive flock (~/.hotdata/config.lock) serializes config read-modify-writes.
• Serialized refresh: ensure_access_token's refresh/re-mint slow path runs under ~/.hotdata/session.lock, re-checking the cached session after acquiring the lock — waiters reuse the winner's fresh token instead of spending the refresh token again. The valid-cache fast path stays lock-free.

Review hardening (commit 2)

An 8-angle adversarial review (plus an independent Codex CLI pass) of commit 1 surfaced these, all applied:

• Timeout on token calls — the mint/refresh endpoints used bare reqwest::blocking::Client::new() (no timeout). With refresh now holding session.lock, a stalled /o/token/ would hang the lock holder and, with it, every concurrent invocation. They now use raw_http::build_http_client() (300s cap + keepalive), which the raw_http module doc already designated for exactly these endpoints.
• Typed RefreshError — Rejected (server refused: clear session, fall through to re-mint, as before) vs Transport (request never completed: the refresh token was not consumed, so keep the session and surface the real connection error instead of forcing a re-login on a flaky network).
• Persistence failure is visible — a failed save_session after a successful refresh now warns on stderr instead of let _ =; an unpersisted rotation sends the next process back to the spent refresh token.
• Lock failures are visible — lock_file warns on unexpected errors (perms, I/O) instead of silently degrading; flock-unsupported filesystems (ENOTSUP/ENOLCK) still degrade silently to the pre-lock behavior.
• Locked update_config primitive — all five config read-modify-write functions route through one locked helper, so future mutations can't forget the lock.
• Shared util::atomic_write(path, bytes, mode) — replaces three copies of the tempfile+rename block; file modes are now explicit (0600 for both session files, 0644 for config.yml, preserving its previous mode).

Known accepted behavior: rename-based writes replace a symlinked destination with a regular file (relevant only if ~/.hotdata files are managed by a dotfile symlinker).

Tests

• concurrent_saves_keep_all_entries_and_parse_cleanly — 8 threads doing config read-modify-write; all entries must survive and parse.
• ensure_concurrent_expiry_refreshes_once_and_shares_result — 8 threads hitting an expired session must produce exactly one refresh request (mockito expect(1)) and all share its token.
• ensure_keeps_session_when_refresh_fails_at_transport_level — connection failure keeps the session and surfaces the real error.
• config_file_stays_world_readable — config.yml stays 0644 through the atomic-write path.
• Full suite: 304 passed, 0 failed; fmt/clippy clean for touched files.

Verification against production

60-second create bursts, before → after (fixed binary, 24,006 creates total, 5 errors ≈ 0.02%):

Concurrency	Broken binary	Fixed binary
32	3,141 created, 274 errors	5,048 created, 1 error
64	3,667 created, 12 errors	5,950 created, 0 errors
128	0 created, 35,146 errors (session revoked)	6,292 created, 2 errors
256	not reached	6,716 created, 2 errors

Post-review commit re-smoked with a 32-wide parallel create burst: 32/32 succeeded, cleanup clean.

The handful of remaining errors print an empty message — a separate CLI error-reporting issue, tracked as a follow-up. Also follow-up: auth.rs/credentials.rs still use un-timeouted Client::new() (not lock-holding paths, so out of scope here).

[claude]

super nit: NamedTempFile cleans up on Drop, but if a process is killed between new_in and persist, the temp file leaks in ~/.hotdata. Under the exact heavy-concurrency-with-kills scenario this PR targets (128-way bursts, some processes terminated), these .tmpXXXXXX files could slowly accumulate. Not a correctness problem — config load reads config.yml directly and ignores them — just orphaned files worth being aware of. (not blocking)

[claude]

Atomic write-then-rename and flock serialization are implemented correctly; the refresh slow path re-checks the cached session after acquiring the lock, and both regression tests cover the documented races. One super nit inline, non-blocking.

[codecov]

Codecov Report

❌ Patch coverage is 93.27731% with 16 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/config.rs	89.47%	12 Missing ⚠️
src/client/jwt.rs	96.29%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[claude]

nit: any non-2xx here is classified as Rejected, which makes ensure_access_token call clear_session() and fall through to re-mint. That's correct for 400 invalid_grant (the session really is dead), but it also fires for transient server failures — 429 Too Many Requests, 500, 502/503. Under the exact 128-way burst this PR targets, an overloaded token endpoint returning 503/429 is plausible, and none of those consume/rotate the refresh token — the session is still good. Since refresh is now serialized under session.lock, clearing on a transient 5xx/429 needlessly kills the session for the whole burst (forced re-login for interactive sessions with no api_key fallback), which partly undercuts the goal here.

Worth treating 5xx/429 like the Transport case (keep the session, surface the error) and only clearing on a genuine 4xx invalid_grant. Note this is still strictly better than the pre-PR Err(_) => clear_session(), so not blocking. (not blocking)

[claude]

Locking + atomic-write approach is sound and well-tested (concurrent config saves, single-refresh regression, transport-vs-rejected split). One non-blocking nit on 5xx/429 error classification left inline.

[claude]

Reviewed the concurrency fix end to end: atomic write/chmod/rename ordering, the single locked update_config primitive, the lock-free fast path + re-check under session.lock, the Transport/Rejected split, and the timeout-bounded token client. Lock ordering is clean (no nesting, flock auto-released on death) and the tests cover the races, transport-retention, persist-failure, and lock-degrade paths. LGTM.