Issue Station
Sign In
Tools hereToolbox

hermes - ✅(Solved) Fix Add write-origin metadata for external memory provider writes [3 pull requests, 1 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#15219•Fetched 2026-04-25 06:23:42
View on GitHub
Comments
0
Participants
1
Timeline
8
Reactions
0
Author
yepyhunyepyhun
Participants
yepyhunyepyhun
Timeline (top)
labeled ×4cross-referenced ×3closed ×1

External memory providers currently receive built-in memory writes without structured write-origin/provenance metadata. This makes it difficult for providers to distinguish user-authored durable memory from background/reflection/assistant-generated memory writes without brittle text heuristics.

This is a feature request for a small backwards-compatible metadata seam on MemoryProvider.on_memory_write(...) and MemoryManager.on_memory_write(...).

Error Message

Without structured origin metadata, a provider has to infer write provenance from text content or surrounding context. That is error-prone and can lead to assistant-authored self-diagnosis being stored as if it were user-established truth.

Root Cause

External memory providers may need different trust handling for writes that come from:

  • direct user memory commands;
  • ordinary assistant tool use;
  • background memory/skill review;
  • reflection/self-diagnosis paths;
  • other automated runtime paths.

Without structured origin metadata, a provider has to infer write provenance from text content or surrounding context. That is error-prone and can lead to assistant-authored self-diagnosis being stored as if it were user-established 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 #15315: fix(memory): add write origin metadata

Description (problem / solution / changelog)

What does this PR do?

Adds backwards-compatible write-origin metadata to external memory provider mirrors for built-in memory writes. Providers that opt into the new metadata argument can distinguish foreground assistant memory-tool writes from background review writes and memory-flush writes without parsing memory text.

The manager still supports existing third-party providers that implement the old 3-argument on_memory_write(action, target, content) hook.

Related Issue

Fixes #15219

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/memory_provider.py: adds optional metadata to MemoryProvider.on_memory_write(...).
  • agent/memory_manager.py: forwards metadata to opt-in providers while preserving old 3-arg provider compatibility.
  • run_agent.py: builds write provenance metadata for foreground memory tools, background review agents, and memory flush writes.
  • plugins/memory/*: updates bundled provider hook signatures; Supermemory includes the provenance in its memory metadata.
  • tests/agent/test_memory_provider.py and tests/run_agent/test_flush_memories_codex.py: cover opt-in metadata delivery, legacy compatibility, and flush provenance.

How to Test

  1. Targeted: ./scripts/run_tests.sh tests/agent/test_memory_provider.py tests/run_agent/test_flush_memories_codex.py tests/plugins/memory/test_supermemory_provider.py tests/plugins/test_retaindb_plugin.py -n 4
  2. Result: 166 passed in 7.35s.
  3. Full suite: ./scripts/run_tests.sh tests/ -n 4
  4. Result: 54 failed, 15434 passed, 40 skipped, 185 warnings in 306.60s. The failures are broad pre-existing repo/environment failures in gateway, Dingtalk, Codex response, backup/profile wrapper, and Tirith tests, not in the targeted memory-provider coverage.

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix/feature
  • I have run pytest tests/ -q and all tests pass
  • I have added tests for my changes
  • I have tested on my platform: Ubuntu/WSL2-style Linux workspace

Documentation & Housekeeping

  • I have updated relevant documentation/docstrings, or N/A
  • I have updated cli-config.yaml.example if I added/changed config keys, or N/A
  • I have updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows, or N/A
  • I have considered cross-platform impact, or N/A
  • I have updated tool descriptions/schemas if I changed tool behavior, or N/A

Screenshots / Logs

Targeted memory-provider tests pass. Full suite currently has unrelated baseline failures listed above.

Changed files

  • agent/memory_manager.py (modified, +43/-2)
  • agent/memory_provider.py (modified, +12/-3)
  • plugins/memory/byterover/__init__.py (modified, +7/-1)
  • plugins/memory/holographic/__init__.py (modified, +8/-2)
  • plugins/memory/honcho/__init__.py (modified, +7/-1)
  • plugins/memory/openviking/__init__.py (modified, +7/-1)
  • plugins/memory/retaindb/__init__.py (modified, +7/-1)
  • plugins/memory/supermemory/__init__.py (modified, +15/-2)
  • run_agent.py (modified, +52/-0)
  • tests/agent/test_memory_provider.py (modified, +52/-0)
  • tests/run_agent/test_flush_memories_codex.py (modified, +25/-0)

PR #15346: fix(memory): add write origin metadata seam

Description (problem / solution / changelog)

Salvage of #15315 by @helix4u — core seam only, bundled plugins untouched.

Summary

External memory providers can now opt into structured write-origin metadata (write_origin, execution_context, session_id, parent_session_id, platform, tool_name, task_id, tool_call_id) by widening their on_memory_write to accept metadata=None. Resolves #15219.

Changes (5 files, +184/-5)

  • agent/memory_provider.py: ABC widened with optional metadata kwarg.
  • agent/memory_manager.py: signature sniffer routes each provider to keyword / positional / legacy branch — legacy 3-arg providers keep working unchanged.
  • run_agent.py: _build_memory_write_metadata() helper + provenance wiring at 3 call sites (foreground memory tool, background-review forked agent, flush_memories).
  • Tests for opt-in metadata delivery, legacy 3-arg compat, and flush provenance.

Not changed

Bundled memory providers (byterover, holographic, honcho, openviking, retaindb, supermemory) stay on their 3-arg signature. The sniffer routes all of them to the legacy branch — zero behavior change. Third-party providers opt in by adding metadata=None when they have a use case.

Validation

BeforeAfter
Opt-in provider receives provenanceno hook{write_origin, execution_context, session_id, parent_session_id, platform, tool_name, ...}
Legacy 3-arg providerworksworks (sniffer → legacy branch)
Bundled plugins3-arg3-arg, unchanged

Targeted tests: 71/71 pass (tests/agent/test_memory_provider.py, tests/run_agent/test_flush_memories_codex.py). Plugin tests: 187/188 pass; the 1 failure is pre-existing on main (hindsight setup test, unrelated).

Closes #15219.

Changed files

  • agent/memory_manager.py (modified, +43/-2)
  • agent/memory_provider.py (modified, +12/-3)
  • run_agent.py (modified, +52/-0)
  • tests/agent/test_memory_provider.py (modified, +52/-0)
  • tests/run_agent/test_flush_memories_codex.py (modified, +25/-0)

Code Example

def on_memory_write(self, action: str, target: str, content: str) -> None:
    """Called when the built-in memory tool writes an entry."""

---

provider.on_memory_write(action, target, content)

---

self._memory_manager.on_memory_write(
    function_args.get("action", ""),
    target,
    function_args.get("content", ""),
)

---

def on_memory_write(
    self,
    action: str,
    target: str,
    content: str,
    metadata: dict[str, Any] | None = None,
) -> None:
    ...
RAW_BUFFERClick to expand / collapse

Summary

External memory providers currently receive built-in memory writes without structured write-origin/provenance metadata. This makes it difficult for providers to distinguish user-authored durable memory from background/reflection/assistant-generated memory writes without brittle text heuristics.

This is a feature request for a small backwards-compatible metadata seam on MemoryProvider.on_memory_write(...) and MemoryManager.on_memory_write(...).

Why this matters

External memory providers may need different trust handling for writes that come from:

  • direct user memory commands;
  • ordinary assistant tool use;
  • background memory/skill review;
  • reflection/self-diagnosis paths;
  • other automated runtime paths.

Without structured origin metadata, a provider has to infer write provenance from text content or surrounding context. That is error-prone and can lead to assistant-authored self-diagnosis being stored as if it were user-established truth.

Current behavior in vanilla Hermes

agent/memory_provider.py exposes:

def on_memory_write(self, action: str, target: str, content: str) -> None:
    """Called when the built-in memory tool writes an entry."""

agent/memory_manager.py forwards only (action, target, content):

provider.on_memory_write(action, target, content)

run_agent.py bridges built-in memory writes to the memory manager without origin/trust metadata:

self._memory_manager.on_memory_write(
    function_args.get("action", ""),
    target,
    function_args.get("content", ""),
)

Requested behavior

Add an optional metadata argument, ideally backwards-compatible:

def on_memory_write(
    self,
    action: str,
    target: str,
    content: str,
    metadata: dict[str, Any] | None = None,
) -> None:
    ...

Useful metadata fields could include:

  • write_origin, e.g. user_direct, assistant_tool, background_review, reflection, etc.;
  • session_id / turn id where available;
  • whether the write was produced by background review or ordinary foreground execution.

Suggested acceptance criteria

  • Existing providers without the metadata parameter continue to work.
  • Providers that accept metadata receive structured write-origin information.
  • Background memory/skill review writes are distinguishable from foreground user-directed writes.
  • The metadata seam remains generic and not tied to a specific external provider.

extent analysis

TL;DR

Add an optional metadata argument to the on_memory_write method to provide structured write-origin information.

Guidance

  • Modify the on_memory_write method in agent/memory_provider.py and agent/memory_manager.py to accept an optional metadata dictionary.
  • Update run_agent.py to pass relevant metadata when calling on_memory_write, such as write_origin and session_id.
  • Ensure existing providers without the metadata parameter continue to work by making the metadata argument optional.
  • Test the changes to verify that providers receive structured write-origin information and can distinguish between different types of writes.

Example

def on_memory_write(
    self,
    action: str,
    target: str,
    content: str,
    metadata: dict[str, Any] | None = None,
) -> None:
    if metadata and 'write_origin' in metadata:
        # Handle write-origin specific logic
        pass

Notes

The proposed change is backwards-compatible, allowing existing providers to continue working without modification. However, providers that accept metadata will need to be updated to handle the new information.

Recommendation

Apply workaround by adding the optional metadata argument to the on_memory_write method, as this will provide the necessary structured write-origin information without breaking existing functionality.

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…
#prompt formatting#chain error#conversation history#tool integration#LLM response

Still need to ship something?

×6

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

Back to top recommendations

TRENDING