fix: prevent silent message loss when state.db persistence fails - #32805
Closed
mlinquan wants to merge 12 commits into
Closed
fix: prevent silent message loss when state.db persistence fails#32805mlinquan wants to merge 12 commits into
mlinquan wants to merge 12 commits into
Conversation
19 tasks
mlinquan
force-pushed
the
fix/session-db-fallthrough
branch
2 times, most recently
from
May 28, 2026 16:20
7fe0328 to
67e3a47
Compare
Contributor
|
I found one issue that looks worth fixing before merge.
if self._session_db_failed:
self._session_db_failed = False
self._session_db_create_fail_count = 0 # ← resets to 0
logger.info("Session DB creation recovered after %d failures",
self._session_db_create_fail_count) # ← always logs 0The counter is zeroed before the log statement reads it, so the message always says "recovered after 0 failures" regardless of how many consecutive failures actually occurred. Why it matters: Operators debugging degraded session persistence rely on this log to understand recovery timing. A misleading "0 failures" message hides the real severity — a session that failed 50 times before recovering looks identical to one that never failed. Suggested fix: if self._session_db_failed:
failed_count = self._session_db_create_fail_count
self._session_db_failed = False
self._session_db_create_fail_count = 0
logger.info("Session DB creation recovered after %d failures", failed_count) |
mlinquan
force-pushed
the
fix/session-db-fallthrough
branch
2 times, most recently
from
May 29, 2026 13:00
3e6bc8f to
f9e00fb
Compare
Author
|
@alt-glitch Week-old ping — ready to merge? |
mlinquan
force-pushed
the
fix/session-db-fallthrough
branch
from
June 1, 2026 21:06
c2d3462 to
6daf794
Compare
This was referenced Jun 7, 2026
mlinquan
force-pushed
the
fix/session-db-fallthrough
branch
2 times, most recently
from
June 15, 2026 11:19
e82835f to
8a6523c
Compare
Three-fold protection against _ensure_db_session and append_message failures that previously caused silent, unrecoverable message loss: 1. _flush_messages_to_session_db: per-message try/except with break on failure. _last_flushed_db_idx now increments per-msg instead of bulk-assign, preserving partial progress. Sets _session_db_failed on any append failure. 2. _persist_session: immediately writes unflushed messages to a JSONL fallback file when _session_db_failed is set. Profile-isolated via HERMES_PROFILE env var. 3. Gateway agent_persisted check: now requires _session_db_created AND !_session_db_failed, not just _session_db is not None. Prevents the gateway from trusting an agent whose DB writes failed. Includes 8 unit tests covering append failure, JSONL fallback, profile isolation, and agent_persisted logic.
… profile from path isolation
- Add timestamp support to SessionDB.append_message - Save pending messages with wrapped timestamp format - Implement pending message recovery in gateway runner - Fix _last_flushed_db_idx update on partial failure - Archive pending files after recovery instead of deleting - Profile-aware pending file lookup - Backward compatible with old pending file format
Use max-overlap-prefix to detect already-persisted messages before inserting, preventing duplicate writes on recovery. Match key: (role, content, tool_name, tool_calls, tool_call_id)
Generate timestamp once before append_message call, store it on the message dict as _flush_timestamp, and pass it explicitly to both the DB insert and the JSONL fallback. Recovery now matches on (role, content, timestamp) so genuine repeats with identical content but different timestamps are preserved.
Previously the flag was sticky — once set it never cleared for the agent's lifetime, causing unnecessary JSONL fallback writes on every subsequent _persist_session call even after DB writes recovered.
…ticky flag Eliminate _session_db_failed flag entirely. When an append_message call fails inside _flush_messages_to_session_db, unflushed messages are written to the pending JSONL file immediately in the except block using last_successful_idx as the start cursor. The _persist_session caller only triggers fallback when _session_db is None entirely. This removes the sticky-flag false-positive problem where a transient failure in turn N caused unnecessary fallback writes in every subsequent turn even after the DB recovered.
…ackground review agent - _execute_write now retries on 'readonly'/'read-only' OperationalError, covering macOS WAL mode where multi-process contention returns SQLITE_READONLY instead of SQLITE_BUSY - background review sub-agent inherits parent's _session_db so its _persist_session calls write to state.db instead of pending fallback (the sub-agent reuses the parent session_id but was missing the DB handle, causing all messages to land in .pending.jsonl) - gateway: distinguish lock contention from other SessionDB init failures in log output
mlinquan
force-pushed
the
fix/session-db-fallthrough
branch
from
June 16, 2026 07:39
f9ac91f to
e35e056
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
append_message()fails (WAL lock contention, disk full, SQLITE_READONLY, etc.), messages accumulate inmessages[]and_last_flushed_db_idxis never advanced. On agent eviction, context compression, or process restart, those unflushed messages are silently lost. The Gateway also blindly trustsagent_persisted— it assumes DB writes succeeded as long as_session_db is not None, never verifying the agent actually committed anything.Additionally, SQLITE_READONLY (not just LOCKED/BUSY) was not retried in
_execute_write, causing transient WAL-readonly states to permanently fail. And background review agents ran without asession_dbhandle, so their writes went nowhere when the session DB had been recovered from a previous failure.Changes (12 commits, rebased on main)
Pending Fallback — Direct, No Sticky Flag
run_agent.py—_flush_messages_to_session_dbPer-message try/except. Each successful
append_messageadvances_last_flushed_db_idx. On failure, immediately calls_write_pending_fallback(messages, last_successful_idx)to dump unflushed messages tosessions/<sid>.pending.jsonl— no sticky flag, no deferred write.run_agent.py—_persist_sessionAdded a catch: when
_session_db is None(SQLite init never succeeded), writes ALL messages to pending fallback via_write_pending_fallback(messages, 0).run_agent.py—_write_pending_fallbackNew method. Wraps each message with its
_flush_timestampas{"_fallback_timestamp": t, "message": m}and appends to<sid>.pending.jsonl, respectingHERMES_PROFILEisolation.Shared Flush Timestamp for Safe Dedup
Every
append_messagein_flush_messages_to_session_dbnow generates a timestamp ONCE and passes it to both the DB write (via the newtimestamp=param ofSessionDB.append_message) and the message dict as_flush_timestamp. This anchors the dedup comparison: if a DB write succeeded but the fallback was still written (false alarm), both carry the same timestamp and won't be double-inserted during recovery.Deduplication is based on
(role, content, tool_name, tool_calls, tool_call_id, timestamp)— max-overlap suffix-vs-prefix match between existing DB messages and pending entries.SQLITE_READONLY Retry
hermes_state.py—_execute_writeAdded
"readonly"and"read-only"to the retry-eligible error string set. Previously only"locked"and"busy"were retried — WAL-mode readonly states were fatal on first attempt.Background Review Agent Gets session_db
agent/background_review.pyPass
session_db=getattr(agent, "_session_db", None)to review agents so their writes land in the correct session store.Gateway Startup Recovery
gateway/run.py—_recover_pending_messagesNew function, called in
GatewayRunner.__init__. Scans<hermes_home>/sessions/*.pending.jsonl, reads wrapped + plain messages, deduplicates against existing DB rows, replays new ones viaSessionDB.append_message(with original timestamps, multimodal handling, reasoning fields), and deletes the pending file on success.Hardening & Cleanup
_session_db_create_fail_counton successful recovery so consecutive-failure logging starts fresh_flush_messages_to_session_dbthat documented the cursor-tracking pattern_session_db_create_fail_countinit inagent_init.pyDesign Principle
Memory is the most unsafe place. When DB persistence fails, get messages to disk immediately — JSONL fallback is the last line of defense before data loss. Dedup via shared timestamp guarantees safe replay on restart.
Related work
Gateway replies with stale context when another process appends to the same session (cross-process agent-cache split-brain) #45966 —
fix(gateway): invalidate agent cache on cross-process session writesDirectly related: addresses the same cross-process session DB coordination problem from the Gateway side (agent writes to DB → Gateway cache was stale). This PR complements it by ensuring writes actually land before the Gateway ever has to invalidate.
Session DB turn-end flush drops assistant after repair_message_sequence compacts list (orphan user → \n\n merge) #44837 —
fix(agent): clamp flush cursor after repair_message_sequence compactionRelated but not identical: manages
_last_flushed_db_idxcorrectness during message sequence repair — the same cursor this PR tracks per-message.fix(agent): persist repaired-turn responses #46071 —
fix(agent): persist repaired-turn responsesRelated: touches the same
_persist_session/_flush_messages_to_session_dbpath to ensure repaired messages are persisted, not just held in memory.fix(state.db): recover from malformed sqlite_master so hidden sessions reappear #43149 —
fix(state.db): recover from malformed sqlite_masterRelated: another state.db resilience fix — covers the
sqlite_mastercorruption case while this PR covers the runtime write-failure case.fix(agent): clear _session_messages in AIAgent.close() #42123 —
fix(agent): clear _session_messages in AIAgent.close()Related: session message lifecycle — prevents stale references from surviving agent teardown.
feat(delegation): async background subagents via delegate_task(background=true) #40946 / fix(delegation): forward background flag so delegate_task(background=true) runs async #46968 —
feat/fix(delegation): async background subagentsRelated: the background review agent path that this PR fixes (was missing
session_db).fix: pass session_db to background review agents #47113 — extracted from this PR: standalone fix that passes
session_dbto background review agents.8905ee6 —
fix(agent): rewind flush cursor exactly when repair compacts before the cursorFollow-up to Session DB turn-end flush drops assistant after repair_message_sequence compacts list (orphan user → \n\n merge) #44837, already on main. A
min()clamp only fixes cursor overshoot past the new end of the list. Whenrepair_message_sequencedrops/merges messages at indexes below the cursor, the clamp leaves the cursor pointing past unflushed rows and the turn-end flush silently skips them. This PR's per-message cursor tracking (last_successful_idx) avoids the entire class of cursor healing bugs.8e4c447 —
fix(gateway): prevent duplicate user messages in state.dbRelated: Gateway-side dedup for user messages. When the agent has its own
_session_db,_flush_messages_to_session_db()persists user messages to SQLite. Two Gateway fallback paths also wrote the same user message withoutskip_db=True. Same problem domain — this PR adds dedup on recovery.46b2caf56 —
fix(state): use TRUNCATE WAL checkpoint to prevent unbounded WAL growthRelated: WAL management. Unbounded WAL growth can trigger readonly states in concurrent writer scenarios — exactly the class of
SQLITE_READONLYerrors that this PR's_execute_writeretry now handles.fix(compression): disable compression on background-review fork (#38727) #41708 —
fix(compression): disable compression on background-review fork to prevent cross-turn stale-parent forkRelated: background review fork isolation. The review fork's lifecycle edge case (stale parent state across turns) mirrors the missing
session_dbissue that fix: pass session_db to background review agents #47113 fixes.