Issue Station
Sign In
Tools hereToolbox

hermes - ✅(Solved) Fix External memory sync should skip interrupted turns [3 pull requests, 1 comments, 2 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
NousResearch/hermes-agent#15218•Fetched 2026-04-25 06:23:43
View on GitHub
Comments
1
Participants
2
Timeline
12
Reactions
0
Author
yepyhunyepyhun
Participants
teknium1teknium1yepyhunyepyhun
Timeline (top)
labeled ×4cross-referenced ×3closed ×1commented ×1

External memory providers currently receive sync_all(original_user_message, final_response) for a turn whenever final_response and original_user_message are present. The sync path does not appear to exclude interrupted turns.

This can cause an external memory provider to persist an incomplete/interrupted exchange as if it were a completed conversational turn.

Error Message

External memory provider: sync the completed turn + queue next prefetch.

Use original_user_message (clean input) — user_message may contain

injected skill content that bloats / breaks provider queries.

if self._memory_manager and final_response and original_user_message: try: self._memory_manager.sync_all(original_user_message, final_response) self._memory_manager.queue_prefetch_all(original_user_message) except Exception: pass

Root Cause

Memory providers need a clean boundary between:

  • a completed assistant turn that is safe to persist as conversational evidence; and
  • an interrupted/reset/cancelled turn whose final state is not reliable durable memory.

If an interrupted turn is mirrored into an external memory backend, downstream recall can treat partial assistant output, partial tool processing, or aborted work as durable conversation truth.

Fix Action

Fixed

PR fix notes

PR #15249: fix(memory): skip external-provider sync on interrupted turns (#15218)

Description (problem / solution / changelog)

What does this PR do?

Fixes `#15218`. `run_conversation` was calling `memory_manager.sync_all(original_user_message, final_response)` at the end of every turn where both args were present, without considering the `interrupted` local flag. Result: an external memory backend received partial assistant output, aborted tool chains, or mid-stream resets as durable conversational truth — downstream recall then treated the not-yet-real state as if the user had seen it complete, poisoning the trust boundary between "what the user took away from the turn" and "what Hermes was in the middle of producing when the interrupt hit".

Reporter's minimal-fix suggestion was to gate on `not was_interrupted`; I implemented that with a small refactor that also makes the logic testable in isolation.

Fix

Extracted the inline sync block in `run_conversation` into a new private method on `AIAgent`:

```python def _sync_external_memory_for_turn( self, *, original_user_message: Any, final_response: Any, interrupted: bool, ) -> None: if interrupted: return if not (self._memory_manager and final_response and original_user_message): return try: self._memory_manager.sync_all(original_user_message, final_response) self._memory_manager.queue_prefetch_all(original_user_message) except Exception: pass ```

Why a helper instead of a one-line guard at the call site?

  • The interrupt guard becomes a single visible check at the top of the method instead of hidden in a boolean-and at the call site
  • Gives tests a clean seam to assert on — the pre-fix layout buried the logic inside the 3,000-line `run_conversation` function where no focused test could reach it
  • Zero behavior change for the positive path
  • The prefetch side-effect is also gated on the same interrupt flag — the user's next message is almost certainly a retry of the same intent, and a prefetch keyed on the interrupted turn would fire against stale context

Skip conditions, encoded explicitly

ConditionOutcomeWhy
`interrupted=True`skip entirelyThe #15218 fix. Applies even if both strings happen to be populated — an interrupt may have landed between a streamed reply and the next tool call, so the strings-on-disk lie
No memory managerskipPre-fix behaviour preserved
No `final_response`skipPre-fix behaviour — tool-only turns that never resolved
No `original_user_message`skipPre-fix behaviour — system-initiated refresh, not a user turn
`sync_all` or `queue_prefetch_all` raisesswallowPre-fix behaviour — external memory providers are strictly best-effort; a misconfigured backend must not block the user from seeing their response

Related Issue

Fixes #15218

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ♻️ Refactor (no behavior change) — extracted inline sync block into a helper
  • ✅ Tests (adding or improving test coverage)

Test plan

  • 16 new tests in `tests/run_agent/test_memory_sync_interrupted.py` — all green on py3.11 venv
  • Existing `test_memory_provider_init.py` still green — the refactor preserves the positive path exactly
  • Import smoke check on `run_agent` module
  • No pre-existing tests assert on `memory_manager.sync_all` — grep -rn confirmed the seam is new

Test coverage detail

`TestSyncExternalMemoryForTurn` (16 tests):

  • `test_interrupted_turn_does_not_sync` — core fix
  • `test_interrupted_turn_skips_even_when_response_is_full` — a long seemingly-complete response is still partial if `interrupted=True`; the strings-on-disk lie
  • `test_completed_turn_syncs_and_queues_prefetch` — regression guard for the positive path; `sync_all` AND `queue_prefetch_all` both called with the right args
  • `test_no_final_response_skips` / `test_no_original_user_message_skips` / `test_no_memory_manager_is_a_no_op` — pre-fix skip paths preserved
  • `test_sync_exception_is_swallowed` / `test_prefetch_exception_is_swallowed` — best-effort invariant
  • `test_sync_matrix` — 8-case parametrised matrix over (interrupted × final_response × user_message), asserts sync fires iff `interrupted=False` AND both strings are non-empty

Not in scope

  • Exposing a structured turn-status to memory providers so they can make their own skip decisions — the reporter mentioned this as an alternative; bigger API surface, saving it for a follow-up if maintainers want it
  • #15219 (write-origin metadata for memory writes) — same neighbourhood, separate change; the reporter filed both at once

Changed files

  • run_agent.py (modified, +48/-8)
  • tests/run_agent/test_memory_sync_interrupted.py (added, +189/-0)

PR #15395: fix(memory): skip external-provider sync on interrupted turns (#15218)

Description (problem / solution / changelog)

Salvage of #15249 by @briandevans (authorship preserved via cherry-pick).

Summary

External memory providers no longer persist interrupted turns as if they were completed exchanges. Previously, the sync block at the end of run_conversation was gated on memory_manager and final_response and original_user_message but not on interrupted — inconsistent with the background-review block four lines below which already checks not interrupted.

Root cause

When a user interrupts mid-turn, final_response is either partial assistant output or the literal string "Operation interrupted: waiting for model response (Ns elapsed).". Either way, mirroring that into an external memory backend poisons downstream recall.

Changes

  • run_agent.py: extract inline sync block into AIAgent._sync_external_memory_for_turn(...) that returns early when interrupted=True
  • tests/run_agent/test_memory_sync_interrupted.py: 16 tests covering interrupt guard, positive path, edge cases, and best-effort exception swallowing

Validation

BeforeAfter
Interrupted turn with partial responsesynced to external memoryskipped
Interrupted turn with "Operation interrupted…" stringsynced as assistant answerskipped
Completed turnsynced + prefetchedunchanged
No memory manager / no response / no user msgskippedskipped
Backend raisesswallowedswallowed
Test suiteN/A16/16 pass

Closes #15218. Closes #15249 (original PR by @briandevans — commit cherry-picked with authorship preserved).

Changed files

  • run_agent.py (modified, +48/-8)
  • tests/run_agent/test_memory_sync_interrupted.py (added, +189/-0)

PR #15381: fix(honcho): bug-fix consolidation — 7 upstream PRs, preserved authorship

Description (problem / solution / changelog)

Consolidates seven Honcho bug fixes from the open PR queue into one stack. Each upstream PR is a single commit (two for #15162 which itself was two) with original author + date preserved.

Tracked fixes (in cherry-pick order):

  • #15162 @briandevans — pinPeerName opt-in keeps memory unified across platforms (fixes #14984, #14401)
  • #13931 @Sanjays2402 — truncate resolve_session_name output to Honcho's 100-char limit (fixes #13868)
  • #13510 @hekaru-agent — thread-safe session cache via RLock (fixes #5102; off-topic cron/jobs.py hunk dropped, small conflict with pinPeerName guard resolved keeping both)
  • #13672 @dontcallmejames — memory-context leak boundary hardening (fixes #5719; only the boundary-fix commits 503fa1b9 + e11bbd98 adopted, the unrelated update-command and conclusions-query commits dropped)
  • #13320 @HiddenPuppy — HOME-aware config fallback path resolution (fixes #13283)
  • #14881 @sasha-id — CLI credential guard accepts self-hosted baseUrl-only configs
  • #13623 @twozle — default SDK HTTP timeout to 30s

Dropped from the original queue as already-shipped or not applicable:

  • #1878 — context_tokens budget already implemented at plugins/memory/honcho/__init__.py:684 (_truncate_to_budget). Can be closed upstream.

Files touched (9 commits total):

  • plugins/memory/honcho/client.py
  • plugins/memory/honcho/session.py
  • plugins/memory/honcho/init.py
  • plugins/memory/honcho/cli.py
  • hermes_state.py
  • run_agent.py
  • tests across honcho_plugin/, run_agent/, test_hermes_state.py

1442 of 1443 tests pass across honcho_plugin + run_agent + hermes_state. The single failure (tests/run_agent/test_tool_arg_coercion.py::TestCoerceNumber::test_inf_stays_string_for_integer_only) is a preexisting flake on main, unrelated to these changes.

Also landed separately and merged earlier (not part of this PR):

  • #7193 on_turn_start() hook — already wired at run_agent.py:9223. Close issue.
  • #7192 on_pre_compress() return value — still broken at run_agent.py:7791. Keep open.

Changed files

  • agent/memory_manager.py (modified, +111/-0)
  • gateway/run.py (modified, +9/-0)
  • hermes_state.py (modified, +6/-1)
  • plugins/memory/honcho/__init__.py (modified, +5/-2)
  • plugins/memory/honcho/cli.py (modified, +21/-2)
  • plugins/memory/honcho/client.py (modified, +67/-3)
  • plugins/memory/honcho/session.py (modified, +61/-36)
  • run_agent.py (modified, +50/-5)
  • tests/agent/test_streaming_context_scrubber.py (added, +150/-0)
  • tests/gateway/test_vision_memory_leak.py (added, +99/-0)
  • tests/honcho_plugin/test_cli.py (modified, +66/-0)
  • tests/honcho_plugin/test_client.py (modified, +112/-3)
  • tests/honcho_plugin/test_pin_peer_name.py (added, +307/-0)
  • tests/honcho_plugin/test_session.py (modified, +33/-0)
  • tests/run_agent/test_run_agent.py (modified, +14/-0)
  • tests/run_agent/test_run_agent_codex_responses.py (modified, +94/-0)
  • tests/test_hermes_state.py (modified, +18/-0)

Code Example

# External memory provider: sync the completed turn + queue next prefetch.
# Use original_user_message (clean input) — user_message may contain
# injected skill content that bloats / breaks provider queries.
if self._memory_manager and final_response and original_user_message:
    try:
        self._memory_manager.sync_all(original_user_message, final_response)
        self._memory_manager.queue_prefetch_all(original_user_message)
    except Exception:
        pass
RAW_BUFFERClick to expand / collapse

Summary

External memory providers currently receive sync_all(original_user_message, final_response) for a turn whenever final_response and original_user_message are present. The sync path does not appear to exclude interrupted turns.

This can cause an external memory provider to persist an incomplete/interrupted exchange as if it were a completed conversational turn.

Why this matters

Memory providers need a clean boundary between:

  • a completed assistant turn that is safe to persist as conversational evidence; and
  • an interrupted/reset/cancelled turn whose final state is not reliable durable memory.

If an interrupted turn is mirrored into an external memory backend, downstream recall can treat partial assistant output, partial tool processing, or aborted work as durable conversation truth.

Current behavior in vanilla Hermes

In run_agent.py, after clear_interrupt() has been called, the external memory sync path is guarded by self._memory_manager and final_response and original_user_message, but not by the interrupted state itself:

# External memory provider: sync the completed turn + queue next prefetch.
# Use original_user_message (clean input) — user_message may contain
# injected skill content that bloats / breaks provider queries.
if self._memory_manager and final_response and original_user_message:
    try:
        self._memory_manager.sync_all(original_user_message, final_response)
        self._memory_manager.queue_prefetch_all(original_user_message)
    except Exception:
        pass

Expected behavior

External memory providers should not be asked to persist interrupted turns as completed conversation turns.

A minimal fix could be one of:

  • preserve an interrupted / was_interrupted local before clearing interrupt state and gate sync on not was_interrupted; or
  • expose a structured turn status to memory providers so they can skip non-final turns.

Suggested acceptance criteria

  • Interrupted turns do not call MemoryManager.sync_all(...) as completed turns.
  • Normal completed turns still sync as before.
  • There is a regression test for interrupted-turn memory sync behavior.
  • The fix is generic and does not depend on any specific external memory provider.

extent analysis

TL;DR

To fix the issue, add a check for the interrupted state before calling sync_all on the external memory provider.

Guidance

  • Introduce a local variable was_interrupted before calling clear_interrupt() to track the interrupted state.
  • Gate the sync_all call on the was_interrupted flag, only syncing when the turn was not interrupted.
  • Consider exposing a structured turn status to memory providers for more flexibility.
  • Ensure the fix is generic and does not depend on specific external memory providers.

Example

was_interrupted = self._interrupted
self.clear_interrupt()
if self._memory_manager and final_response and original_user_message and not was_interrupted:
    try:
        self._memory_manager.sync_all(original_user_message, final_response)
        self._memory_manager.queue_prefetch_all(original_user_message)
    except Exception:
        pass

Notes

The suggested fix assumes that clear_interrupt() resets the interrupted state. If this is not the case, an alternative approach may be needed.

Recommendation

Apply the workaround by introducing the was_interrupted flag and gating the sync_all call on it, as this provides a straightforward fix without requiring significant changes to the existing codebase.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…

FAQ

Expected behavior

External memory providers should not be asked to persist interrupted turns as completed conversation turns.

A minimal fix could be one of:

  • preserve an interrupted / was_interrupted local before clearing interrupt state and gate sync on not was_interrupted; or
  • expose a structured turn status to memory providers so they can skip non-final turns.
#chain error#conversation history#tool integration#LLM response#prompt template

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

hermes - ✅(Solved) Fix External memory sync should skip interrupted turns [3 pull requests, 1 comments, 2 participants]