fix(state): don't clear in-flight turn marker when canonical flush fails

[eaglezzz0522-cloud]

Summary

AIAgent._persist_session() called note_turn_persisted() unconditionally, even when _flush_messages_to_session_db() returned False (SQLite write failure on exception). This made the runtime overstate success: the turn overlap tripwire lost the evidence that the previous turn never completed its persist.

Changes

# Before: return value of _flush_messages_to_session_db was discarded
self._flush_messages_to_session_db(messages, conversation_history)
note_turn_persisted(self)  # always called, even on flush failure

# After: flush result is checked, note_turn_persisted only called on success
flush_ok = self._flush_messages_to_session_db(messages, conversation_history)
if flush_ok is not False:
    note_turn_persisted(self)
    return True
return False

_flush_messages_to_session_db returns:

• True on success — clear the in-flight marker
• None when persist is disabled or no session DB exists — consistent state, still clear
• False on exception — do not clear the marker, preserve the evidence

Related

Closes #74284

[teknium1]

Thanks for isolating the meaningful False return from the SQLite flush.

Problems

• run_agent.py:1860 changes the tripwire's semantics, but agent/agent_runtime_helpers.py:482-485 explicitly says the marker is cleared unconditionally so this diagnostic under-reports rather than emits false overlap reports. With this change, a completed turn whose flush fails leaves the marker behind; the next serial turn is then treated as an overlap by note_turn_start() at agent/agent_runtime_helpers.py:429-444.
• The new boolean return does not yet establish a caller-visible persist contract: the normal finalizer discards _persist_session()'s result at agent/turn_finalizer.py:352.

Suggested changes

• Keep the overlap marker's unconditional clear. Surface failed SQLite persistence through a separate diagnostic or recovery path rather than using a concurrency tripwire.
• Add a regression test for a failed flush followed by a serial turn, ensuring only real concurrent turns trigger the overlap warning.

Automated hermes-sweeper review.

[teknium1]

Retaining this marker makes the next serial turn look like a concurrent overlap: note_turn_start() warns whenever a prior marker remains (agent/agent_runtime_helpers.py:429-444). The tripwire was explicitly made unconditional to avoid noisy diagnostics (agent/agent_runtime_helpers.py:482-485, commit 59787b9ada6). Please preserve that clear and report flush failure through a separate signal.

[eaglezzz0522-cloud]

Thanks for the review, hermes-sweeper. I have redesigned the approach based on the feedback.

Changes:

1. Reverted the gating of note_turn_persisted on flush_ok — the marker is now cleared unconditionally again, preserving the design contract that "a diagnostic must never be noisier than the defect it hunts."
2. Instead, a logger.warning is emitted when flush_ok is False, surfacing the failed SQLite persistence through a separate diagnostic path (with turn_id and session_id context).
3. _persist_and_drain() return type reverted to None since the finalizer already discards the result.

Rationale per the suggestion:

• "Keep the overlap marker's unconditional clear." ✅ Done
• "Surface failed SQLite persistence through a separate diagnostic." ✅ Done — logger.warning(...) now fires on flush failure with full context.
• "Add a regression test for a failed flush followed by a serial turn." — The marker is now unconditional, so a failed flush no longer causes false overlap warnings. The diagnostic path is purely log-based, which is consistent with how other transient SQLite failures are surfaced in the codebase.

[GottZ]

This was generated by AI during triage.

Summary

Two PRs address #74284 by reacting to an explicit False result from the canonical SQLite flush. #74305 propagates the failure and preserves the in-flight marker, while #74540 originally gated marker clearing but is reported by its author to have moved toward a separate warning diagnostic.

Related pull requests

• fix(agent): preserve in-flight marker on flush failure #74305 duplicate — (+60/-11) — overlapping fix with a design conflict: the diff returns False, skips token-count draining, and preserves the in-flight marker after a failed canonical flush, directly addressing the issue but causing the next serial turn to be diagnosed as an overlap under the tripwire semantics documented in the review of fix(state): don't clear in-flight turn marker when canonical flush fails #74540.
• fix(state): don't clear in-flight turn marker when canonical flush fails #74540 best fix — (+19/-1) — keep open pending verifiable revision: the recorded change checks the canonical flush result, while the author reports subsequently restoring unconditional marker clearing and adding a separate warning. This follows the keep_open review on fix(state): don't clear in-flight turn marker when canonical flush fails #74540, which rejects marker retention because it creates false overlap diagnostics and requests a failed-flush-then-serial-turn regression test; that test is not evidenced here.

Duplicates

#74305 and #74540 target the same _persist_session() handling of a False canonical flush; #74305 is the broader duplicate implementation, but both originally couple persistence failure to the overlap marker.

Suggested consolidation

Keep #74540 open with a salvage path: retain the separate persistence-failure diagnostic, preserve unconditional overlap-marker clearing, and add the contributor-requested regression test proving that a failed flush followed by a serial turn does not emit a false overlap warning. The revised behavior must be visible in the actual diff before consolidation; close #74305 as a duplicate of #74540 because its supplied diff retains the marker and therefore conflicts with the documented tripwire design.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I74284(["issue #74284 (open)"])
    subgraph Dup74305 ["PRs duplicating each other"]
        P74305["PR #74305 (open)"]
        P74540["PR #74540 (open)"]
    end
    P74540 -->|best fix| I74284
    class I74284 open
    class P74305 open
    class P74540 open
    class P74305 best
    class P74540 best
    class P74540 target
    click I74284 "https://github.com/NousResearch/hermes-agent/issues/74284"
    click P74305 "https://github.com/NousResearch/hermes-agent/pull/74305"
    click P74540 "https://github.com/NousResearch/hermes-agent/pull/74540"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Diffs were read for 1 of 2 PRs (rest unavailable); Assessment working set: 5 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (3 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.