mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Stop a tool-context memory rename from aborting on read-only memories (#2081)
Rename propagation * `rename_memory_and_propagate_references` enumerated `get_full_list()`, which includes the memories matched by `read_only_memory_patterns`. * Writing one of those raises `PermissionError` in `_check_write_access`, so a rename of a memory that was referenced from a read-only memory failed *after* `move_memory` had already applied the rename. * The memory graph was then half-updated: the renamed memory existed under its new name while the writable referrers still held `mem:OLD_NAME`, and retrying the rename failed with "Memory not found". Fix * In a tool context, propagate only into the memories that accept writes, sorted to keep the enumeration order that `get_full_list()` provided. * Outside a tool context nothing changes, so read-only memories are still updated when the user renames through the CLI. * A reference which thereby remains in a read-only memory is still reported as stale by `validate_referential_integrity`, so it is not hidden. Documentation * `docs/02-usage/045_memories.md` promised that the tool rewrites every reference across all memories. That is now only true for the memories the agent may write, so the caveat is stated where the promise is made. Tests * Add a regression pair: the tool-context rename completes and rewrites both occurrences in a writable referrer, while the CLI context additionally rewrites the read-only one.
This commit is contained in:
4 files changed
+60
-6
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user