From 208ba7cc657a5f0c26f0281040a3d1e58f51bfe6 Mon Sep 17 00:00:00 2001 From: Kris <2040188027@qq.com> Date: Sat, 19 Sep 2026 10:38:22 +0800 Subject: [PATCH] feat(memory): opt-in tolerant MarkdownMemoryStorage (#3124) (#5545) * feat(memory): add opt-in tolerant MarkdownMemoryStorage (fixes #3124) User-memory summary load is now tolerant of corrupt/partially written files: a Markdown summary (fenced `memory-json` block, best-effort structured fallback) or JSON is accepted, and an unrecoverable file recovers to an empty memory instead of raising MemoryStorageCorruption and taking down the agent. On-disk JSON format and the JSON UI are unchanged, so enabling `memory.storage_class: markdown` is fully opt-in and cannot break existing deployments. Co-authored-by: WorkBuddy * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(markdown-memory): address review - schema-safe loader, greedy fence, quarantine, CI tests - drop lossy structured Markdown fallback (invalid-shape crash on load/save) - greedy fence match: ``` inside remembered strings round-trips losslessly - quarantine unreadable summary before returning None (no silent erase) - remove dead _render_memory_markdown (deferred to write-path change) - move tests to backend/tests/ so CI runs them; update storage_class docs * fix(memory): correct tolerant Markdown parsing and regression tests --------- Co-authored-by: WorkBuddy Co-authored-by: Willem Jiang --- README.md | 7 ++ .../harness/deerflow/agents/memory/AGENTS.md | 8 ++ .../memory/backends/deermem/deermem/config.py | 7 +- .../deermem/deermem/core/markdown_format.py | 56 ++++++++++ .../deermem/deermem/core/markdown_storage.py | 102 ++++++++++++++++++ .../backends/deermem/deermem/core/storage.py | 4 + backend/tests/test_memory_storage.py | 102 ++++++++++++++++++ 7 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_format.py create mode 100644 backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_storage.py diff --git a/README.md b/README.md index 2ba961540..bd3a5b098 100644 --- a/README.md +++ b/README.md @@ -1640,6 +1640,13 @@ and the [request contract](backend/docs/API.md#referencing-a-previous-conversati ### Long-Term Memory +For DeerMem, `memory.backend_config.storage_class: markdown` opts into tolerant +summary reads while keeping JSON writes and the existing UI. A hand-edited +`memory.json` can contain its JSON object inside a fenced `memory-json` block; +embedded backticks and later fenced notes are supported. Unparseable summary +text is moved to `memory.json.corrupt-` for recovery before rebuilding. +`storage_path` must point to the data root directory, not an existing JSON file. + Gateway shutdown drains memory updates before closing the backend, even when shutdown is cancelled. Config reload failures are logged without aborting runtime teardown. For Kubernetes, budget `terminationGracePeriodSeconds` for all shutdown diff --git a/backend/packages/harness/deerflow/agents/memory/AGENTS.md b/backend/packages/harness/deerflow/agents/memory/AGENTS.md index 29beccb2d..fb6dc5433 100644 --- a/backend/packages/harness/deerflow/agents/memory/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/memory/AGENTS.md @@ -84,6 +84,14 @@ existing enabled behavior. #### DeerMem storage contract +`memory.backend_config.storage_class: markdown` opts into tolerant summary +reads; writes still use JSON. Only a JSON object in a closed `memory-json` +fence is accepted from Markdown. Decode the value before checking its closing +fence so embedded backticks and later fenced notes stay lossless. Parser and +storage regressions belong in `tests/test_memory_storage.py`; configure a data +root directory, not the existing manifest file. Legacy v1 reads still migrate +and advance the revision. Unparseable summary text is quarantined before rebuild. + `FileMemoryStorage` owns canonical storage and the retrieval adapter. Do not reach into its private adapter state from higher layers. diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/config.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/config.py index 04ce1d194..e849953d8 100644 --- a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/config.py +++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/config.py @@ -53,7 +53,12 @@ class DeerMemConfig(BaseModel): ) storage_class: str = Field( default="", - description="Dotted class path for an alternative storage provider; empty (default) = FileMemoryStorage (no importlib, portable).", + description=( + "Dotted class path for an alternative storage provider, or a built-in alias: " + "``file`` = FileMemoryStorage (default) or ``markdown`` = MarkdownMemoryStorage " + "(tolerant load path, same JSON on disk); empty (default) = FileMemoryStorage " + "(no importlib, portable)." + ), ) strict_user_scope: bool = Field( default=False, diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_format.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_format.py new file mode 100644 index 000000000..e367d5725 --- /dev/null +++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_format.py @@ -0,0 +1,56 @@ +"""Markdown-aware parsing for DeerMem user-memory summaries. + +This module is intentionally dependency-free so it can be unit-tested and +imported without the rest of the DeerMem stack. + +Design (read path only) +----------------------- +A Markdown summary carries its *lossless* state inside a fenced +```` ```memory-json ```` block. When loading, the fenced JSON block is the +only trusted Markdown representation: if it is present and parses to a JSON +object it is returned verbatim; anything else (no fence, malformed fence, +non-object JSON) yields ``None`` so the caller can decide policy (the +default is to quarantine the unreadable file rather than silently rebuild +over persistent state). + +The JSON decoder locates the end of the value before the closing fence is +checked. Backticks inside remembered strings cannot truncate the value, and +later fenced notes cannot be accidentally consumed as part of the JSON. + +A lossy structured parse of the human-readable sections is deliberately NOT +provided: it cannot reproduce the manifest schema (``user``/``history`` must +be objects, ``version``/``revision`` scalars) and previously surfaced as +``ValueError``/``AttributeError`` crashes on the very hand-edited files the +loader claimed to tolerate. Rendering Markdown is deferred to a future +write-path change. +""" + +from __future__ import annotations + +import json +import re +from typing import Any + +_OPEN_FENCE_RE = re.compile(r"```memory-json[ \t]*\r?\n") +_CLOSE_FENCE_RE = re.compile(r"[ \t\r\n]*\r?\n[ \t]*```[ \t]*(?:\r?\n|$)") + + +def _parse_markdown_memory(raw: str) -> dict[str, Any] | None: + """Parse a Markdown summary into a dict, or None when nothing usable. + + Only the fenced ```` ```memory-json ```` block is trusted. There is no + structured fallback: without a valid fenced block the file cannot be + mapped onto the manifest schema losslessly, so returning ``None`` (the + caller quarantines and starts fresh) is the honest outcome. + """ + opening = _OPEN_FENCE_RE.search(raw) + if opening is None: + return None + payload = raw[opening.end() :].lstrip() + try: + value, end = json.JSONDecoder().raw_decode(payload) + except json.JSONDecodeError: + return None + if _CLOSE_FENCE_RE.match(payload, end) is None: + return None + return value if isinstance(value, dict) else None diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_storage.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_storage.py new file mode 100644 index 000000000..443ef9e3f --- /dev/null +++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/markdown_storage.py @@ -0,0 +1,102 @@ +"""Opt-in Markdown-aware summary storage for DeerMem. + +The default :class:`FileMemoryStorage` persists the user-memory *summary* as a +single JSON document. Reasoning/thinking models occasionally emit malformed +JSON, and a partially written summary historically raised +``MemoryStorageCorruption`` and took down the whole agent. + +``MarkdownMemoryStorage`` keeps the same on-disk JSON as the default for full +backward compatibility, but its loader is *tolerant*: + +* a corrupt or partially written summary no longer crashes the agent; +* a Markdown summary is accepted only through its fenced ```` ```memory-json ```` + block, which is parsed losslessly; +* when the on-disk file is unreadable (neither valid JSON nor a Markdown + summary with a usable fenced block), it is *quarantined* as + ``memory.json.corrupt-`` before the loader returns ``None``. + Quarantining keeps the content recoverable: returning ``None`` alone would + make the next :meth:`save` rebuild the manifest from scratch (revision + reset, no journal backup) and silently erase the unreadable state. + +Writes still persist JSON (the write path is untouched). Hand-edited +Markdown files are therefore a *read-time* convenience: the next write +rewrites ``memory.json`` as JSON, so the Markdown rendering is temporary +until a Markdown write path lands. + +This is intentionally a small, additive change scoped to the load path only: +the JSON UI and all other backends are untouched. Enabling it cannot break +existing deployments. +""" + +from __future__ import annotations + +import json +from datetime import datetime +from pathlib import Path +from typing import Any + +from .markdown_format import _parse_markdown_memory +from .storage import FileMemoryStorage, logger + + +class MarkdownMemoryStorage(FileMemoryStorage): + """File-backed storage whose summary loader tolerates corrupt/Markdown. + + Fully opt-in. Enable via ``memory.storage_class: markdown`` (or the full + import path ``deerflow.agents.memory.backends.deermem.deermem.core. + markdown_storage.MarkdownMemoryStorage``). The default JSON summary + format is unchanged, so the existing JSON UI and all other backends keep + working. + """ + + def _load_memory_file(self, path: Path) -> dict[str, Any] | None: + if not path.exists(): + return None + try: + raw = path.read_text(encoding="utf-8") + except (OSError, UnicodeError) as exc: + logger.warning("Cannot read memory summary %s: %s", path, exc) + return None + + parsed: dict[str, Any] | None = None + try: + value = json.loads(raw) + except json.JSONDecodeError: + value = None + if isinstance(value, dict): + parsed = value + else: + # Not a JSON object: the opt-in Markdown summary is accepted only + # through its lossless fenced ```memory-json block. No structured + # fallback -- a best-effort section parse cannot reproduce the + # manifest schema (object-shaped user/history) and previously + # crashed load()/save() on the files it claimed to tolerate. + parsed = _parse_markdown_memory(raw) + + if parsed is not None: + return parsed + + logger.warning( + "Memory summary %s is unreadable (neither valid JSON nor a Markdown summary with a usable ```memory-json block); quarantining the file so its content stays recoverable instead of being silently overwritten by the next save.", + path, + ) + self._quarantine_unreadable(path) + return None + + @staticmethod + def _quarantine_unreadable(path: Path) -> None: + """Move an unreadable summary aside; the next save rebuilds from scratch. + + Without this, returning ``None`` would let ``_commit_changes_locked`` + rebuild the manifest from ``create_empty_memory()`` and skip its + recovery backup (which only runs when a current memory exists), + silently destroying the previous state. + """ + stamp = datetime.now().strftime("%Y%m%d-%H%M%S-%f") + target = path.with_name(f"{path.name}.corrupt-{stamp}") + try: + path.replace(target) + logger.warning("Quarantined unreadable memory summary as %s", target) + except OSError as exc: + # Quarantine is best-effort: keep the tolerant-read guarantee. + logger.error("Could not quarantine unreadable memory summary %s: %s", path, exc) diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py index 397434641..cdda9e161 100644 --- a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py +++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/storage.py @@ -1843,6 +1843,10 @@ def create_storage(config: DeerMemConfig, retrieval: RetrievalPort | None = None storage_class_path = config.storage_class if not storage_class_path or storage_class_path == "file": return FileMemoryStorage(config, retrieval=retrieval) + if storage_class_path == "markdown": + from .markdown_storage import MarkdownMemoryStorage + + return MarkdownMemoryStorage(config, retrieval=retrieval) try: module_path, class_name = storage_class_path.rsplit(".", 1) storage_class = getattr(importlib.import_module(module_path), class_name) diff --git a/backend/tests/test_memory_storage.py b/backend/tests/test_memory_storage.py index f7a4a83fa..fd8dbcbd5 100644 --- a/backend/tests/test_memory_storage.py +++ b/backend/tests/test_memory_storage.py @@ -1,11 +1,14 @@ """Tests for memory storage providers (DI: FileMemoryStorage(config) / create_storage).""" +import json import threading from unittest.mock import patch import pytest from deerflow.agents.memory.backends.deermem.deermem.config import DeerMemConfig +from deerflow.agents.memory.backends.deermem.deermem.core import markdown_format as mf +from deerflow.agents.memory.backends.deermem.deermem.core.markdown_storage import MarkdownMemoryStorage from deerflow.agents.memory.backends.deermem.deermem.core.paths import validate_agent_name from deerflow.agents.memory.backends.deermem.deermem.core.storage import ( FileMemoryStorage, @@ -178,3 +181,102 @@ class TestCreateStorage: def test_dotted_storage_class_resolves(self): storage = create_storage(DeerMemConfig(storage_class="deerflow.agents.memory.backends.deermem.deermem.core.storage.FileMemoryStorage")) assert isinstance(storage, FileMemoryStorage) + + +class TestMarkdownMemoryStorage: + """Opt-in ``storage_class="markdown"``: tolerant load path (issue #3124).""" + + def _markdown_storage_at(self, memory_file) -> MarkdownMemoryStorage: + return MarkdownMemoryStorage(DeerMemConfig(storage_path=str(memory_file.parent.resolve()))) + + def test_markdown_alias_resolves(self): + storage = create_storage(DeerMemConfig(storage_class="markdown")) + assert isinstance(storage, MarkdownMemoryStorage) + assert isinstance(storage, FileMemoryStorage) + + @pytest.mark.parametrize(("version", "expected_revision"), [("1.0", 8), ("2.0", 7)]) + def test_corrupt_json_with_fenced_block_recovers(self, tmp_path, version, expected_revision): + """A partially written JSON summary that still carries a fenced + ```memory-json block is recovered losslessly instead of crashing.""" + memory_file = tmp_path / "memory.json" + manifest = create_empty_memory() + manifest["version"] = version + if version == "2.0": + manifest.pop("facts") # v2 manifests keep facts in separate files. + manifest["revision"] = 7 + manifest["user"]["workContext"] = {"summary": "recovered"} + fenced = '{"version": 1, "revision": 7, "user": {"lang": "zh"}\n```memory-json\n' + json.dumps(manifest, ensure_ascii=False) + "\n```" + memory_file.write_text(fenced, encoding="utf-8") + storage = self._markdown_storage_at(memory_file) + loaded = storage.load() + # Legacy manifests migrate to v2 on load and advance the revision. + assert loaded["version"] == "2.0" + assert loaded["revision"] == expected_revision + assert loaded["user"]["workContext"]["summary"] == "recovered" + + def test_hand_edited_markdown_without_fence_is_quarantined(self, tmp_path): + """Markdown without a fenced block cannot be mapped onto the manifest + schema losslessly: load() must not crash AND must not return an + invalid shape -- the file is quarantined so nothing is silently lost.""" + memory_file = tmp_path / "memory.json" + body = "# DeerFlow Memory\n\n- version: 2\n- revision: 5\n\n## User\n- summary: likes tea\n" + memory_file.write_text(body, encoding="utf-8") + storage = self._markdown_storage_at(memory_file) + loaded = storage.load() # must not raise + assert isinstance(loaded, dict) + quarantined = list(tmp_path.glob("memory.json.corrupt-*")) + assert len(quarantined) == 1, "unreadable file must be preserved via quarantine" + assert quarantined[0].read_text(encoding="utf-8") == body + + def test_truncated_json_is_quarantined_before_rebuild(self, tmp_path): + """A truncated manifest must not be silently erased by the next save: + the unreadable file is quarantined (revision reset is then safe).""" + memory_file = tmp_path / "memory.json" + truncated = '{"version": "2.0", "revision": 41, "user": {"work' + memory_file.write_text(truncated, encoding="utf-8") + storage = self._markdown_storage_at(memory_file) + loaded = storage.load() + assert isinstance(loaded, dict) + assert storage.save(create_empty_memory()) is True + quarantined = list(tmp_path.glob("memory.json.corrupt-*")) + assert len(quarantined) == 1 + assert quarantined[0].read_text(encoding="utf-8") == truncated + + def test_fenced_block_containing_backticks_parses_losslessly(self): + """Remembered code snippets must not terminate the JSON block.""" + manifest = create_empty_memory() + manifest["user"]["workContext"] = {"summary": "prefers ```python\nprint('hi')\n``` snippets"} + rendered = "# DeerFlow Memory\n\n```memory-json\n" + json.dumps(manifest, ensure_ascii=False) + "\n```\n" + parsed = mf._parse_markdown_memory(rendered) + assert parsed == manifest + + @pytest.mark.parametrize("newline", ["\n", "\r\n"]) + def test_fenced_summary_with_trailing_code_block_loads_without_quarantine(self, tmp_path, newline): + manifest = create_empty_memory() + manifest["version"] = "2.0" + manifest.pop("facts") + manifest["revision"] = 7 + manifest["user"]["workContext"]["summary"] = "prefers ```python snippets```" + body = newline.join(["# Memory", "```memory-json", json.dumps(manifest, indent=2), "", "```", "Notes:", "```python", "print('example')", "```", ""]) + memory_file = tmp_path / "memory.json" + memory_file.write_text(body, encoding="utf-8") + assert mf._parse_markdown_memory(body) == manifest + loaded = self._markdown_storage_at(memory_file).load() + assert loaded["revision"] == 7 + assert loaded["user"]["workContext"] == manifest["user"]["workContext"] + assert not list(tmp_path.glob("memory.json.corrupt-*")) + + @pytest.mark.parametrize( + "body", + [ + "# Memory without a fence", + '```memory-json\n{"user": ', + '```memory-json\n{"user": {}}', + '```memory-json\n{"user": {}} trailing garbage\n```', + '```memory-json\n{"user": {}}\n```python\nnotes\n```', + "```memory-json\n[]\n```", + "```memory-json\nnull\n```", + ], + ) + def test_invalid_or_non_object_fenced_summary_rejected(self, body): + assert mf._parse_markdown_memory(body) is None