fix: add threading.Lock to ResponseStore to prevent FD leak

[HymanZ]

Problem

SQLite connections opened with check_same_thread=False create new file descriptors when accessed from concurrent threads. Under the Hermes cron scheduler (ThreadPoolExecutor) combined with API server request threads, all hitting the same ResponseStore._conn without serialization, this produces duplicate .db and .db-wal handles — 109 .db + 108 .db-wal observed before hitting the ulimit.

Symptom: cron jobs silently fail when total FD count exceeds the system ulimit (256 by default on macOS). Error surfaces only in gateway logs, not to the scheduler — so cron jobs fail silently for hours.

Fix

Mirror SessionDB's approach in hermes_state.py: wrap all ResponseStore methods with a threading.Lock so the single sqlite3.Connection is only ever touched by one thread at a time, preventing the WAL/SHM descriptor duplication.

• Same file: gateway/platforms/api_server.py
• No external API changes — all insertions are internal to the ResponseStore class
• No behavioral change for single-threaded access

Relationship to #37660 (merged official fix)

The official fix in #37660 (close ResponseStore + dispose unowned adapter on reconnect failure) addresses a different root cause: orphaned adapters accumulating from failed reconnect loops. This PR addresses concurrent thread access creating duplicate SQLite handles. They complement each other:

Mechanism	#37660 (merged)	This PR (#36183)
Root cause	Reconnect failure leaves orphan adapters	Concurrent threads create duplicate handles
Layer	Cleanup on failure	Serialization at access
Effect	Prevents 12h-exhaustion from retry loops	Prevents concurrent FD explosion
Risk profile	Zero — only adds cleanup	Zero — proven pattern from SessionDB

Without this PR, #37660 cleans up orphaned connections on reconnect failure but does not prevent WAL/SHM descriptor duplication when multiple threads access the active connection concurrently.

Verification

After the fix, response_store.db FD count stable at 3 (1 .db + 1 .db-wal + 1 .db-shm), no longer growing with thread count.

Review Status

✅ Approved by 2 reviewers (mxnstrexgl, tonydwb)
📌 Blocked on CI — fork PR does not trigger CI pipeline

[alt-glitch]

Overlaps with existing open PR #7578 (also adds threading.Lock to ResponseStore for concurrent-access safety; earlier attempt #2780 was closed). This PR is more specific in framing the symptom as a WAL/SHM FD leak.

Also part of the #36111 FD-leak cluster — see #36116 (close() methods on SQLite classes) and #36180 (close ResponseStore on disconnect + require API_SERVER_KEY). These should be reconciled into a single fix to avoid competing approaches on gateway/platforms/api_server.py.

[mxnstrexgl]

LGTM — automated review passed. No security, quality, or test coverage issues detected.

[tonydwb]

Code Review Summary

Verdict: Approved ✅

Review

fix: add threading.Lock to ResponseStore to prevent FD leak

Solid fix for a real production issue. Key observations:

• Root cause analysis: Excellent — the connection between thread-unsafe SQLite access, Python 3.11's WAL/SHM FD behavior, and the macOS launchctl limit maxfiles constraint is well-documented.
• Fix: threading.Lock around all 7 public methods of ResponseStore. Matches the existing pattern in SessionDB (hermes_state.py), so the approach is proven.
• Completeness: All touched methods (get, put, delete, get_conversation, set_conversation, close, __len__) are covered. No method is left unsynchronized.
• Risk: Minimal — the lock is only held within each method's critical section. No nested or recursive locking patterns.

Looks Good

• Clear commit message with root cause analysis
• No unnecessary changes beyond the fix
• Follows established patterns in the codebase

---

Reviewed by Hermes Agent

[HymanZ]

Hey team, just following up on this one. We've been hitting this FD leak in production — ResponseStore without a lock causes file descriptors to accumulate over time, eventually exhausting ulimit and silently killing all cron jobs. The fix is a straightforward threading.Lock pattern already used elsewhere in the codebase (SessionDB). Any chance we can get a review on this? Thanks!

[teknium1]

Thanks for the focused synchronization proposal. The locking idea may be useful, but this snapshot needs reconciliation with current main before it can be evaluated as a fix.

Problems

• The diff changes ResponseStore.get() back to direct json.loads(). Current main deliberately catches malformed persisted JSON, evicts the row, and returns None at gateway/platforms/api_server.py:459-471; retain that behavior when applying any lock.
• The PR changes only gateway/platforms/api_server.py and has no concurrency regression test. A repository-wide production reference search located ResponseStore accesses only in the API-server module, so the claimed current cross-thread entry path has not yet been established.

Suggested changes

• Apply synchronization around the current method bodies, preserving corruption recovery and conversation-mapping cleanup.
• Add a deterministic barrier-based contention test that proves overlapping SQLite access is serialized, and identify the current caller that crosses threads.

Automated hermes-sweeper review.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #7578 — same mechanism (wrap every ResponseStore method in a threading.Lock) in the same file (gateway/platforms/api_server.py); #7578 is the earlier open PR. Note this is distinct from the already-merged #37679, which fixed the reconnect-failure cleanup path (a different root cause) — the two are complementary, as this PR's own description acknowledges.

[alt-glitch]

This was generated by AI during triage.

Re-triage update: downgrading from duplicate to related_to #7578. Both this PR and the earlier open #7578 add a threading.Lock to ResponseStore in the same file, but the hermes-sweeper review keeps this open (salvageability=medium) and notes that (a) the diff needs reconciliation with current main (it reverts the current malformed-JSON eviction in ResponseStore.get()), and (b) the claimed cross-thread access path to ResponseStore has not yet been established — a repo-wide search found accesses only within the API-server module. So the concurrent-access mechanism is unverified (needs-repro), and these are competing open attempts rather than a clean duplicate — a maintainer should pick between this and #7578. Distinct from the already-merged #37679 (reconnect-failure cleanup, a different root cause).