fix(state): discard pooled read connections after DatabaseError

[Christopher-Schulze]

What does this PR do?

_read_ctx always returned a pooled read connection to the LIFO pool, even after sqlite3.DatabaseError. After an external replace/repair of state.db (forked curator closing the write fd, offline heal), that reader then either fails forever (file is not a database) or silently serves the old inode. The write path already self-heals via _reconnect_after_notadb; the read pool did not.

On DatabaseError, the connection is now closed and discarded. Successful reads still return to the pool.

Related Issue

Fixes #86518

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• hermes_state.py (_read_ctx): close the borrowed reader on sqlite3.DatabaseError instead of put_nowait.
• tests/test_session_db_read_conn_pool.py: a simulated file is not a database error is discarded; a successful checkout is still recycled.

How to Test

1. Inject a pooled reader that raises sqlite3.DatabaseError inside _read_ctx.
2. On current main the connection is returned to the pool (put_nowait called).
3. On this branch _close_read_conn is called and put_nowait is not.
4. A successful _read_ctx still returns the connection to the pool.
5. scripts/run_tests.sh tests/test_session_db_read_conn_pool.py for the two new tests plus existing fallback coverage.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (Darwin aarch64)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Fail-on-base: test_database_error_discards_pooled_read_conn fails on origin/main because the broken reader is recycled. Branch: discard + recycle tests pass. ruff / uv lock --check clean.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #85255: both repair the same _read_ctx pooled-reader requeue behavior after a SQLite DatabaseError.

[Enough1122]

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

fix(state): discard pooled read connections after DatabaseError

1. hermes_state.py:_read_ctx — only sqlite3.DatabaseError triggers the discard path. Its subclasses (OperationalError, ProgrammingError, IntegrityError, ...) are covered, so the main classes are handled — but note that sqlite3.Warning-class errors and non-sqlite exceptions raised inside the with body still return the connection to the pool. For the healthy-conn case that is correct; just confirming the granularity is deliberate (a transient 'database is locked' OperationalError also discards the reader, which is safe but slightly wasteful — the conn was probably fine).

2. The fallback branch (with self._lock: yield self._conn — the writer connection used when no reader is available) is not covered by this fix: a DatabaseError raised there propagates without discarding, and the reader pool is untouched. That may be fine since the write path self-heals via _reconnect_after_notadb, but the asymmetry between the two branches of the same contextmanager is worth a comment so a future reader doesn't assume both are handled.

3. Consider also clearing any pooled writer state on DatabaseError if the failure is disk-I/O class rather than schema class — currently only the individual reader is closed; a corrupted-file scenario leaves the write conn in the pool until the write path notices. Low risk given the existing self-heal, but the fix could note the boundary.

4. The change itself is clean, minimal, and the two new tests (discard on error / recycle on success) pin exactly the right behavior.