diff --git a/hermes_cli/agent_import.py b/hermes_cli/agent_import.py index bd686e59348ce..38e743a1fa33b 100644 --- a/hermes_cli/agent_import.py +++ b/hermes_cli/agent_import.py @@ -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 ``.bak.`` 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: diff --git a/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py b/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py index 7f6e9b3fb52e9..62cae3f6d8b91 100644 --- a/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py +++ b/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py @@ -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]: diff --git a/tests/hermes_cli/test_agent_import.py b/tests/hermes_cli/test_agent_import.py index b2b8dd462115b..268121e777bbf 100644 --- a/tests/hermes_cli/test_agent_import.py +++ b/tests/hermes_cli/test_agent_import.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tests/skills/test_openclaw_migration.py b/tests/skills/test_openclaw_migration.py index e75dad8792a2a..bc7ee92e25a15 100644 --- a/tests/skills/test_openclaw_migration.py +++ b/tests/skills/test_openclaw_migration.py @@ -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"]