fix(cli): stop shredding an existing MEMORY.md on hermes import-agent

memories/MEMORY.md is the "§"-delimited store written by MemoryStore, not a
markdown document. parse_existing_memory_entries() fell back to
extract_markdown_entries() -- the *source* parser for CLAUDE.md / AGENTS.md --
whenever the destination held no delimiter, which is exactly the case for a
single-entry store or one that was hand-edited or shell-appended. That
extractor skips fenced code blocks, skips table rows, splits a block into one
entry per bullet and reflows paragraphs. The shredded result was then written
straight back over the user's store and reported as "Imported", with no backup
to recover from.

Parse the destination the way MemoryStore._parse_entries does: split on
ENTRY_DELIMITER only, so a store with no delimiter is one intact entry.
extract_markdown_entries() is unchanged and still used on the sources, where
it is correct.

Also restore the safety net the port dropped. The openclaw migration script
this module was ported from calls maybe_backup(destination) before rewriting a
memory store; the port did not. Snapshot the store to <name>.bak.<unix_ts>
(same naming as MemoryStore._backup_drifted_file), refuse to rewrite when the
snapshot fails, and write via temp file + atomic rename so an interrupted
import cannot leave a truncated store and a symlinked MEMORY.md stays a
symlink.

The identical fallback lives in openclaw_to_hermes.py, where it is reached
from migrate_memory() (memories/MEMORY.md and memories/USER.md) and
migrate_daily_memory(); fixed there too.
This commit is contained in:
briandevans 2026-07-27 16:58:29 -07:00 committed by kshitij
parent 23e44a2843
commit 8a9ab8b56b
4 changed files with 192 additions and 8 deletions

View File

@ -40,12 +40,17 @@ from __future__ import annotations
import json
import logging
import os
import re
import shutil
import sys
import tempfile
import time
from pathlib import Path
from typing import Any, Dict, List, Optional, Sequence, Tuple
from utils import atomic_replace
logger = logging.getLogger(__name__)
# Same entry delimiter as the Hermes memory store and the openclaw migration
@ -205,14 +210,68 @@ def extract_markdown_entries(text: str) -> List[str]:
def parse_existing_memory_entries(path: Path) -> List[str]:
"""Parse the DESTINATION memory store into entries.
``memories/MEMORY.md`` is the entry-delimited store written by
``MemoryStore._write_file`` (tools/memory_tool.py), not a markdown
document, so this splits on ``ENTRY_DELIMITER`` only exactly what
``MemoryStore._parse_entries`` does. A store with no delimiter (a single
entry, or one that was hand-edited / shell-appended) is therefore ONE
intact entry.
Do NOT fall back to :func:`extract_markdown_entries` here. That extractor
is correct for CLAUDE.md / AGENTS.md *sources*, but it drops fenced code
blocks and table rows and splits a block into one entry per bullet and
the merged result is written straight back over the user's store, so the
loss is permanent.
"""
if not path.exists():
return []
raw = read_text(path)
if not raw.strip():
return []
if ENTRY_DELIMITER in raw:
return [e.strip() for e in raw.split(ENTRY_DELIMITER) if e.strip()]
return extract_markdown_entries(raw)
return [e.strip() for e in raw.split(ENTRY_DELIMITER) if e.strip()]
def backup_memory_file(path: Path) -> Optional[Path]:
"""Snapshot ``path`` before a destructive rewrite; return the backup path.
Restores parity with the openclaw migration script this module was ported
from, which calls ``maybe_backup(destination)`` before rewriting a memory
store. Uses the same ``<name>.bak.<unix_ts>`` naming as
``MemoryStore._backup_drifted_file``. Returns None when there is nothing
to back up.
"""
if not path.exists():
return None
backup = path.with_suffix(path.suffix + f".bak.{int(time.time())}")
shutil.copy2(path, backup)
return backup
def atomic_write_text(path: Path, content: str) -> None:
"""Write ``content`` to ``path`` via temp file + atomic rename.
Mirrors ``MemoryStore._write_file``: an interrupted or failed write can
never leave a truncated memory store on disk, and readers always see
either the old complete file or the new one. ``atomic_replace`` also
keeps a symlinked destination a symlink.
"""
fd, tmp_path = tempfile.mkstemp(
dir=str(path.parent), suffix=".tmp", prefix=".import_"
)
try:
with os.fdopen(fd, "w", encoding="utf-8") as handle:
handle.write(content)
handle.flush()
os.fsync(handle.fileno())
atomic_replace(tmp_path, path)
except BaseException:
try:
os.unlink(tmp_path)
except OSError:
pass
raise
def merge_entries(
@ -513,9 +572,19 @@ class AgentImporter:
return
if self.execute:
destination.parent.mkdir(parents=True, exist_ok=True)
destination.write_text(
try:
backup = backup_memory_file(destination)
except OSError as exc:
# Never rewrite the store when the safety net failed.
self.record(kind, source, destination, "error",
f"Could not back up existing memory file: {exc}",
**details)
return
if backup is not None:
details["backup"] = str(backup)
atomic_write_text(
destination,
ENTRY_DELIMITER.join(merged) + ("\n" if merged else ""),
encoding="utf-8",
)
self.record(kind, source, destination, "imported", **details)
else:

View File

@ -450,14 +450,21 @@ def rebrand_text(text: str) -> str:
def parse_existing_memory_entries(path: Path) -> List[str]:
"""Parse a DESTINATION Hermes memory store (memories/MEMORY.md, USER.md).
Splits on ``ENTRY_DELIMITER`` only, matching ``MemoryStore._parse_entries``
in ``tools/memory_tool.py``: a store with no delimiter is ONE intact entry.
Do NOT fall back to :func:`extract_markdown_entries` it is the *source*
parser (workspace/MEMORY.md and friends), it drops fenced code blocks and
table rows and splits a block into one entry per bullet, and the merged
result is written back over the destination.
"""
if not path.exists():
return []
raw = read_text(path)
if not raw.strip():
return []
if ENTRY_DELIMITER in raw:
return [e.strip() for e in raw.split(ENTRY_DELIMITER) if e.strip()]
return extract_markdown_entries(raw)
return [e.strip() for e in raw.split(ENTRY_DELIMITER) if e.strip()]
def extract_markdown_entries(text: str) -> List[str]:

View File

@ -17,11 +17,13 @@ import pytest
import yaml
from hermes_cli.agent_import import (
ENTRY_DELIMITER,
AgentImporter,
claude_rule_to_command_pattern,
detect_agents,
extract_markdown_entries,
is_secret_key,
parse_existing_memory_entries,
sanitize_mcp_env,
)
@ -486,6 +488,86 @@ class TestMergeSemantics:
assert memory_items[0]["status"] == "skipped"
# ---------------------------------------------------------------------------
# The DESTINATION memories/MEMORY.md is a §-delimited store, not a document
# ---------------------------------------------------------------------------
# A realistic hand-edited store: one entry, no "§", with a fenced code block
# and a markdown table — exactly the content extract_markdown_entries() drops.
EXISTING_MEMORY = """Homelab runbook. Restart the ingress controller with:
```bash
kubectl -n ingress rollout restart deploy/nginx
```
Escalation ladder:
| Severity | Contact | Window |
|----------|---------|--------|
| SEV1 | on-call | 15m |
| SEV2 | #ops | 4h |
Never page for SEV3.
"""
class TestExistingMemoryStorePreserved:
"""An import must not shred the memory store it merges into.
``memories/MEMORY.md`` is the entry-delimited store written by
``MemoryStore._write_file``; a single-entry or hand-edited store contains
no ``§`` delimiter. Parsing it with the *source* markdown extractor drops
code blocks and table rows and splits one entry into fragments, and the
merged result is written straight back over the file.
"""
@pytest.fixture()
def seeded_home(self, hermes_home):
memory = hermes_home / "memories" / "MEMORY.md"
memory.parent.mkdir(parents=True, exist_ok=True)
memory.write_text(EXISTING_MEMORY, encoding="utf-8")
return hermes_home
def test_undelimited_store_is_one_entry(self, seeded_home):
path = seeded_home / "memories" / "MEMORY.md"
assert ENTRY_DELIMITER not in path.read_text(encoding="utf-8")
assert parse_existing_memory_entries(path) == [EXISTING_MEMORY.strip()]
def test_agrees_with_memory_store_parser(self, seeded_home):
from tools.memory_tool import MemoryStore
path = seeded_home / "memories" / "MEMORY.md"
raw = path.read_text(encoding="utf-8")
assert parse_existing_memory_entries(path) == MemoryStore._parse_entries(raw)
def test_import_preserves_existing_entry_verbatim(
self, claude_tree, seeded_home):
path = seeded_home / "memories" / "MEMORY.md"
run_import("claude-code", claude_tree, seeded_home, execute=True)
entries = path.read_text(encoding="utf-8").split(ENTRY_DELIMITER)
# The pre-existing store survives byte-intact as a SINGLE entry ...
assert entries[0] == EXISTING_MEMORY.strip()
# ... including the parts the markdown extractor would have dropped.
assert "kubectl -n ingress rollout restart deploy/nginx" in entries[0]
assert "| SEV1 | on-call | 15m |" in entries[0]
# ... and the imported entries are still appended after it.
assert any("type hints" in e for e in entries[1:])
def test_import_backs_up_the_previous_store(self, claude_tree, seeded_home):
memories = seeded_home / "memories"
run_import("claude-code", claude_tree, seeded_home, execute=True)
backups = sorted(memories.glob("MEMORY.md.bak.*"))
assert len(backups) == 1
assert backups[0].read_text(encoding="utf-8") == EXISTING_MEMORY
def test_dry_run_leaves_the_store_untouched(self, claude_tree, seeded_home):
memories = seeded_home / "memories"
run_import("claude-code", claude_tree, seeded_home, execute=False)
assert (memories / "MEMORY.md").read_text(
encoding="utf-8") == EXISTING_MEMORY
assert not list(memories.glob("MEMORY.md.bak.*"))
# ---------------------------------------------------------------------------
# CLI wiring
# ---------------------------------------------------------------------------

View File

@ -56,6 +56,32 @@ def test_extract_markdown_entries_promotes_heading_context():
assert "Tyler Williams > Active Projects: Hermes Agent" in entries
def test_parse_existing_memory_entries_keeps_undelimited_store_intact(tmp_path):
"""The DESTINATION store is §-delimited, not a markdown document.
``migrate_memory`` and ``migrate_daily_memory`` read the Hermes-side
memories/MEMORY.md (and USER.md) and write the merged result back over it.
A store with no delimiter is ONE entry running the source markdown
extractor over it would drop the code block and the table row below.
"""
mod = load_module()
raw = (
"Homelab runbook. Restart the ingress controller with:\n"
"\n"
"```bash\n"
"kubectl -n ingress rollout restart deploy/nginx\n"
"```\n"
"\n"
"| Severity | Contact | Window |\n"
"| SEV1 | on-call | 15m |\n"
)
path = tmp_path / "MEMORY.md"
path.write_text(raw, encoding="utf-8")
assert mod.ENTRY_DELIMITER not in raw
assert mod.parse_existing_memory_entries(path) == [raw.strip()]
def test_merge_entries_respects_limit_and_reports_overflow():
mod = load_module()
existing = ["alpha"]