fix(gateway): serialize hosted-rooms first-open of shared state.db

[jwilson411]

Summary

Every profile gateway starts the hosted-rooms worker against the install-root state.db, not the per-profile file. On a simultaneous multi-profile restart (hermes update fires the fleet back-to-back) those first opens overlapped inside PRAGMA journal_mode and left a file whose header was no longer a SQLite database (sqlite3.DatabaseError: file is not a database). BEGIN IMMEDIATE only covers the schema migration that runs after journal mode is applied.

This PR serializes _connect with a blocking advisory lock (state.db.hosted-rooms.lock, distinct from FTS / repair / auto-maintenance locks) and retries a transient file is not a database under that lock. A timed-out lock still opens (pre-fix behavior) rather than failing the worker.

Test plan

scripts/run_tests.sh tests/gateway/test_hosted_rooms_shared_db_lock.py tests/gateway/test_hosted_room_gateway_lifecycle.py

• Two racing first opens cannot scramble the SQLite header (deterministic overlap stand-in; sabotage of the lock fails the invariant).
• A held lock makes the second opener wait instead of skipping.
• Transient file is not a database retries; a persistently corrupt file still raises.

Platforms

Linux (fcntl flock; this box). Windows uses the existing msvcrt non-blocking lock loop. No new HERMES_* env vars. No updater rewrite.

Fixes #102120

[andrexibiza]

Review of exact head 0ada168c61d8eba70e32c4631db225f6ded64d61 against its recorded base 562ee8ab76a703b7f524172cae0b1d52f9f94bd3, with current main now at 63279301bcbdc185c1b07b98a9312eb0c862f26d (this head is 1 ahead / 2 behind current main).

This is a high-value fix target. #102120 is a P1 state-integrity report, the patch is narrow, and putting a cross-process fence around the shared install-root DB initialization is the right class of mechanism. I also like that the lock namespace is distinct from repair/FTS/maintenance locks, retries are bounded, and persistent file is not a database still surfaces rather than being silently laundered into success.

I would not merge this exact head yet. There are four concrete closure gates.

1. The safety invariant currently fails open on the exact path where the lock cannot be proven

_shared_db_open_lock() deliberately yields False both when the lock file cannot be opened and when acquisition times out. _connect() then ignores that boolean and calls _connect_locked(path) anyway.

That means the postcondition is not actually “hosted-room first opens are serialized”; it is “serialized unless the lock is unavailable for 20 seconds or cannot be opened, then perform the same journal-mode/schema mutation without serialization.” The latter is precisely the mutation this PR says can corrupt the shared DB under overlap.

For a state-integrity fence, inability to prove exclusive ownership cannot authorize the write. A wedged worker is recoverable; a corrupted shared state.db is not. Please make the mutation fail/defer/retry when the lock is indeterminate or unavailable instead of falling back to the pre-fix unsafe open. The supervised worker can retry a typed/ordinary failure; it should not cross the protected boundary without the fence.

Required regressions:

• hold the lock past the configured timeout and assert the second opener never enters _connect_locked / apply_wal_with_fallback;
• make the lock path unopenable and assert no DB mutation occurs;
• retain an allowed-case control proving a normally acquired lock still opens/migrates successfully.

This is the “other side” of the new lock: the success path is covered; the proof-failure path currently restores the defect.

2. The test named as cross-process coverage never crosses a process boundary, and the corruption reproduction is synthetic

test_concurrent_first_open_cannot_leave_a_non_database uses two threading.Threads in one interpreter. The production incident is multiple independently restarted gateway processes, and the implementation has different POSIX and Windows process-lock branches. A thread test is useful for local serialization, but it is not evidence that the process boundary from #102120 is fenced.

Also, the test's racing_apply() itself writes zero bytes over the SQLite header when the barrier overlaps. That proves “if overlap is defined to corrupt the file, the lock prevents overlap”; it does not reproduce the reported header corruption through the unmodified SQLite/Hermes path. That distinction matters because #102120 also reports a per-profile state.db damaged in the same restart window, which this hosted-room path does not explain.

Please add a real subprocess/multiprocess fence test: one process holds the hosted-rooms open lock while another attempts the same DB open, and the second must not enter the protected journal/schema section until ownership transfers. On Linux this should use the actual flock path; where practical, keep a Windows-specific contract for the msvcrt branch. If the original corruption itself cannot be made deterministic in a hermetic test, say that plainly—the lock can still be justified as a containment fence—but do not let the injected header overwrite stand in as causal reproduction.

3. This patch collides with the active owner split for this exact storage seam

#101474 is the complementary structural PR for gateway/hosted_rooms.py; it exists specifically because this owner is over the repository's 2K limit and moves _connect, root-DB schema/transactions, and related storage helpers into gateway/hosted_room_storage.py (1,095 lines in that candidate, facade 1,138). #99107 is already built on that split and also modifies the storage owner.

#102153 is not a duplicate of either PR: Justin's first-open serialization is new behavior and should keep its attribution. But this head adds 149 lines to the same monolithic file while the storage seam is already claimed by #101474, creating both a FILE-LIST collision and merge-order ambiguity.

Please make the order explicit and preserve contributor credit. The clean composition is: refresh/land the #101474 ownership split, then rebase this exact behavioral fix into gateway/hosted_room_storage.py, then let #99107 rebase over that storage owner. If the repository chooses a different order because #102120 is urgent, the eventual split still needs to carry this exact behavior and Justin Wilson's authorship forward rather than reimplementing it anonymously.

4. Exact-head completion evidence is not green, and Fixes #102120 currently over-closes the issue

For 0ada168c..., Docker (33753976427) and Nix (33753976459) are green, but the exact-head .github/workflows/ci.yaml run 33753976211 is completed failure. This PR has one commit, so the every-commit gate is not green. Current main has also advanced two commits since the PR base. Rebase/current-main composition and a fresh successful exact-head CI run are required before merge.

Separately, #102120 explicitly records a second observation: a per-profile state.db was also corrupted in the same restart window, and that half was not root-caused. This PR only fences the shared hosted-room DB. Fixes #102120 would therefore auto-close a report that still contains an unresolved state-corruption side. Either split/retain that per-profile corruption as a sibling issue before closing #102120, or narrow the closure claim after proving it is the same class and is actually covered. Do not lose that evidence in auto-close.

Once the lock fails closed, the process boundary is tested, the active storage-owner train is composed without dropping credit, and exact-head CI is green, the core shape here looks good: one shared DB, one explicit first-open authority fence, bounded retry, and persistent corruption still fails loudly.

[Halldrix]

Confirmed the retry path is dead code on the failure mode it was built for,
and the suite proves it.

Repro on this head (0ada168):

python3 -m pytest tests/gateway/test_hosted_rooms_shared_db_lock.py -v
=> 2 failed, 3 passed
FAILED test_first_open_retries_transient_not_a_database
FAILED test_persistent_not_a_database_still_raises

Root cause, independent of the other review threads:

1. sqlite3.connect() creates the target file at 0 bytes on first open.
2. _is_transient_first_open_corruption (gateway/hosted_rooms.py:1227)
   returns False when size == 0 ("A zero-length file is a brand-new
   database, never a scrambled header").
3. But in the real failure sequence the 0-byte window is exactly when the
   concurrent first open is racing: opener A created the file, is inside the
   journal-mode pragma, and opener B reads it while the header is unwritten
   or half-written. Every "file is not a database" raised in that window
   hits a 0-byte file, fails the transient check, and propagates without a
   single retry.

I verified this directly: patching apply_wal_with_fallback to raise
DatabaseError("file is not a database") against a fresh path, _connect
calls it exactly once and raises — the retry loop in _connect:1246 never
executes a second attempt, which is also why
test_persistent_not_a_database_still_raises asserts 3 calls and observes 1.

So on a fresh multi-profile install (no state.db yet — precisely the case in
#102120's environment, 16 profiles restored from backup), the bounded-retry
safety net described in the PR does not exist; the lock is the only barrier,
and per andrexibiza's review thread it fails open. Two suggested directions:

• Classify "0-byte file + file is not a database" as transient (retry);
  a genuinely brand-new DB open does not raise that error at all — the
  error itself is evidence another writer touched the file.
• Fix the test suite to assert the retry path actually fires against a
  0-byte target, so this mode can't silently regress again.

Not blocking on top of andrexibiza's four gates, but this one isn't
in his list and it independently defeats the patch's stated design on the
exact scenario from #102120.