fix(state): evict a poisoned pooled read connection instead of requeuing it

[pierrenode]

Summary

_reconnect_after_notadb() self-heals the single shared write connection (self._conn) when it starts raising sqlite3.DatabaseError("file is not a database") — the runtime-corruption signature left when a sibling process (a forked curator agent, an external repair pass) replaces/truncates the backing file out from under a live connection. It's called from exactly one place: _execute_write's retry loop.

_read_ctx()'s pooled read connections (self._read_pool, opened via _get_read_conn()) have no equivalent. Its finally block unconditionally requeues the checked-out connection regardless of whether the yield conn block raised:

conn = self._checkout_read_conn()
if conn is not None:
    try:
        yield conn
    finally:
        ...
        self._read_pool.put_nowait(conn)   # returned even if the read above raised

If the same corruption class hits a pooled read connection, every subsequent read that draws that connection from the pool hits the identical "file is not a database" error, forever — unlike the writer's self._conn, nothing ever reopens a pooled connection once it starts failing, since the write-side self-heal only touches self._conn and has no visibility into (or reach into) the read pool's separate mode=ro connections. Up to _READ_POOL_MAX (8) read slots can become permanently wedged for the life of the process this way, even after the write path has already self-healed.

Fix

Scope the fix to the exact same signature _is_not_a_database_error() already detects for the write path. On that specific error, close/discard the pooled connection (releasing its descriptor permit via the existing _close_read_conn()) instead of returning it to the pool — a fresh connection opens on the next miss via _get_read_conn(). Any other exception (a caller's own bad query, a DatabaseError of a different class) still requeues the connection exactly as before: a healthy connection must not be discarded just because the code using it raised for an unrelated reason.

Testing

• Added TestReadPoolEvictsPoisonedConnection to tests/test_state_db_notadb_selfheal.py (the existing home for this exact self-heal mechanism's tests), mirroring its established real-SessionDB-instance testing style:
  • test_poisoned_connection_is_evicted_not_requeued: checks a connection out of the pool twice (proving it's the same pooled object), simulates the corruption signature on the second use, and verifies the pool is empty afterward and a third read opens a genuinely new connection rather than the poisoned one.
  • test_other_database_errors_still_requeue_normally: proves the eviction is scoped to the specific signature — an unrelated DatabaseError (e.g. "database disk image is malformed") still requeues a healthy connection.
• Mutation-verified: reverted the production fix (kept the tests) and confirmed test_poisoned_connection_is_evicted_not_requeued fails against pre-fix code (assert 1 == 0 — the poisoned connection was requeued).
• tests/test_state_db_notadb_selfheal.py: 11/11 passed (9 pre-existing + 2 new).
• Broader neighbor sweep: tests/test_hermes_state.py — 221/222 passed. The one failure (TestFTS5Search::test_search_projection_skips_context_enrichment_queries) is pre-existing and unrelated: verified by reverting the fix and re-running that single test in isolation — it fails identically with or without this change. tests/test_wal_checkpoint_strategy.py, tests/test_state_db_malformed_repair.py, tests/test_state_db_stats.py: 36/36 passed.
• ruff check clean on both touched files.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(state): evict a poisoned pooled read connection instead of requeuing it

1. hermes_state.py (_read_ctx): the eviction path relies on _close_read_conn(conn) releasing the pool permit so a fresh connection opens on the next miss (confirmed by the tests asserting pool size drops to 0 and a new connection is created). A comment noting that _close_read_conn is safe to call when the pool is already closed (the close() drain path) would make that invariant explicit for future readers.

2. Scoping eviction to _is_not_a_database_error is the right call, and re-raising the original exception to the caller preserves the writer-side _reconnect_after_notadb contract (the first failure is still surfaced to the current query). Consistent behavior — no issue.

3. Minor alternative worth considering: detect the poisoned connection at checkout time (a cheap health probe) instead of on first failure, so the first real query never sees the error. That adds a round-trip per checkout, so the current lazy detection is a reasonable trade-off — noting it only for completeness.