diff --git a/CHANGELOG.md b/CHANGELOG.md index ce43a05e9..4e01b6390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ 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 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 could silently race tsserver's project load and return incomplete results, because the fixed 2s 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 new file mode 100644 index 000000000..879de2776 --- /dev/null +++ b/src/serena/memories/frontmatter.py @@ -0,0 +1,215 @@ +from __future__ import annotations + +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 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: + 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") + + @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 cls._without_line_ending(lines[0]) != cls.DELIMITER or len(lines) < 2: + return FrontmatterParseResult(frontmatter={}, body=content) + + marker_index: int | None = None + for index, line in enumerate(lines[1:], start=1): + if cls._without_line_ending(line).strip(): + marker_index = index + break + if marker_index is None: + return FrontmatterParseResult(frontmatter={}, body=content) + + 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] = {} + 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 + 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 + + 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, prefix=prefix, _fields=tuple(fields)) + + @classmethod + 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") + + 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 daa037252..35a49c944 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, FrontmatterParseResult from .memory_reference_analysis import ( MEMORY_REF_PREFIX, AutofixReport, @@ -201,24 +202,72 @@ 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() + with open(memory_file_path, encoding=self._encoding, newline="") as f: + 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) 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: + 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 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, 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]: + 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 = 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: def __init__(self) -> None: self.memories: list[str] = [] @@ -237,8 +286,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: @@ -395,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 6ebffd9b1..7ce816b12 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. + + 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: + """ + 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 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; ``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) class DeleteMemoryTool(Tool, ToolMarkerCanEdit): diff --git a/test/serena/test_frontmatter.py b/test/serena/test_frontmatter.py new file mode 100644 index 000000000..29bd06494 --- /dev/null +++ b/test/serena/test_frontmatter.py @@ -0,0 +1,169 @@ +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" + + result = FrontmatterParser.parse(content) + + assert result.frontmatter == {} + assert result.body == content + assert not result.is_managed + + +@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 == { + "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_marker_is_first_field_even_after_blank_lines() -> None: + content = '---\n\nserena_frontmatter_version: 1\ntype: "Serena Memory"\n---\nBody\n' + + result = FrontmatterParser.parse(content) + + assert result.is_managed + assert result.body == "Body\n" + assert result.with_body(result.body) == content + + +@pytest.mark.parametrize( + ("content", "error"), + [ + ("---\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_marked_frontmatter_is_rejected(content: str, error: str) -> None: + with pytest.raises(ValueError, match=error): + FrontmatterParser.parse(content) + + +def test_render_uses_marker_default_type_and_reversible_json_strings() -> None: + frontmatter = { + "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 rendered.startswith('---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\n') + assert parsed.frontmatter == {"type": "Serena Memory", **frontmatter} + assert parsed.body == body + 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"), + ({"description": "bad\nvalue"}, "single line"), + ({"description": "bad\rvalue"}, "single line"), + ({"type": ""}, "type"), + ({"serena_frontmatter_version": "1"}, "reserved"), + ], +) +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 158ab379a..090f3dc8f 100644 --- a/test/serena/test_memories_manager.py +++ b/test/serena/test_memories_manager.py @@ -269,6 +269,186 @@ def _write(manager: MemoryManager, name: str, content: str) -> None: manager.save_memory(name, content, is_tool_context=False) +class TestMemoryFrontmatter: + @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") == { + "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_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", "description", "Updated", is_tool_context=True) + + 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()) + + fs_manager.edit_memory("notes", "Old", "New", "literal", False, is_tool_context=True) + + assert path.read_bytes() == (prefix + "New body\n").encode() + + @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", "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( + '---\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") + 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", "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", "description", "Global", is_tool_context=True) + + 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") + 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") == { + "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: """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..6e67885f0 --- /dev/null +++ b/test/serena/test_memory_tools.py @@ -0,0 +1,100 @@ +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, + ReadMemoryTool, +) + +MARKED_PREFIX = '---\nserena_frontmatter_version: 1\ntype: "Serena Memory"\ndescription: "Notes"\n---\n' + + +@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", 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_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", 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": {"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", "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", "error"), + [ + ("description\nowner", "Notes", "single line"), + ("description", "Notes\nowner: injected", "single line"), + ("serena_frontmatter_version", "2", "reserved"), + ], +) +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=error): + MemoryAddFrontmatterTool(agent).apply("notes", key, value) + + assert path.read_bytes() == before