diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d518a05..2d9f577e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,10 @@ Status of the `main` branch. Changes prior to the next official version change w the write could destroy the previous, valid content instead of just losing the update. Both now write through a temp-file-plus-`os.replace` helper, matching the approach `save_yaml()` already uses for settings files (#1958) + - Fix: renaming a memory through the `rename_memory` tool raised `PermissionError` when another memory + marked read-only by `read_only_memory_patterns` referenced it, after the rename had already been + applied, leaving the memory graph half-updated; reference propagation in tool contexts now covers + only writable memories, as documented, while the CLI still propagates into read-only ones * JetBrains: - Fix: Concurrent Serena sessions activating different projects at the same time with diff --git a/docs/02-usage/045_memories.md b/docs/02-usage/045_memories.md index 1aa8e3b9..5c6b3129 100644 --- a/docs/02-usage/045_memories.md +++ b/docs/02-usage/045_memories.md @@ -80,7 +80,9 @@ This convention has two practical consequences: - **Renames keep references intact.** When you rename or move a memory with the `rename_memory` tool, Serena rewrites every `` `mem:OLD_NAME` `` occurrence across all memories to point to - the new name. References that do not use the `mem:` prefix will not be updated automatically. + the new name, except in memories matched by `read_only_memory_patterns`, which the agent cannot + write; `serena memories check` reports such a reference as stale. + References that do not use the `mem:` prefix will not be updated automatically. - **Integrity checks** (see [below](memory-cli)) report any `` `mem:NAME` `` whose target does not resolve to an existing memory, and propose similarly-named candidates as likely intended targets. diff --git a/src/serena/memories/memory_manager.py b/src/serena/memories/memory_manager.py index f9e83385..c185ff18 100644 --- a/src/serena/memories/memory_manager.py +++ b/src/serena/memories/memory_manager.py @@ -348,22 +348,32 @@ class MemoryManager: def rename_memory_and_propagate_references(self, old_name: str, new_name: str, is_tool_context: bool) -> tuple[str, int]: """ - Renames a memory and updates every ``mem:OLD_NAME`` reference across all memories. + Renames a memory and updates every ``mem:OLD_NAME`` reference in the memories which + accept writes in the given context. Memories whose content does not contain a reference to ``old_name`` are left - untouched (no spurious mtime changes). Memories that do are rewritten via - :meth:`save_memory`. + untouched (no spurious mtime changes); those that do are rewritten via + :meth:`save_memory`. References in a memory which does not accept writes (a read-only + memory in a tool context) are not affected and remain reported as stale by + :meth:`validate_referential_integrity`. :param old_name: the current memory name (the source of the rename) :param new_name: the target memory name :param is_tool_context: forwarded to :meth:`save_memory` for read-only enforcement :return: a tuple of (rename message returned by :meth:`move_memory`, total number of - ``mem:`` reference occurrences rewritten across all memories). + ``mem:`` reference occurrences rewritten in those memories). """ renaming_message = self.move_memory(old_name, new_name, is_tool_context=is_tool_context) + # propagate the reference, enumerating after the move such that the renamed memory + # itself is covered; the read-only memories are excluded in a tool context because + # writing to one would raise after the move was already applied, leaving the memory + # graph half-updated + memories_list = self.list_memories() + target_names = sorted(memories_list.memories) if is_tool_context else memories_list.get_full_list() + total_updates = 0 - for memory_name in self.list_memories().get_full_list(): + for memory_name in target_names: content = self.load_memory(memory_name) updated_content, n_replacements = self.rename_references_to_memory(content, old_name, new_name) if n_replacements > 0: diff --git a/test/serena/test_memories_manager.py b/test/serena/test_memories_manager.py index 46c1b47b..513f9057 100644 --- a/test/serena/test_memories_manager.py +++ b/test/serena/test_memories_manager.py @@ -844,3 +844,41 @@ class TestAutoPrefixBareReferences: # idempotent: the second run should not touch anything assert second.total_replacements == 0 assert fs_manager.load_memory("docs") == "the mem:auth/login process" + + +class TestRenameMemorySparesReadOnlyMemories: + """Regression: a tool-context rename enumerated read-only memories, so propagating the + reference into one raised ``PermissionError`` after the rename itself had already been applied. + """ + + @staticmethod + def _manager(tmp_path, monkeypatch) -> MemoryManager: + manager = MemoryManager(serena_data_folder=tmp_path, read_only_memory_patterns=[r"frozen/.*"]) + # the global memories of the machine would otherwise join the enumeration as well + global_dir = tmp_path / "global" + global_dir.mkdir() + monkeypatch.setattr(manager, "_global_memory_dir", global_dir) + _write(manager, "auth/login", "# login notes") + _write(manager, "frozen/notes", "see `mem:auth/login`") + _write(manager, "docs", "first `mem:auth/login`, then `mem:auth/login`") + return manager + + def test_tool_context_rename_completes_and_leaves_read_only_reference_alone(self, tmp_path, monkeypatch) -> None: + manager = self._manager(tmp_path, monkeypatch) + + message, n_updated = manager.rename_memory_and_propagate_references("auth/login", "auth/signin", is_tool_context=True) + + assert "auth/signin" in message + assert manager.load_memory("auth/signin") == "# login notes" + assert manager.load_memory("docs") == "first `mem:auth/signin`, then `mem:auth/signin`" + assert manager.load_memory("frozen/notes") == "see `mem:auth/login`" + assert n_updated == 2 + + def test_cli_context_rename_still_propagates_into_read_only_memories(self, tmp_path, monkeypatch) -> None: + manager = self._manager(tmp_path, monkeypatch) + + _, n_updated = manager.rename_memory_and_propagate_references("auth/login", "auth/signin", is_tool_context=False) + + assert manager.load_memory("frozen/notes") == "see `mem:auth/signin`" + assert manager.load_memory("docs") == "first `mem:auth/signin`, then `mem:auth/signin`" + assert n_updated == 3