fix(state): retry _connect_and_init when concurrent repair is in prog…

[MorAlekss]

Summary

Adds a retry loop to SessionDB.__init__() when _claim_repair_attempt()
returns False, indicating another thread or process is already repairing
a malformed state.db. Previously the losing caller raised immediately
with sqlite3.DatabaseError, surfacing a misleading "Session database not
available" error even though the database was being healed concurrently.

---

Root cause

_connect_and_init() in hermes_state.py catches sqlite3.DatabaseError
for malformed schema errors and attempts automatic repair via
repair_state_db_schema(). The repair path is guarded by
_claim_repair_attempt(), which ensures only one caller performs the
repair per process.

When a second caller loses the claim race (_claim_repair_attempt()
returns False), the previous implementation immediately re-raised the
original exception. This gave concurrent callers no opportunity to wait
for the repair to complete and retry initialization.

Practical scenario: two threads or processes attempt to open state.db
at startup after a crash left the database with a malformed schema. One
caller successfully claims and performs the repair; the other fails with
sqlite3.DatabaseError instead of benefiting from the ongoing repair.

---

Behavioral change

Before: if _claim_repair_attempt() returned False, SessionDB.__init__()
immediately raised the malformed-schema exception. The caller recorded
_last_init_error and degraded to "Session database not available".

After: when the error is identified as a malformed-schema error and
_claim_repair_attempt() returns False, the losing caller enters a
bounded retry loop. The loop reuses the existing retry configuration from
_execute_write() (_WRITE_MAX_RETRIES, _WRITE_RETRY_MIN_S,
_WRITE_RETRY_MAX_S) and sleeps for a random 20-150ms between attempts.

On each iteration _connect_and_init() is retried. Once the winning
caller completes repair, the retrying caller connects successfully
without surfacing an error.

If retries are exhausted, the original exception is re-raised. Likewise,
non-malformed database errors continue to be raised immediately. In both
cases the existing _last_init_error and degradation behavior remain
unchanged.

---

What changed

hermes_state.py

• Split the previous combined condition

  if not is_malformed_db_error(exc) or not _claim_repair_attempt():

  into separate malformed-error and repair-claim checks

• Added a bounded retry path when:

  • is_malformed_db_error(exc) is True, and
  • _claim_repair_attempt() returns False

• The retry loop reuses the existing _execute_write() retry constants
  (_WRITE_MAX_RETRIES, _WRITE_RETRY_MIN_S,
  _WRITE_RETRY_MAX_S) rather than introducing new configuration

• Each retry waits using the same
  random.uniform(...) + time.sleep(...) jitter pattern already used by
  _execute_write() (approximately 20-150ms per attempt, up to
  _WRITE_MAX_RETRIES attempts)

• _connect_and_init() is retried until repair completes or retry
  attempts are exhausted

tests/test_state_db_malformed_repair.py

• Added import threading
• Added import time
• Added test_retry_when_concurrent_repair_completes

The test:

• Creates a malformed database
• Starts a background thread that repairs the database after a short delay
• Monkeypatches _claim_repair_attempt() to always return False,
  ensuring the code exercises the retry path rather than the repair path
• Verifies that SessionDB initialization succeeds once the delayed
  repair completes

---

What is NOT changed

• Happy path (single caller, successful repair) is unchanged

• _claim_repair_attempt() logic is unchanged

• repair_state_db_schema() is unchanged

• Non-malformed database errors are still raised immediately

• All 10 existing tests in test_state_db_malformed_repair.py continue to
  pass, including repair-path coverage such as:

  • test_repair_preserves_sessions_and_messages
  • test_repaired_db_search_works
  • test_sessiondb_auto_heals_on_open
  • test_auto_heal_attempted_once_per_process

• No behavior changes occur when state.db is healthy

• Existing _last_init_error handling and degradation behavior remain
  unchanged when initialization ultimately fails

[liuhao1024]

Reviewed the concurrent-repair retry logic — the approach is sound.

Verification notes:

• The retry loop correctly waits for a concurrent repair to complete, matching the jitter retry pattern used elsewhere (_WRITE_RETRY_MIN_S / _WRITE_RETRY_MAX_S).
• The _claim_repair_attempt re-check inside the retry loop correctly detects when the other repair finishes (returns True), at which point the retry either succeeds or raises appropriately.
• last_err is properly tracked and raised if all retries exhaust.
• The test with fake_claim always returning False + a background thread performing delayed repair is a solid concurrency test design.

LGTM. Good fix for the race condition when multiple threads/processes detect a malformed DB simultaneously.

[Morad37]

Solid fix for the concurrent repair race. The retry-with-jitter matches the existing write-contention pattern and the test that simulates a competing thread finishing mid-retry is well crafted.

[teknium1]

Thanks for addressing a real recovery race: current main re-raises when an in-process caller loses _claim_repair_attempt() (hermes_state.py:1006).

Problems

• _claim_repair_attempt() is deliberately process-local (hermes_state.py:479-491), so the new “Another thread/process” path cannot coordinate two processes. Each process can claim independently and enter repair. Please either scope this change to threads or add real inter-process coordination.
• The new test directly calls repair_state_db_schema() after forcing every claim to return False (tests/test_state_db_malformed_repair.py:262-278). It does not exercise a winning SessionDB initializer and a losing initializer sharing the real claim guard.

Suggested changes

• Add a coordinated two-SessionDB regression test: block the winning repair after it obtains the actual claim, start the losing initializer, release repair, and assert both initializations succeed.
• Salvage this against the current initializer at hermes_state.py:946-1035; GitHub currently marks the branch conflicting.

Automated hermes-sweeper review.

[teknium1]

_claim_repair_attempt() is only a module-level set protected by threading.Lock (hermes_state.py:479-491), so it cannot identify a repair owned by another process. Please scope this comment/behavior to same-process callers, or add an inter-process coordination primitive.

[teknium1]

This repairs the DB directly while every claim is forced to fail, so it does not exercise a real winning SessionDB repair claim. Coordinate two SessionDB initializations instead: block the winner after the real claim, then verify the loser opens when that repair completes.

[MorAlekss]

Both issues addressed. The branch has been rebased on current main — the merge conflict in tests/test_state_db_malformed_repair.py is resolved.

Comment scoped to threads: the "Another thread/process" comment has been updated to "Another thread is already repairing" with an explicit note that _claim_repair_attempt() is process-local a module-level set behind a threading.Lock so it serialises threads within one process only, not separate processes.

Coordinated two-opener regression test: added test_concurrent_repair_serialises_two_sessiondb_openers in tests/test_state_db_malformed_repair.py. The test uses the real _claim_repair_attempt guard (with a fresh _repair_attempted_paths via monkeypatch) so winner and loser are determined by the actual mechanism, not a fake claim. The winning opener is blocked inside repair_state_db_schema via a threading.Event barrier after obtaining the real claim. The losing opener is started while the winner holds the barrier so it is inside its retry loop. The barrier is released, both threads join, and the test asserts both SessionDB instances open cleanly, both see the recovered session row, and repair_state_db_schema was called exactly once proving the loser reused the winner's repair rather than performing its own.