From d2c3b5c28a1036215060a22c3a03aad8b7a6d618 Mon Sep 17 00:00:00 2001 From: Tyce Herrman Date: Sat, 25 Jul 2026 08:11:00 -0400 Subject: [PATCH 1/2] feat: add optional memory frontmatter tools Port and update the frontmatter work from oraios/serena#1119 for the current memory manager architecture. Co-authored-by: Mehdi Ait Kajaoud <102723977+Meh10t@users.noreply.github.com> --- CHANGELOG.md | 4 ++ src/serena/memories/frontmatter.py | 73 +++++++++++++++++++++++++ src/serena/memories/memory_manager.py | 47 ++++++++++++++-- src/serena/tools/memory_tools.py | 49 ++++++++++++++++- test/serena/test_frontmatter.py | 77 ++++++++++++++++++++++++++ test/serena/test_memories_manager.py | 79 +++++++++++++++++++++++++++ test/serena/test_memory_tools.py | 72 ++++++++++++++++++++++++ 7 files changed, 394 insertions(+), 7 deletions(-) create mode 100644 src/serena/memories/frontmatter.py create mode 100644 test/serena/test_frontmatter.py create mode 100644 test/serena/test_memory_tools.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ce43a05e9..ca7d6e253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ Status of the `main` branch. Changes prior to the next official version change w * JetBrains: - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file +* Memories: + - Add opt-in tools for reading and updating scalar frontmatter metadata while keeping `read_memory` + focused on memory body content + * Language Servers: - `typescript`: Fix: on large projects, the first `find_referencing_symbols`/`request_references` call could silently race tsserver's project load and return incomplete results, because the fixed 2s diff --git a/src/serena/memories/frontmatter.py b/src/serena/memories/frontmatter.py new file mode 100644 index 000000000..7f068795b --- /dev/null +++ b/src/serena/memories/frontmatter.py @@ -0,0 +1,73 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class FrontmatterParseResult: + frontmatter: dict[str, str] + body: str + + +class FrontmatterParser: + """Parser and renderer for simple scalar frontmatter fields.""" + + DELIMITER = "---" + + @staticmethod + def _validate_field(key: str, value: str) -> None: + if not key: + raise ValueError("Frontmatter key must not be empty") + if ":" in key: + raise ValueError("Frontmatter key must not contain ':'") + if "\n" in key or "\r" in key: + raise ValueError("Frontmatter key must be a single line") + if "\n" in value or "\r" in value: + raise ValueError("Frontmatter value must be a single line") + + @classmethod + def parse(cls, content: str) -> FrontmatterParseResult: + lines = content.splitlines(keepends=True) + if not lines or lines[0].rstrip("\r\n") != cls.DELIMITER: + return FrontmatterParseResult(frontmatter={}, body=content) + + closing_index: int | None = None + for index, line in enumerate(lines[1:], start=1): + if line.rstrip("\r\n") == cls.DELIMITER: + closing_index = index + break + + if closing_index is None: + return FrontmatterParseResult(frontmatter={}, body=content) + + frontmatter: dict[str, str] = {} + for line in lines[1:closing_index]: + field = line.rstrip("\r\n") + if not field: + continue + if ":" not in field: + return FrontmatterParseResult(frontmatter={}, body=content) + + key, value = field.split(":", 1) + key = key.strip() + if not key: + return FrontmatterParseResult(frontmatter={}, body=content) + + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in {'"', "'"}: + value = value[1:-1] + frontmatter[key] = value + + body = "".join(lines[closing_index + 1 :]) + return FrontmatterParseResult(frontmatter=frontmatter, body=body) + + @classmethod + def render(cls, frontmatter: dict[str, str], body: str) -> str: + if not frontmatter: + return body + + for key, value in frontmatter.items(): + cls._validate_field(key, value) + + fields = "\n".join(f"{key}: {value}" for key, value in frontmatter.items()) + return f"{cls.DELIMITER}\n{fields}\n{cls.DELIMITER}\n{body}" diff --git a/src/serena/memories/memory_manager.py b/src/serena/memories/memory_manager.py index daa037252..d96ec20fb 100644 --- a/src/serena/memories/memory_manager.py +++ b/src/serena/memories/memory_manager.py @@ -12,6 +12,7 @@ from serena.constants import SERENA_FILE_ENCODING from serena.util.text_utils import ContentReplacer +from .frontmatter import FrontmatterParser from .memory_reference_analysis import ( MEMORY_REF_PREFIX, AutofixReport, @@ -201,16 +202,16 @@ def _check_write_access(self, name: str, is_tool_context: bool) -> None: if is_tool_context and self._is_read_only_memory(name): raise PermissionError(f"Attempted to write to read_only memory: '{name}')") - def load_memory(self, name: str) -> str: + def _load_memory_raw(self, name: str) -> tuple[str, str]: name = self._sanitize_name(name) self._check_not_ignored(name) memory_file_path = self.get_memory_file_path(name) if not memory_file_path.exists(): raise FileNotFoundError(f"Memory named '{name}' not found") with open(memory_file_path, encoding=self._encoding) as f: - return f.read() + return name, f.read() - def save_memory(self, name: str, content: str, is_tool_context: bool) -> str: + def _save_memory_raw(self, name: str, content: str, is_tool_context: bool) -> str: name = self._sanitize_name(name) self._check_not_ignored(name) self._check_write_access(name, is_tool_context) @@ -219,6 +220,42 @@ def save_memory(self, name: str, content: str, is_tool_context: bool) -> str: f.write(content) return f"Memory {name} written." + def load_memory(self, name: str) -> str: + _, raw = self._load_memory_raw(name) + return FrontmatterParser.parse(raw).body + + def save_memory(self, name: str, content: str, is_tool_context: bool) -> str: + """ + Saves memory body content while retaining any existing frontmatter. + + New memories may still be created from raw content containing a frontmatter + block. Once frontmatter exists, callers that only know about the body cannot + accidentally discard it. + """ + name = self._sanitize_name(name) + self._check_not_ignored(name) + self._check_write_access(name, is_tool_context) + memory_file_path = self.get_memory_file_path(name) + if memory_file_path.exists(): + with open(memory_file_path, encoding=self._encoding) as f: + parsed = FrontmatterParser.parse(f.read()) + if parsed.frontmatter: + content = FrontmatterParser.render(parsed.frontmatter, content) + return self._save_memory_raw(name, content, is_tool_context) + + def get_memory_frontmatter(self, name: str) -> dict[str, str]: + _, raw = self._load_memory_raw(name) + return dict(FrontmatterParser.parse(raw).frontmatter) + + def add_memory_frontmatter(self, name: str, key: str, value: str, is_tool_context: bool) -> str: + name, raw = self._load_memory_raw(name) + self._check_write_access(name, is_tool_context) + parsed = FrontmatterParser.parse(raw) + frontmatter = dict(parsed.frontmatter) + frontmatter[key] = value + content = FrontmatterParser.render(frontmatter, parsed.body) + return self._save_memory_raw(name, content, is_tool_context) + class MemoriesList: def __init__(self) -> None: self.memories: list[str] = [] @@ -237,8 +274,8 @@ def extend(self, other: "MemoryManager.MemoriesList") -> None: self.memories.extend(other.memories) self.read_only_memories.extend(other.read_only_memories) - def to_dict(self) -> dict[str, list[str]]: - result = {} + def to_dict(self) -> dict[str, object]: + result: dict[str, object] = {} if self.memories: result["memories"] = sorted(self.memories) if self.read_only_memories: diff --git a/src/serena/tools/memory_tools.py b/src/serena/tools/memory_tools.py index 6ebffd9b1..bc896e4b4 100644 --- a/src/serena/tools/memory_tools.py +++ b/src/serena/tools/memory_tools.py @@ -1,7 +1,7 @@ import logging from typing import Literal -from serena.tools import Tool, ToolMarkerCanEdit +from serena.tools import Tool, ToolMarkerCanEdit, ToolMarkerOptional log = logging.getLogger(__name__) @@ -55,8 +55,53 @@ class ListMemoriesTool(Tool): def apply(self, topic: str = "") -> str: """ Lists available memories, optionally filtered by topic. + Includes frontmatter metadata when the optional memory_get_frontmatter + tool is active. """ - return self._to_json(self.memory_manager.list_memories(topic).to_dict()) + memories = self.memory_manager.list_memories(topic) + result = memories.to_dict() + + if MemoryGetFrontmatterTool.get_name_from_cls() in self.agent.get_active_tool_names(): + frontmatter = {} + for memory_name in memories.get_full_list(): + metadata = self.memory_manager.get_memory_frontmatter(memory_name) + if metadata: + frontmatter[memory_name] = metadata + if frontmatter: + result["frontmatter"] = frontmatter + + return self._to_json(result) + + +class MemoryGetFrontmatterTool(Tool, ToolMarkerOptional): + """ + Reads scalar frontmatter metadata from a memory. + + The metadata is stored in an optional block delimited by ``---`` lines at + the beginning of the memory file. + """ + + def apply(self, memory_name: str) -> str: + """ + Return the memory's frontmatter as JSON, or an empty object when absent. + + :param memory_name: memory name + """ + return self._to_json(self.memory_manager.get_memory_frontmatter(memory_name)) + + +class MemoryAddFrontmatterTool(Tool, ToolMarkerCanEdit, ToolMarkerOptional): + """Adds or updates one scalar frontmatter field in a memory.""" + + def apply(self, memory_name: str, key: str, value: str) -> str: + """ + Add or update a frontmatter field without changing the memory body. + + :param memory_name: memory name + :param key: frontmatter field name + :param value: scalar frontmatter value + """ + return self.memory_manager.add_memory_frontmatter(memory_name, key, value, is_tool_context=True) class DeleteMemoryTool(Tool, ToolMarkerCanEdit): diff --git a/test/serena/test_frontmatter.py b/test/serena/test_frontmatter.py new file mode 100644 index 000000000..961d834d1 --- /dev/null +++ b/test/serena/test_frontmatter.py @@ -0,0 +1,77 @@ +import pytest + +from serena.memories.frontmatter import FrontmatterParser + + +def test_parse_without_frontmatter_returns_content_unchanged() -> None: + content = "# Memory\n\nBody\n" + + result = FrontmatterParser.parse(content) + + assert result.frontmatter == {} + assert result.body == content + + +def test_parse_frontmatter_preserves_body_whitespace() -> None: + content = '---\nsummary: "Short description"\nurl: https://example.com:443/docs\n---\n\n# Memory\n\nBody\n' + + result = FrontmatterParser.parse(content) + + assert result.frontmatter == { + "summary": "Short description", + "url": "https://example.com:443/docs", + } + assert result.body == "\n# Memory\n\nBody\n" + + +def test_parse_empty_frontmatter() -> None: + result = FrontmatterParser.parse("---\n---\nBody\n") + + assert result.frontmatter == {} + assert result.body == "Body\n" + + +@pytest.mark.parametrize( + "content", + [ + "---\nsummary: missing closing delimiter\n", + "---\nnot a scalar field\n---\nBody\n", + "---\n: missing key\n---\nBody\n", + " ---\nsummary: indented opening delimiter\n---\nBody\n", + ], +) +def test_malformed_frontmatter_is_plain_content(content: str) -> None: + result = FrontmatterParser.parse(content) + + assert result.frontmatter == {} + assert result.body == content + + +def test_render_round_trip_is_stable() -> None: + frontmatter = { + "summary": "Short description", + "custom": "value:with:colons", + } + body = "\n# Memory\n\nBody with trailing whitespace.\n\n" + + rendered = FrontmatterParser.render(frontmatter, body) + parsed = FrontmatterParser.parse(rendered) + + assert parsed.frontmatter == frontmatter + assert parsed.body == body + assert FrontmatterParser.render(parsed.frontmatter, parsed.body) == rendered + + +@pytest.mark.parametrize( + ("frontmatter", "error"), + [ + ({"": "value"}, "must not be empty"), + ({"bad:key": "value"}, "must not contain"), + ({"bad\nkey": "value"}, "single line"), + ({"summary": "bad\nvalue"}, "single line"), + ({"summary": "bad\rvalue"}, "single line"), + ], +) +def test_render_rejects_non_scalar_fields(frontmatter: dict[str, str], error: str) -> None: + with pytest.raises(ValueError, match=error): + FrontmatterParser.render(frontmatter, "Body\n") diff --git a/test/serena/test_memories_manager.py b/test/serena/test_memories_manager.py index 158ab379a..f9aabe061 100644 --- a/test/serena/test_memories_manager.py +++ b/test/serena/test_memories_manager.py @@ -269,6 +269,85 @@ def _write(manager: MemoryManager, name: str, content: str) -> None: manager.save_memory(name, content, is_tool_context=False) +class TestMemoryFrontmatter: + def test_load_hides_frontmatter_and_preserves_body_whitespace(self, fs_manager: MemoryManager) -> None: + content = "---\nsummary: Notes\ncustom: value:with:colons\n---\n\n# Body\n\nTail\n" + _write(fs_manager, "notes", content) + + assert fs_manager.get_memory_frontmatter("notes") == { + "summary": "Notes", + "custom": "value:with:colons", + } + assert fs_manager.load_memory("notes") == "\n# Body\n\nTail\n" + + def test_add_and_update_frontmatter_without_changing_body(self, fs_manager: MemoryManager) -> None: + body = "# Body\n\nTail\n" + _write(fs_manager, "notes", body) + + fs_manager.add_memory_frontmatter("notes", "summary", "First", is_tool_context=True) + fs_manager.add_memory_frontmatter("notes", "summary", "Updated", is_tool_context=True) + fs_manager.add_memory_frontmatter("notes", "owner", "team:core", is_tool_context=True) + + assert fs_manager.get_memory_frontmatter("notes") == { + "summary": "Updated", + "owner": "team:core", + } + assert fs_manager.load_memory("notes") == body + assert fs_manager.get_memory_file_path("notes").read_text(encoding="utf-8") == ( + "---\nsummary: Updated\nowner: team:core\n---\n# Body\n\nTail\n" + ) + + def test_save_memory_retains_existing_frontmatter(self, fs_manager: MemoryManager) -> None: + _write(fs_manager, "notes", "---\nsummary: Keep me\n---\nOriginal\n") + + fs_manager.save_memory("notes", "Updated\n", is_tool_context=False) + + assert fs_manager.get_memory_frontmatter("notes") == {"summary": "Keep me"} + assert fs_manager.load_memory("notes") == "Updated\n" + + def test_missing_frontmatter_memory_raises(self, fs_manager: MemoryManager) -> None: + with pytest.raises(FileNotFoundError): + fs_manager.get_memory_frontmatter("missing") + with pytest.raises(FileNotFoundError): + fs_manager.add_memory_frontmatter("missing", "summary", "Missing", is_tool_context=True) + + def test_ignored_memory_frontmatter_is_inaccessible(self, tmp_path) -> None: + manager = MemoryManager(serena_data_folder=tmp_path, ignored_memory_patterns=[r"secret"]) + manager.get_memory_file_path("secret").write_text("---\nsummary: Hidden\n---\nBody\n", encoding="utf-8") + + with pytest.raises(ValueError, match="ignored_memory_patterns"): + manager.get_memory_frontmatter("secret") + with pytest.raises(ValueError, match="ignored_memory_patterns"): + manager.add_memory_frontmatter("secret", "owner", "private", is_tool_context=True) + + def test_read_only_memory_rejects_frontmatter_writes(self, tmp_path) -> None: + manager = MemoryManager(serena_data_folder=tmp_path, read_only_memory_patterns=[r"frozen/.*"]) + _write(manager, "frozen/notes", "Body\n") + + with pytest.raises(PermissionError): + manager.add_memory_frontmatter("frozen/notes", "summary", "Frozen", is_tool_context=True) + + def test_global_topic_frontmatter(self, fs_manager: MemoryManager, tmp_path, monkeypatch) -> None: + global_dir = tmp_path / "global" + monkeypatch.setattr(fs_manager, "_global_memory_dir", global_dir) + _write(fs_manager, "global/topic/notes", "Body\n") + + fs_manager.add_memory_frontmatter("global/topic/notes", "summary", "Global", is_tool_context=True) + + assert fs_manager.get_memory_frontmatter("global/topic/notes") == {"summary": "Global"} + assert fs_manager.load_memory("global/topic/notes") == "Body\n" + + def test_reference_autofix_retains_frontmatter(self, fs_manager: MemoryManager) -> None: + _write(fs_manager, "auth/login", "# Login\n") + _write(fs_manager, "docs", "---\nsummary: Authentication docs\n---\nSee auth/login for details.\n") + + report = fs_manager.auto_prefix_bare_references() + + assert report.total_replacements == 1 + assert fs_manager.get_memory_frontmatter("docs") == {"summary": "Authentication docs"} + assert fs_manager.load_memory("docs") == "See mem:auth/login for details.\n" + + class TestListMemoriesFollowsSymlinks: """Regression: memories reachable only through a directory symlink (e.g. a monorepo whose ``.serena/memories`` symlinks each submodule's memory folder, making them addressable as diff --git a/test/serena/test_memory_tools.py b/test/serena/test_memory_tools.py new file mode 100644 index 000000000..37a3cbca5 --- /dev/null +++ b/test/serena/test_memory_tools.py @@ -0,0 +1,72 @@ +import json +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from serena.agent import SerenaAgent +from serena.memories.memory_manager import MemoryManager +from serena.tools.memory_tools import ListMemoriesTool, MemoryAddFrontmatterTool, MemoryGetFrontmatterTool + + +@pytest.fixture +def memory_tool_context(tmp_path) -> tuple[MagicMock, MemoryManager]: + manager = MemoryManager(serena_data_folder=tmp_path) + agent = MagicMock(spec=SerenaAgent) + agent.get_active_project_or_raise.return_value = SimpleNamespace(memory_manager=manager) + agent.get_active_tool_names.return_value = [] + return agent, manager + + +def test_list_memories_default_response_is_unchanged(memory_tool_context: tuple[MagicMock, MemoryManager]) -> None: + agent, manager = memory_tool_context + manager.save_memory("plain", "Body\n", is_tool_context=False) + manager.save_memory("metadata", "---\nsummary: Notes\n---\nBody\n", is_tool_context=False) + + result = json.loads(ListMemoriesTool(agent).apply()) + + assert result == {"memories": ["metadata", "plain"]} + + +def test_list_memories_includes_metadata_when_get_tool_is_active( + memory_tool_context: tuple[MagicMock, MemoryManager], +) -> None: + agent, manager = memory_tool_context + manager.save_memory("plain", "Body\n", is_tool_context=False) + manager.save_memory("metadata", "---\nsummary: Notes\n---\nBody\n", is_tool_context=False) + agent.get_active_tool_names.return_value = [MemoryGetFrontmatterTool.get_name_from_cls()] + + result = json.loads(ListMemoriesTool(agent).apply()) + + assert result == { + "memories": ["metadata", "plain"], + "frontmatter": {"metadata": {"summary": "Notes"}}, + } + + +def test_get_and_add_frontmatter_tools(memory_tool_context: tuple[MagicMock, MemoryManager]) -> None: + agent, manager = memory_tool_context + manager.save_memory("notes", "# Body\n", is_tool_context=False) + add_tool = MemoryAddFrontmatterTool(agent) + get_tool = MemoryGetFrontmatterTool(agent) + + assert add_tool.apply("notes", "summary", "Notes") == "Memory notes written." + assert json.loads(get_tool.apply("notes")) == {"summary": "Notes"} + assert manager.load_memory("notes") == "# Body\n" + + +@pytest.mark.parametrize( + ("key", "value"), + [ + ("summary\nowner", "Notes"), + ("summary", "Notes\nowner: injected"), + ], +) +def test_add_frontmatter_rejects_line_injection(memory_tool_context: tuple[MagicMock, MemoryManager], key: str, value: str) -> None: + agent, manager = memory_tool_context + manager.save_memory("notes", "# Body\n", is_tool_context=False) + + with pytest.raises(ValueError, match="single line"): + MemoryAddFrontmatterTool(agent).apply("notes", key, value) + + assert manager.load_memory("notes") == "# Body\n" From 85086235bb0d5bf23b6a684d473c7d57d8396e82 Mon Sep 17 00:00:00 2001 From: Tyce Herrman Date: Thu, 6 Aug 2026 11:59:02 -0400 Subject: [PATCH 2/2] fix: version and preserve memory frontmatter --- CHANGELOG.md | 7 +- docs/02-usage/045_memories.md | 25 ++++ src/serena/memories/frontmatter.py | 200 ++++++++++++++++++++++---- src/serena/memories/memory_manager.py | 58 +++++--- src/serena/tools/memory_tools.py | 8 +- test/serena/test_frontmatter.py | 140 ++++++++++++++---- test/serena/test_memories_manager.py | 159 ++++++++++++++++---- test/serena/test_memory_tools.py | 54 +++++-- 8 files changed, 527 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca7d6e253..4e01b6390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,8 +20,11 @@ Status of the `main` branch. Changes prior to the next official version change w - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file * Memories: - - Add opt-in tools for reading and updating scalar frontmatter metadata while keeping `read_memory` - focused on memory body content + - Add opt-in tools for reading and updating versioned scalar frontmatter metadata while keeping + `read_memory` focused on memory body content. Managed blocks start with + `serena_frontmatter_version: 1`, require `type`, and use the Open Knowledge Format field name + `description` where applicable. This is field-level alignment, not OKF bundle conformance; + unmarked legacy `---` blocks remain ordinary memory body content. * Language Servers: - `typescript`: Fix: on large projects, the first `find_referencing_symbols`/`request_references` call diff --git a/docs/02-usage/045_memories.md b/docs/02-usage/045_memories.md index 1aa8e3b9e..64cf1fa01 100644 --- a/docs/02-usage/045_memories.md +++ b/docs/02-usage/045_memories.md @@ -71,6 +71,31 @@ Memories can be organized into **topics** by using `/` in the memory name (e.g. The structure is mapped to the file system, where topics correspond to subdirectories. The `list_memories` tool can filter by topic, allowing the agent to explore even large numbers of memories in a structured way. +### Optional Frontmatter Metadata + +The optional `memory_get_frontmatter` and `memory_add_frontmatter` tools expose scalar metadata +without mixing it into the content returned by `read_memory`. Serena only treats a leading block +as managed frontmatter when its first field is the persisted version marker shown below: + +```yaml +--- +serena_frontmatter_version: 1 +type: "Serena Memory" +description: "Short summary" +--- +``` + +The marker is reserved for Serena and is not returned as metadata. A non-empty `type` is required; +new blocks default it to `"Serena Memory"`. Existing files whose leading `---` block lacks the +marker remain ordinary memory body content, including valid-looking or malformed legacy blocks. +Frontmatter recognition does not depend on which tools are active, so `read_memory` consistently +hides marked metadata and body-only writes preserve its original serialization. + +The names `type` and `description` follow the useful field-level conventions of Google's +[Open Knowledge Format](https://github.com/GoogleCloudPlatform/knowledge-catalog/blob/main/okf/SPEC.md). +Serena memories are not OKF bundles: Serena does not use OKF's reserved `index.md` or `log.md` +behavior, and structured OKF fields are outside this scalar-only metadata feature. + (memory-references)= ### Referencing Memories from Other Memories diff --git a/src/serena/memories/frontmatter.py b/src/serena/memories/frontmatter.py index 7f068795b..879de2776 100644 --- a/src/serena/memories/frontmatter.py +++ b/src/serena/memories/frontmatter.py @@ -1,23 +1,64 @@ from __future__ import annotations -from dataclasses import dataclass +import json +from dataclasses import dataclass, field + + +@dataclass(frozen=True) +class _FrontmatterField: + key: str + line_index: int + value_start: int + value_end: int @dataclass(frozen=True) class FrontmatterParseResult: frontmatter: dict[str, str] body: str + prefix: str | None = None + _fields: tuple[_FrontmatterField, ...] = field(default=(), repr=False) + + @property + def is_managed(self) -> bool: + return self.prefix is not None + + def with_body(self, body: str) -> str: + if self.prefix is None: + return body + return self.prefix + body class FrontmatterParser: - """Parser and renderer for simple scalar frontmatter fields.""" + """Parser and byte-preserving updater for Serena scalar frontmatter.""" DELIMITER = "---" + VERSION_KEY = "serena_frontmatter_version" + VERSION = "1" + TYPE_KEY = "type" + DEFAULT_TYPE = "Serena Memory" + + @staticmethod + def _line_ending(line: str) -> str: + if line.endswith("\r\n"): + return "\r\n" + if line.endswith("\n"): + return "\n" + if line.endswith("\r"): + return "\r" + return "" + + @classmethod + def _without_line_ending(cls, line: str) -> str: + ending = cls._line_ending(line) + return line[: -len(ending)] if ending else line @staticmethod def _validate_field(key: str, value: str) -> None: if not key: raise ValueError("Frontmatter key must not be empty") + if key != key.strip(): + raise ValueError("Frontmatter key must not have surrounding whitespace") if ":" in key: raise ValueError("Frontmatter key must not contain ':'") if "\n" in key or "\r" in key: @@ -25,49 +66,150 @@ def _validate_field(key: str, value: str) -> None: if "\n" in value or "\r" in value: raise ValueError("Frontmatter value must be a single line") + @staticmethod + def _parse_value(value: str, line_number: int) -> str: + token = value.strip() + if not token: + return "" + + if token.startswith('"') or token.endswith('"'): + try: + decoded = json.loads(token) + except json.JSONDecodeError as exc: + raise ValueError(f"Malformed quoted frontmatter value on line {line_number}: {exc.msg}") from exc + if not isinstance(decoded, str): + raise ValueError(f"Quoted frontmatter value on line {line_number} must decode to a string") + return decoded + + if token.startswith("'") or token.endswith("'"): + raise ValueError(f"Single-quoted frontmatter values are unsupported on line {line_number}; use JSON double quotes") + + return token + + @classmethod + def _parse_field(cls, line: str, line_index: int) -> tuple[_FrontmatterField, str]: + raw = cls._without_line_ending(line) + if ":" not in raw: + raise ValueError(f"Malformed frontmatter field on line {line_index + 1}: expected 'key: value'") + + colon_index = raw.index(":") + key = raw[:colon_index].strip() + if not key: + raise ValueError(f"Malformed frontmatter field on line {line_index + 1}: key must not be empty") + + value_part = raw[colon_index + 1 :] + leading_length = len(value_part) - len(value_part.lstrip()) + trailing_length = len(value_part) - len(value_part.rstrip()) + value_start = colon_index + 1 + leading_length + value_end = len(raw) - trailing_length if trailing_length else len(raw) + if not value_part.strip(): + value_end = value_start + value = cls._parse_value(value_part, line_index + 1) + return _FrontmatterField(key, line_index, value_start, value_end), value + @classmethod def parse(cls, content: str) -> FrontmatterParseResult: lines = content.splitlines(keepends=True) - if not lines or lines[0].rstrip("\r\n") != cls.DELIMITER: + if not lines or cls._without_line_ending(lines[0]) != cls.DELIMITER or len(lines) < 2: return FrontmatterParseResult(frontmatter={}, body=content) - closing_index: int | None = None + marker_index: int | None = None for index, line in enumerate(lines[1:], start=1): - if line.rstrip("\r\n") == cls.DELIMITER: - closing_index = index + if cls._without_line_ending(line).strip(): + marker_index = index break + if marker_index is None: + return FrontmatterParseResult(frontmatter={}, body=content) - if closing_index is None: + first_raw = cls._without_line_ending(lines[marker_index]) + if ":" not in first_raw: + if first_raw.strip().startswith(cls.VERSION_KEY): + raise ValueError("Malformed Serena frontmatter version marker: expected 'serena_frontmatter_version: 1'") return FrontmatterParseResult(frontmatter={}, body=content) + first_key, first_value = first_raw.split(":", 1) + if first_key.strip() != cls.VERSION_KEY: + return FrontmatterParseResult(frontmatter={}, body=content) + if first_value.strip() != cls.VERSION: + raise ValueError(f"Unsupported Serena frontmatter version {first_value.strip()!r}; expected {cls.VERSION}") + + closing_index: int | None = None + for index, line in enumerate(lines[marker_index + 1 :], start=marker_index + 1): + if cls._without_line_ending(line) == cls.DELIMITER: + closing_index = index + break + if closing_index is None: + raise ValueError("Malformed Serena frontmatter: missing closing '---' delimiter") + frontmatter: dict[str, str] = {} - for line in lines[1:closing_index]: - field = line.rstrip("\r\n") - if not field: + fields: list[_FrontmatterField] = [] + seen_keys: set[str] = set() + for index, line in enumerate(lines[1:closing_index], start=1): + raw = cls._without_line_ending(line) + if not raw.strip(): continue - if ":" not in field: - return FrontmatterParseResult(frontmatter={}, body=content) - - key, value = field.split(":", 1) - key = key.strip() - if not key: - return FrontmatterParseResult(frontmatter={}, body=content) + parsed_field, value = cls._parse_field(line, index) + if parsed_field.key in seen_keys: + raise ValueError(f"Duplicate frontmatter key {parsed_field.key!r} on line {index + 1}") + seen_keys.add(parsed_field.key) + fields.append(parsed_field) + + if parsed_field.key == cls.VERSION_KEY: + if index != marker_index or value != cls.VERSION: + raise ValueError(f"Unsupported Serena frontmatter version {value!r}; expected {cls.VERSION}") + continue + frontmatter[parsed_field.key] = value - value = value.strip() - if len(value) >= 2 and value[0] == value[-1] and value[0] in {'"', "'"}: - value = value[1:-1] - frontmatter[key] = value + memory_type = frontmatter.get(cls.TYPE_KEY) + if memory_type is None: + raise ValueError("Marked Serena frontmatter must contain a non-empty 'type' field") + if not memory_type.strip(): + raise ValueError("Marked Serena frontmatter 'type' field must not be empty") + prefix = "".join(lines[: closing_index + 1]) body = "".join(lines[closing_index + 1 :]) - return FrontmatterParseResult(frontmatter=frontmatter, body=body) + return FrontmatterParseResult(frontmatter=frontmatter, body=body, prefix=prefix, _fields=tuple(fields)) @classmethod - def render(cls, frontmatter: dict[str, str], body: str) -> str: - if not frontmatter: - return body - - for key, value in frontmatter.items(): + def render(cls, frontmatter: dict[str, str], body: str, newline: str = "\n") -> str: + if newline not in {"\n", "\r\n"}: + raise ValueError("Frontmatter newline must be either LF or CRLF") + if cls.VERSION_KEY in frontmatter: + raise ValueError(f"Frontmatter key {cls.VERSION_KEY!r} is reserved and cannot be updated") + + metadata = dict(frontmatter) + metadata.setdefault(cls.TYPE_KEY, cls.DEFAULT_TYPE) + for key, value in metadata.items(): cls._validate_field(key, value) + if not metadata[cls.TYPE_KEY].strip(): + raise ValueError("Marked Serena frontmatter 'type' field must not be empty") - fields = "\n".join(f"{key}: {value}" for key, value in frontmatter.items()) - return f"{cls.DELIMITER}\n{fields}\n{cls.DELIMITER}\n{body}" + ordered_fields = [(cls.TYPE_KEY, metadata.pop(cls.TYPE_KEY)), *metadata.items()] + lines = [cls.DELIMITER, f"{cls.VERSION_KEY}: {cls.VERSION}"] + lines.extend(f"{key}: {json.dumps(value, ensure_ascii=False)}" for key, value in ordered_fields) + lines.append(cls.DELIMITER) + return newline.join(lines) + newline + body + + @classmethod + def upsert(cls, parsed: FrontmatterParseResult, key: str, value: str) -> str: + cls._validate_field(key, value) + if key == cls.VERSION_KEY: + raise ValueError(f"Frontmatter key {cls.VERSION_KEY!r} is reserved and cannot be updated") + + if not parsed.is_managed: + return cls.render({key: value}, parsed.body) + + assert parsed.prefix is not None + prefix_lines = parsed.prefix.splitlines(keepends=True) + rendered_value = json.dumps(value, ensure_ascii=False) + field = next((item for item in parsed._fields if item.key == key), None) + if field is not None: + line = prefix_lines[field.line_index] + prefix_lines[field.line_index] = line[: field.value_start] + rendered_value + line[field.value_end :] + else: + newline = next((cls._line_ending(line) for line in prefix_lines if cls._line_ending(line)), "\n") + prefix_lines.insert(len(prefix_lines) - 1, f"{key}: {rendered_value}{newline}") + + updated = "".join(prefix_lines) + parsed.body + cls.parse(updated) + return updated diff --git a/src/serena/memories/memory_manager.py b/src/serena/memories/memory_manager.py index d96ec20fb..35a49c944 100644 --- a/src/serena/memories/memory_manager.py +++ b/src/serena/memories/memory_manager.py @@ -12,7 +12,7 @@ from serena.constants import SERENA_FILE_ENCODING from serena.util.text_utils import ContentReplacer -from .frontmatter import FrontmatterParser +from .frontmatter import FrontmatterParser, FrontmatterParseResult from .memory_reference_analysis import ( MEMORY_REF_PREFIX, AutofixReport, @@ -208,7 +208,7 @@ def _load_memory_raw(self, name: str) -> tuple[str, str]: memory_file_path = self.get_memory_file_path(name) if not memory_file_path.exists(): raise FileNotFoundError(f"Memory named '{name}' not found") - with open(memory_file_path, encoding=self._encoding) as f: + with open(memory_file_path, encoding=self._encoding, newline="") as f: return name, f.read() def _save_memory_raw(self, name: str, content: str, is_tool_context: bool) -> str: @@ -216,44 +216,56 @@ def _save_memory_raw(self, name: str, content: str, is_tool_context: bool) -> st self._check_not_ignored(name) self._check_write_access(name, is_tool_context) memory_file_path = self.get_memory_file_path(name) - with open(memory_file_path, "w", encoding=self._encoding) as f: + with open(memory_file_path, "w", encoding=self._encoding, newline="") as f: f.write(content) return f"Memory {name} written." + @staticmethod + def _parse_frontmatter(name: str, content: str) -> FrontmatterParseResult: + try: + return FrontmatterParser.parse(content) + except ValueError as exc: + raise ValueError(f"Invalid Serena frontmatter in memory '{name}': {exc}") from exc + def load_memory(self, name: str) -> str: - _, raw = self._load_memory_raw(name) - return FrontmatterParser.parse(raw).body + name, raw = self._load_memory_raw(name) + return self._parse_frontmatter(name, raw).body def save_memory(self, name: str, content: str, is_tool_context: bool) -> str: """ Saves memory body content while retaining any existing frontmatter. - New memories may still be created from raw content containing a frontmatter - block. Once frontmatter exists, callers that only know about the body cannot - accidentally discard it. + New memories may still be created from raw content containing a valid, marked + frontmatter block. Once managed frontmatter exists, callers that only know + about the body cannot accidentally discard or reserialize it. """ name = self._sanitize_name(name) self._check_not_ignored(name) self._check_write_access(name, is_tool_context) memory_file_path = self.get_memory_file_path(name) if memory_file_path.exists(): - with open(memory_file_path, encoding=self._encoding) as f: - parsed = FrontmatterParser.parse(f.read()) - if parsed.frontmatter: - content = FrontmatterParser.render(parsed.frontmatter, content) + with open(memory_file_path, encoding=self._encoding, newline="") as f: + parsed = self._parse_frontmatter(name, f.read()) + if parsed.is_managed: + content = parsed.with_body(content) + else: + self._parse_frontmatter(name, content) + else: + self._parse_frontmatter(name, content) return self._save_memory_raw(name, content, is_tool_context) def get_memory_frontmatter(self, name: str) -> dict[str, str]: - _, raw = self._load_memory_raw(name) - return dict(FrontmatterParser.parse(raw).frontmatter) + name, raw = self._load_memory_raw(name) + return dict(self._parse_frontmatter(name, raw).frontmatter) def add_memory_frontmatter(self, name: str, key: str, value: str, is_tool_context: bool) -> str: name, raw = self._load_memory_raw(name) self._check_write_access(name, is_tool_context) - parsed = FrontmatterParser.parse(raw) - frontmatter = dict(parsed.frontmatter) - frontmatter[key] = value - content = FrontmatterParser.render(frontmatter, parsed.body) + parsed = self._parse_frontmatter(name, raw) + try: + content = FrontmatterParser.upsert(parsed, key, value) + except ValueError as exc: + raise ValueError(f"Invalid Serena frontmatter update for memory '{name}': {exc}") from exc return self._save_memory_raw(name, content, is_tool_context) class MemoriesList: @@ -432,12 +444,12 @@ def edit_memory( memory_file_path = self.get_memory_file_path(name) if not memory_file_path.exists(): raise FileNotFoundError(f"Memory {name} not found.") - with open(memory_file_path, encoding=self._encoding) as f: - original_content = f.read() + with open(memory_file_path, encoding=self._encoding, newline="") as f: + parsed = self._parse_frontmatter(name, f.read()) replacer = ContentReplacer(mode=mode, allow_multiple_occurrences=allow_multiple_occurrences, regex_multiline=regex_multiline) - updated_content = replacer.replace(original_content, needle, repl) - with open(memory_file_path, "w", encoding=self._encoding) as f: - f.write(updated_content) + updated_body = replacer.replace(parsed.body, needle, repl) + with open(memory_file_path, "w", encoding=self._encoding, newline="") as f: + f.write(parsed.with_body(updated_body)) return f"Memory {name} edited successfully." def validate_referential_integrity( diff --git a/src/serena/tools/memory_tools.py b/src/serena/tools/memory_tools.py index bc896e4b4..7ce816b12 100644 --- a/src/serena/tools/memory_tools.py +++ b/src/serena/tools/memory_tools.py @@ -77,8 +77,8 @@ class MemoryGetFrontmatterTool(Tool, ToolMarkerOptional): """ Reads scalar frontmatter metadata from a memory. - The metadata is stored in an optional block delimited by ``---`` lines at - the beginning of the memory file. + Managed metadata starts with ``serena_frontmatter_version: 1`` and requires + a non-empty ``type`` field. The Serena version marker is not returned. """ def apply(self, memory_name: str) -> str: @@ -91,14 +91,14 @@ def apply(self, memory_name: str) -> str: class MemoryAddFrontmatterTool(Tool, ToolMarkerCanEdit, ToolMarkerOptional): - """Adds or updates one scalar frontmatter field in a memory.""" + """Adds or updates one scalar field in versioned Serena frontmatter.""" def apply(self, memory_name: str, key: str, value: str) -> str: """ Add or update a frontmatter field without changing the memory body. :param memory_name: memory name - :param key: frontmatter field name + :param key: frontmatter field name; ``serena_frontmatter_version`` is reserved :param value: scalar frontmatter value """ return self.memory_manager.add_memory_frontmatter(memory_name, key, value, is_tool_context=True) diff --git a/test/serena/test_frontmatter.py b/test/serena/test_frontmatter.py index 961d834d1..29bd06494 100644 --- a/test/serena/test_frontmatter.py +++ b/test/serena/test_frontmatter.py @@ -1,8 +1,21 @@ +import json + import pytest from serena.memories.frontmatter import FrontmatterParser +def _marked(*fields: str, body: str = "Body\n", newline: str = "\n") -> str: + lines = [ + "---", + "serena_frontmatter_version: 1", + 'type: "Serena Memory"', + *fields, + "---", + ] + return newline.join(lines) + newline + body + + def test_parse_without_frontmatter_returns_content_unchanged() -> None: content = "# Memory\n\nBody\n" @@ -10,68 +23,147 @@ def test_parse_without_frontmatter_returns_content_unchanged() -> None: assert result.frontmatter == {} assert result.body == content + assert not result.is_managed -def test_parse_frontmatter_preserves_body_whitespace() -> None: - content = '---\nsummary: "Short description"\nurl: https://example.com:443/docs\n---\n\n# Memory\n\nBody\n' +@pytest.mark.parametrize( + "content", + [ + "---\ndescription: Valid-looking legacy metadata\n---\nBody\n", + "---\n---\nBody\n", + "---\ndescription: missing closing delimiter\n", + "---\nnot a scalar field\n---\nBody\n", + "---\n: missing key\n---\nBody\n", + "---\ndescription: legacy\nserena_frontmatter_version: 1\n---\nBody\n", + " ---\nserena_frontmatter_version: 1\ntype: Serena Memory\n---\nBody\n", + ], +) +def test_unmarked_legacy_delimiters_are_plain_content(content: str) -> None: + result = FrontmatterParser.parse(content) + + assert result.frontmatter == {} + assert result.body == content + assert not result.is_managed + + +def test_parse_marked_frontmatter_preserves_prefix_and_body_whitespace() -> None: + content = _marked( + 'description: " Short description "', + "url: https://example.com:443/docs", + r'note: "A quote: \"hello\" and a backslash: \\ "', + body="\n# Memory\n\nBody\n", + ) result = FrontmatterParser.parse(content) assert result.frontmatter == { - "summary": "Short description", + "type": "Serena Memory", + "description": " Short description ", "url": "https://example.com:443/docs", + "note": 'A quote: "hello" and a backslash: \\ ', } + assert "serena_frontmatter_version" not in result.frontmatter + assert result.prefix == content[: -len(result.body)] assert result.body == "\n# Memory\n\nBody\n" -def test_parse_empty_frontmatter() -> None: - result = FrontmatterParser.parse("---\n---\nBody\n") +def test_marker_is_first_field_even_after_blank_lines() -> None: + content = '---\n\nserena_frontmatter_version: 1\ntype: "Serena Memory"\n---\nBody\n' - assert result.frontmatter == {} + result = FrontmatterParser.parse(content) + + assert result.is_managed assert result.body == "Body\n" + assert result.with_body(result.body) == content @pytest.mark.parametrize( - "content", + ("content", "error"), [ - "---\nsummary: missing closing delimiter\n", - "---\nnot a scalar field\n---\nBody\n", - "---\n: missing key\n---\nBody\n", - " ---\nsummary: indented opening delimiter\n---\nBody\n", + ("---\nserena_frontmatter_version 1\ntype: Serena Memory\n---\nBody\n", "version marker"), + ("---\nserena_frontmatter_version: 2\ntype: Serena Memory\n---\nBody\n", "Unsupported"), + ("---\nserena_frontmatter_version: 1\ntype: Serena Memory\n", "closing"), + ("---\nserena_frontmatter_version: 1\ntype: Serena Memory\ninvalid\n---\nBody\n", "Malformed"), + ("---\nserena_frontmatter_version: 1\ndescription: Missing type\n---\nBody\n", "type"), + ('---\nserena_frontmatter_version: 1\ntype: " "\n---\nBody\n', "type"), + ( + "---\nserena_frontmatter_version: 1\ntype: Serena Memory\ndescription: one\n description : two\n---\nBody\n", + "Duplicate", + ), + ('---\nserena_frontmatter_version: 1\ntype: Serena Memory\ndescription: "unterminated\n---\nBody\n', "Malformed quoted"), + ("---\nserena_frontmatter_version: 1\ntype: 'Serena Memory'\n---\nBody\n", "Single-quoted"), ], ) -def test_malformed_frontmatter_is_plain_content(content: str) -> None: - result = FrontmatterParser.parse(content) - - assert result.frontmatter == {} - assert result.body == content +def test_malformed_marked_frontmatter_is_rejected(content: str, error: str) -> None: + with pytest.raises(ValueError, match=error): + FrontmatterParser.parse(content) -def test_render_round_trip_is_stable() -> None: +def test_render_uses_marker_default_type_and_reversible_json_strings() -> None: frontmatter = { - "summary": "Short description", - "custom": "value:with:colons", + "description": 'Short "description"', + "custom": "value:with:colons\\and\\slashes", } body = "\n# Memory\n\nBody with trailing whitespace.\n\n" rendered = FrontmatterParser.render(frontmatter, body) parsed = FrontmatterParser.parse(rendered) - assert parsed.frontmatter == frontmatter + assert rendered.startswith('---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\n') + assert parsed.frontmatter == {"type": "Serena Memory", **frontmatter} assert parsed.body == body - assert FrontmatterParser.render(parsed.frontmatter, parsed.body) == rendered + assert parsed.with_body(parsed.body) == rendered + + +def test_upsert_replaces_only_selected_value_bytes() -> None: + content = ( + "---\r\n" + "serena_frontmatter_version: 1\r\n" + 'type : "Serena Memory" \r\n' + "description : old:value \r\n" + 'custom:\t"keep:me"\r\n' + "---\r\n" + "\r\nBody\r\n" + ) + value = 'New "quoted" value \\ path:part' + + updated = FrontmatterParser.upsert(FrontmatterParser.parse(content), "description", value) + + expected = content.replace("old:value", json.dumps(value, ensure_ascii=False)) + assert updated == expected + assert FrontmatterParser.parse(updated).frontmatter["description"] == value + + +def test_upsert_inserts_before_closing_delimiter_without_reserializing() -> None: + content = _marked('description : "Keep formatting" ', body="\nBody\n") + + updated = FrontmatterParser.upsert(FrontmatterParser.parse(content), "owner", "team:core") + + assert updated == content.replace("---\n\nBody", 'owner: "team:core"\n---\n\nBody', 1) @pytest.mark.parametrize( ("frontmatter", "error"), [ ({"": "value"}, "must not be empty"), + ({" bad": "value"}, "surrounding whitespace"), ({"bad:key": "value"}, "must not contain"), ({"bad\nkey": "value"}, "single line"), - ({"summary": "bad\nvalue"}, "single line"), - ({"summary": "bad\rvalue"}, "single line"), + ({"description": "bad\nvalue"}, "single line"), + ({"description": "bad\rvalue"}, "single line"), + ({"type": ""}, "type"), + ({"serena_frontmatter_version": "1"}, "reserved"), ], ) -def test_render_rejects_non_scalar_fields(frontmatter: dict[str, str], error: str) -> None: +def test_render_rejects_invalid_fields(frontmatter: dict[str, str], error: str) -> None: with pytest.raises(ValueError, match=error): FrontmatterParser.render(frontmatter, "Body\n") + + +def test_upsert_rejects_reserved_version_and_empty_type() -> None: + parsed = FrontmatterParser.parse(_marked(body="Body\n")) + + with pytest.raises(ValueError, match="reserved"): + FrontmatterParser.upsert(parsed, "serena_frontmatter_version", "2") + with pytest.raises(ValueError, match="type"): + FrontmatterParser.upsert(parsed, "type", " ") diff --git a/test/serena/test_memories_manager.py b/test/serena/test_memories_manager.py index f9aabe061..090f3dc8f 100644 --- a/test/serena/test_memories_manager.py +++ b/test/serena/test_memories_manager.py @@ -270,50 +270,143 @@ def _write(manager: MemoryManager, name: str, content: str) -> None: class TestMemoryFrontmatter: - def test_load_hides_frontmatter_and_preserves_body_whitespace(self, fs_manager: MemoryManager) -> None: - content = "---\nsummary: Notes\ncustom: value:with:colons\n---\n\n# Body\n\nTail\n" + @pytest.mark.parametrize( + "content", + [ + "---\ndescription: Legacy metadata\n---\n\n# Body\n", + "---\n---\nBody\n", + "---\nmalformed legacy block\nBody\n", + "---\r\ndescription: Legacy CRLF\r\n---\r\n\r\nBody\r\n", + ], + ) + def test_unmarked_legacy_content_survives_read_save_cycle_exactly(self, fs_manager: MemoryManager, content: str) -> None: + _write(fs_manager, "notes", content) + + loaded = fs_manager.load_memory("notes") + fs_manager.save_memory("notes", loaded, is_tool_context=False) + + assert loaded == content + assert fs_manager.get_memory_frontmatter("notes") == {} + assert fs_manager.get_memory_file_path("notes").read_bytes() == content.encode() + + def test_load_hides_marked_frontmatter_and_preserves_body_whitespace(self, fs_manager: MemoryManager) -> None: + content = ( + "---\n" + "serena_frontmatter_version: 1\n" + 'type: "Serena Memory"\n' + 'description: "Notes"\n' + "custom: value:with:colons\n" + "---\n" + "\n# Body\n\nTail\n" + ) _write(fs_manager, "notes", content) assert fs_manager.get_memory_frontmatter("notes") == { - "summary": "Notes", + "type": "Serena Memory", + "description": "Notes", "custom": "value:with:colons", } + assert "serena_frontmatter_version" not in fs_manager.get_memory_frontmatter("notes") assert fs_manager.load_memory("notes") == "\n# Body\n\nTail\n" - def test_add_and_update_frontmatter_without_changing_body(self, fs_manager: MemoryManager) -> None: - body = "# Body\n\nTail\n" - _write(fs_manager, "notes", body) + def test_add_frontmatter_wraps_complete_legacy_content(self, fs_manager: MemoryManager) -> None: + legacy = "---\r\ndescription: legacy\r\n---\r\n\r\n# Body\r\n" + _write(fs_manager, "notes", legacy) + + fs_manager.add_memory_frontmatter("notes", "description", 'A "quoted" summary', is_tool_context=True) + + raw = fs_manager.get_memory_file_path("notes").read_bytes().decode() + expected_prefix = '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription: "A \\"quoted\\" summary"\n---\n' + assert raw == expected_prefix + legacy + assert fs_manager.load_memory("notes") == legacy + + def test_update_frontmatter_preserves_unrelated_lines_and_body(self, fs_manager: MemoryManager) -> None: + original = ( + "---\r\n" + "serena_frontmatter_version: 1\r\n" + 'type : "Serena Memory" \r\n' + "description : old:value \r\n" + 'owner:\t"team:core"\r\n' + "---\r\n" + "\r\n# Body\r\n" + ) + path = fs_manager.get_memory_file_path("notes") + path.write_bytes(original.encode()) - fs_manager.add_memory_frontmatter("notes", "summary", "First", is_tool_context=True) - fs_manager.add_memory_frontmatter("notes", "summary", "Updated", is_tool_context=True) - fs_manager.add_memory_frontmatter("notes", "owner", "team:core", is_tool_context=True) + fs_manager.add_memory_frontmatter("notes", "description", "Updated", is_tool_context=True) - assert fs_manager.get_memory_frontmatter("notes") == { - "summary": "Updated", - "owner": "team:core", - } - assert fs_manager.load_memory("notes") == body - assert fs_manager.get_memory_file_path("notes").read_text(encoding="utf-8") == ( - "---\nsummary: Updated\nowner: team:core\n---\n# Body\n\nTail\n" - ) + assert path.read_bytes() == original.replace("old:value", '"Updated"').encode() + assert fs_manager.load_memory("notes") == "\r\n# Body\r\n" + + def test_save_memory_retains_exact_marked_prefix_with_crlf(self, fs_manager: MemoryManager) -> None: + prefix = '---\r\nserena_frontmatter_version: 1\r\ntype : "Serena Memory" \r\ndescription:\t"Keep me"\r\n---\r\n' + path = fs_manager.get_memory_file_path("notes") + path.write_bytes((prefix + "Original\r\n").encode()) + + fs_manager.save_memory("notes", "Updated\r\n", is_tool_context=False) + + assert path.read_bytes() == (prefix + "Updated\r\n").encode() + + def test_edit_memory_only_edits_body(self, fs_manager: MemoryManager) -> None: + prefix = '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription : "Keep bytes" \n---\n' + path = fs_manager.get_memory_file_path("notes") + path.write_bytes((prefix + "Old body\n").encode()) - def test_save_memory_retains_existing_frontmatter(self, fs_manager: MemoryManager) -> None: - _write(fs_manager, "notes", "---\nsummary: Keep me\n---\nOriginal\n") + fs_manager.edit_memory("notes", "Old", "New", "literal", False, is_tool_context=True) - fs_manager.save_memory("notes", "Updated\n", is_tool_context=False) + assert path.read_bytes() == (prefix + "New body\n").encode() - assert fs_manager.get_memory_frontmatter("notes") == {"summary": "Keep me"} - assert fs_manager.load_memory("notes") == "Updated\n" + @pytest.mark.parametrize( + "invalid", + [ + "---\nserena_frontmatter_version: 2\ntype: Serena Memory\n---\nBody\n", + "---\nserena_frontmatter_version: 1\ndescription: Missing type\n---\nBody\n", + "---\nserena_frontmatter_version: 1\ntype: Serena Memory\ntype: Duplicate\n---\nBody\n", + '---\nserena_frontmatter_version: 1\ntype: Serena Memory\ndescription: "bad\n---\nBody\n', + ], + ) + def test_invalid_marked_memory_fails_before_modification(self, fs_manager: MemoryManager, invalid: str) -> None: + path = fs_manager.get_memory_file_path("notes") + path.write_bytes(invalid.encode()) + + with pytest.raises(ValueError, match="memory 'notes'"): + fs_manager.save_memory("notes", "Replacement\n", is_tool_context=False) + with pytest.raises(ValueError, match="memory 'notes'"): + fs_manager.add_memory_frontmatter("notes", "owner", "team", is_tool_context=True) + + assert path.read_bytes() == invalid.encode() + + def test_invalid_marked_new_memory_is_not_created(self, fs_manager: MemoryManager) -> None: + invalid = "---\nserena_frontmatter_version: 1\ndescription: Missing type\n---\nBody\n" + path = fs_manager.get_memory_file_path("notes") + + with pytest.raises(ValueError, match="memory 'notes'"): + fs_manager.save_memory("notes", invalid, is_tool_context=False) + + assert not path.exists() + + def test_reserved_marker_update_fails_without_modification(self, fs_manager: MemoryManager) -> None: + _write(fs_manager, "notes", "Body\n") + path = fs_manager.get_memory_file_path("notes") + before = path.read_bytes() + + with pytest.raises(ValueError, match="reserved"): + fs_manager.add_memory_frontmatter("notes", "serena_frontmatter_version", "2", is_tool_context=True) + + assert path.read_bytes() == before def test_missing_frontmatter_memory_raises(self, fs_manager: MemoryManager) -> None: with pytest.raises(FileNotFoundError): fs_manager.get_memory_frontmatter("missing") with pytest.raises(FileNotFoundError): - fs_manager.add_memory_frontmatter("missing", "summary", "Missing", is_tool_context=True) + fs_manager.add_memory_frontmatter("missing", "description", "Missing", is_tool_context=True) def test_ignored_memory_frontmatter_is_inaccessible(self, tmp_path) -> None: manager = MemoryManager(serena_data_folder=tmp_path, ignored_memory_patterns=[r"secret"]) - manager.get_memory_file_path("secret").write_text("---\nsummary: Hidden\n---\nBody\n", encoding="utf-8") + manager.get_memory_file_path("secret").write_text( + '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription: "Hidden"\n---\nBody\n', + encoding="utf-8", + ) with pytest.raises(ValueError, match="ignored_memory_patterns"): manager.get_memory_frontmatter("secret") @@ -325,27 +418,35 @@ def test_read_only_memory_rejects_frontmatter_writes(self, tmp_path) -> None: _write(manager, "frozen/notes", "Body\n") with pytest.raises(PermissionError): - manager.add_memory_frontmatter("frozen/notes", "summary", "Frozen", is_tool_context=True) + manager.add_memory_frontmatter("frozen/notes", "description", "Frozen", is_tool_context=True) def test_global_topic_frontmatter(self, fs_manager: MemoryManager, tmp_path, monkeypatch) -> None: global_dir = tmp_path / "global" monkeypatch.setattr(fs_manager, "_global_memory_dir", global_dir) _write(fs_manager, "global/topic/notes", "Body\n") - fs_manager.add_memory_frontmatter("global/topic/notes", "summary", "Global", is_tool_context=True) + fs_manager.add_memory_frontmatter("global/topic/notes", "description", "Global", is_tool_context=True) - assert fs_manager.get_memory_frontmatter("global/topic/notes") == {"summary": "Global"} + assert fs_manager.get_memory_frontmatter("global/topic/notes") == { + "type": "Serena Memory", + "description": "Global", + } assert fs_manager.load_memory("global/topic/notes") == "Body\n" def test_reference_autofix_retains_frontmatter(self, fs_manager: MemoryManager) -> None: _write(fs_manager, "auth/login", "# Login\n") - _write(fs_manager, "docs", "---\nsummary: Authentication docs\n---\nSee auth/login for details.\n") + prefix = '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription : "Authentication docs" \n---\n' + _write(fs_manager, "docs", prefix + "See auth/login for details.\n") report = fs_manager.auto_prefix_bare_references() assert report.total_replacements == 1 - assert fs_manager.get_memory_frontmatter("docs") == {"summary": "Authentication docs"} + assert fs_manager.get_memory_frontmatter("docs") == { + "type": "Serena Memory", + "description": "Authentication docs", + } assert fs_manager.load_memory("docs") == "See mem:auth/login for details.\n" + assert fs_manager.get_memory_file_path("docs").read_bytes().startswith(prefix.encode()) class TestListMemoriesFollowsSymlinks: diff --git a/test/serena/test_memory_tools.py b/test/serena/test_memory_tools.py index 37a3cbca5..6e67885f0 100644 --- a/test/serena/test_memory_tools.py +++ b/test/serena/test_memory_tools.py @@ -6,7 +6,14 @@ from serena.agent import SerenaAgent from serena.memories.memory_manager import MemoryManager -from serena.tools.memory_tools import ListMemoriesTool, MemoryAddFrontmatterTool, MemoryGetFrontmatterTool +from serena.tools.memory_tools import ( + ListMemoriesTool, + MemoryAddFrontmatterTool, + MemoryGetFrontmatterTool, + ReadMemoryTool, +) + +MARKED_PREFIX = '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription: "Notes"\n---\n' @pytest.fixture @@ -21,52 +28,73 @@ def memory_tool_context(tmp_path) -> tuple[MagicMock, MemoryManager]: def test_list_memories_default_response_is_unchanged(memory_tool_context: tuple[MagicMock, MemoryManager]) -> None: agent, manager = memory_tool_context manager.save_memory("plain", "Body\n", is_tool_context=False) - manager.save_memory("metadata", "---\nsummary: Notes\n---\nBody\n", is_tool_context=False) + manager.save_memory("metadata", MARKED_PREFIX + "Body\n", is_tool_context=False) result = json.loads(ListMemoriesTool(agent).apply()) assert result == {"memories": ["metadata", "plain"]} -def test_list_memories_includes_metadata_when_get_tool_is_active( +def test_list_memories_includes_metadata_without_version_marker_when_get_tool_is_active( memory_tool_context: tuple[MagicMock, MemoryManager], ) -> None: agent, manager = memory_tool_context manager.save_memory("plain", "Body\n", is_tool_context=False) - manager.save_memory("metadata", "---\nsummary: Notes\n---\nBody\n", is_tool_context=False) + manager.save_memory("metadata", MARKED_PREFIX + "Body\n", is_tool_context=False) agent.get_active_tool_names.return_value = [MemoryGetFrontmatterTool.get_name_from_cls()] result = json.loads(ListMemoriesTool(agent).apply()) assert result == { "memories": ["metadata", "plain"], - "frontmatter": {"metadata": {"summary": "Notes"}}, + "frontmatter": {"metadata": {"type": "Serena Memory", "description": "Notes"}}, } +@pytest.mark.parametrize("active_tools", [[], [MemoryGetFrontmatterTool.get_name_from_cls()]]) +def test_read_memory_hides_marked_metadata_regardless_of_active_tools( + memory_tool_context: tuple[MagicMock, MemoryManager], active_tools: list[str] +) -> None: + agent, manager = memory_tool_context + manager.save_memory("notes", MARKED_PREFIX + "\n# Body\n", is_tool_context=False) + agent.get_active_tool_names.return_value = active_tools + + assert ReadMemoryTool(agent).apply("notes") == "\n# Body\n" + assert manager.get_memory_file_path("notes").read_bytes() == (MARKED_PREFIX + "\n# Body\n").encode() + + def test_get_and_add_frontmatter_tools(memory_tool_context: tuple[MagicMock, MemoryManager]) -> None: agent, manager = memory_tool_context manager.save_memory("notes", "# Body\n", is_tool_context=False) add_tool = MemoryAddFrontmatterTool(agent) get_tool = MemoryGetFrontmatterTool(agent) - assert add_tool.apply("notes", "summary", "Notes") == "Memory notes written." - assert json.loads(get_tool.apply("notes")) == {"summary": "Notes"} + assert add_tool.apply("notes", "description", 'A "quoted" note: core\\path') == "Memory notes written." + assert json.loads(get_tool.apply("notes")) == { + "type": "Serena Memory", + "description": 'A "quoted" note: core\\path', + } + assert "serena_frontmatter_version" not in json.loads(get_tool.apply("notes")) assert manager.load_memory("notes") == "# Body\n" @pytest.mark.parametrize( - ("key", "value"), + ("key", "value", "error"), [ - ("summary\nowner", "Notes"), - ("summary", "Notes\nowner: injected"), + ("description\nowner", "Notes", "single line"), + ("description", "Notes\nowner: injected", "single line"), + ("serena_frontmatter_version", "2", "reserved"), ], ) -def test_add_frontmatter_rejects_line_injection(memory_tool_context: tuple[MagicMock, MemoryManager], key: str, value: str) -> None: +def test_add_frontmatter_rejects_invalid_updates_without_modifying_memory( + memory_tool_context: tuple[MagicMock, MemoryManager], key: str, value: str, error: str +) -> None: agent, manager = memory_tool_context manager.save_memory("notes", "# Body\n", is_tool_context=False) + path = manager.get_memory_file_path("notes") + before = path.read_bytes() - with pytest.raises(ValueError, match="single line"): + with pytest.raises(ValueError, match=error): MemoryAddFrontmatterTool(agent).apply("notes", key, value) - assert manager.load_memory("notes") == "# Body\n" + assert path.read_bytes() == before