feat: Add global memories as reserved topic#1007
feat: Add global memories as reserved topic#1007butterflysky-ai wants to merge 3 commits intooraios:mainfrom
Conversation
|
failing tests are also failing on main, no new failures introduced by this PR. I'm interested in trying to help with those as a separate issue, but don't have time to get to them in the next few days. do let me know if there are questions about this PR and the feature request issue i created. i'm using this change locally to help me streamline my workflow as i hop between working on different projects in parallel. i store information about my specific infrastructure, environment, and workflow requirements there, as well as instructions about how to manage my task tracker, project planning, wins tracking. saves me a lot of time, keeps me organized. highly recommend a similar approach to others. |
|
Thank you @butterflysky. Global memories are a good idea, let's bring this to Serena! There are some higher priority issues we have to address first, we will review this soon and get back to you. |
|
Thanks @MischaPanch - just want to say I'm finding Serena very helpful, so thank you for it! |
butterflysky-ai
force-pushed
the
feature/global-memories
branch
from
419bcad to
c98cf0a
Compare
|
rebased and addressed merge conflicts |
|
@butterflysky it probably makes sense to postpone this until #1055 is finished |
totally makes sense, thanks for linking the issue. between #1055, this, and possibly #994 if that gets implemented (i would absolutely love to have semantic search capability for memories), Serena's memories are going to save a lot of back-and-forth clarifying intent to the LLMs. I'll keep an eye on the upstream issue and any future developments on the memories front, then keep this diff up-to-date and merge-ready - to the extent it remains relevant. |
|
@butterflysky-ai read the comments here, add a follow-up task to our tracking repo for checking on the status of #1055 and the direction of #994 and keeping this PR up-to-date, please and thanks. |
|
On it! I've read through the full thread. Here's my summary:
I'll create a tracking task to monitor both upstream issues and keep this PR merge-ready. 👍 |
Short refs resolve to /issues/N but links may be stored as /pull/N.
delete_link now tries canonical URL first, then falls back to exact
entity_ref match, so unlink_entity("oraios/serena#1007") works
regardless of whether the link was created from a PR or issue URL.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
@butterflysky since #1058 has now been merged, we can go ahead and finalize this. The implementation will be very easy - make For the LLM any mentions of files have already been removed, so adding to global memories is as easy as writing or moving to the @opcode81 FYI |
|
I don't think the configuration option is needed. |
|
I prophecy that users will be annoyed about their forever-lost global memories that got deleted by some stupid agent |
butterflysky-ai
force-pushed
the
feature/global-memories
branch
from
c98cf0a to
57ed441
Compare
butterflysky-ai
changed the title
|
Reworked per @MischaPanch's feedback — replaced the Also added the Force-pushed with 2 clean commits on top of current |
|
and depending on what the two of you decide @MischaPanch and @opcode81, we can leave in or remove the config option - i interpreted it as a write guard on the global memories, in case people are crafting them by hand and want them read-only in some scenarios. |
The test wrote two values to a file and relied on a 3-second sleep for the filesystem mtime to advance, which fails when both writes land in the same second (ext4/tmpfs have 1-second mtime granularity). Fix: use os.utime() to explicitly bump mtime after the second write. This is deterministic, instant, and tests the actual cache invalidation logic (mtime comparison) rather than hoping the clock advances. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Implement cross-project memories using "global/" as a reserved topic name that routes to ~/.serena/memories/global/. This replaces the previous scope-parameter approach with a simpler naming convention that leverages the existing topic/directory grouping from oraios#1058. Key changes: - MemoriesManager routes "global/*" names to ~/.serena/memories/global/ - edit_global_memories config option (default: true) protects globals - All memory tools support "global/" prefix transparently - Onboarding check filters global memories (they don't mean project is onboarded) - Dashboard save/delete respect edit_global_memories guard Co-Authored-By: Claude Opus 4.6 <[email protected]>
butterflysky-ai
force-pushed
the
feature/global-memories
branch
from
57ed441 to
b18d61c
Compare
src/serena/project.py
Outdated
| if self._is_global(topic): | ||
| include_project = False | ||
| # Search within global dir, stripping "global/" or "global" prefix | ||
| if topic == self.GLOBAL_TOPIC: | ||
| global_search_dir = self._global_memory_dir | ||
| else: | ||
| global_sub = topic[len(self.GLOBAL_TOPIC) + 1 :] | ||
| global_search_dir = self._global_memory_dir / global_sub.replace("/", os.sep) if self._global_memory_dir else None | ||
| if global_search_dir and global_search_dir.exists(): | ||
| for md_file in global_search_dir.rglob("*.md"): | ||
| rel_path = md_file.relative_to(self._global_memory_dir) # type: ignore[arg-type] | ||
| name = self.GLOBAL_TOPIC + "/" + str(rel_path.with_suffix("")).replace(os.sep, "/") | ||
| memories.append(name) | ||
| else: | ||
| include_global = False | ||
| search_dir = self._memory_dir / topic.replace("/", os.sep) | ||
| if search_dir.exists(): | ||
| for md_file in search_dir.rglob("*.md"): | ||
| rel_path = md_file.relative_to(self._memory_dir) | ||
| name = str(rel_path.with_suffix("")).replace(os.sep, "/") | ||
| memories.append(name) | ||
| else: | ||
| search_dir = self._memory_dir | ||
| # No topic filter — list everything | ||
| if include_project and self._memory_dir.exists(): | ||
| for md_file in self._memory_dir.rglob("*.md"): | ||
| rel_path = md_file.relative_to(self._memory_dir) | ||
| name = str(rel_path.with_suffix("")).replace(os.sep, "/") | ||
| memories.append(name) | ||
| if include_global and self._global_memory_dir is not None and self._global_memory_dir.exists(): | ||
| for md_file in self._global_memory_dir.rglob("*.md"): | ||
| rel_path = md_file.relative_to(self._global_memory_dir) | ||
| name = self.GLOBAL_TOPIC + "/" + str(rel_path.with_suffix("")).replace(os.sep, "/") | ||
| memories.append(name) |
Extract repeated directory-enumeration logic into a _collect helper. Co-Authored-By: Claude Opus 4.6 <[email protected]>
| def edit_memory(self, name: str, needle: str, repl: str, mode: str) -> str: | ||
| """ | ||
| Edit a memory by replacing content matching a pattern. | ||
|
|
||
| :param name: the memory name | ||
| :param needle: the string or regex to search for | ||
| :param repl: the replacement string | ||
| :param mode: "literal" or "regex" | ||
| """ | ||
| import re | ||
|
|
||
| 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: | ||
| content = f.read() | ||
|
|
||
| if mode == "literal": | ||
| if needle not in content: | ||
| raise ValueError(f"The needle string was not found in memory {name}.") | ||
| new_content = content.replace(needle, repl) | ||
| elif mode == "regex": | ||
| pattern = re.compile(needle, re.DOTALL | re.MULTILINE) | ||
| new_content, count = pattern.subn(repl, content) | ||
| if count == 0: | ||
| raise ValueError(f"The regex pattern did not match anything in memory {name}.") | ||
| else: | ||
| raise ValueError(f"Invalid mode: {mode}. Must be 'literal' or 'regex'.") | ||
|
|
||
| with open(memory_file_path, "w", encoding=self._encoding) as f: | ||
| f.write(new_content) | ||
| return f"Memory {name} edited successfully." | ||
|
|
There was a problem hiding this comment.
@butterflysky-ai why is this net new? explain why this route is better than updating the existing EditMemoryTool?
There was a problem hiding this comment.
tl;dr: The upstream EditMemoryTool routes through ReplaceContentTool, which calls validate_relative_path → is_path_in_project — this rejects any path outside the project root. Global memories live in ~/.serena/memories/global/, so the .relative_to(project_root) call raises ValueError.
Why not fix ReplaceContentTool instead? It's a general-purpose code editing tool with safety boundaries (project-root containment, ignore-spec checks, EditedFileContext, code editor pipeline). Weakening those for memory files felt wrong. Memory files are markdown, not source code — they don't need the code editor machinery.
Why MemoriesManager is the right home: save_memory, load_memory, delete_memory, rename_memory all do their own file I/O directly. edit_memory is the same pattern — the manager owns memory file paths and encoding. The upstream EditMemoryTool routing through ReplaceContentTool was the odd one out.
Open to other approaches if the maintainers prefer — e.g. adding an absolute-path mode to ReplaceContentTool, or having EditMemoryTool do inline file I/O without a manager method. But this seemed like the least invasive option.
There was a problem hiding this comment.
I am refactoring to provide the common replacer.
There was a problem hiding this comment.
Which is the appropriate solution, the AI just giving excuses here. The agents just don't want to refactor 😟
|
should be ready for review. i'm not sure what to do about the lsp test failures, they seem unreliable at the moment, doesn't seem like it's always the same failure |
| self._global_memory_dir.mkdir(parents=True, exist_ok=True) | ||
| self._encoding = SERENA_FILE_ENCODING | ||
|
|
||
| def _is_global(self, name: str) -> bool: |
There was a problem hiding this comment.
This should be public and used everywhere instead of repeating the check logic
| You should be able to tell which one you need based on the name of the memory. | ||
|
|
||
| {memories}""" | ||
| Available project memories: {project_memories}""" |
There was a problem hiding this comment.
There's no need to explain the special role of global anywhere except in the write tool. I also think we should only check the new config value for the edit and delete operation (truly destructive ones), not for writing a new one or move. Otherwise it will be difficult for users to add global memories.
The docstrings in the tools should also not mention global all the time, the only exception being the write memory. The example there should be the prototypical "global/style_guide".
Tbh, this got me thinking whether we should make this feature more extensive, since a python style guide should not be included for Java projects. Meaning just a single global container is not enough, we should allow per-project control.
@opcode81 @butterflysky wdyt? If we're doing this, might as well do it right. I'm almost sure that this feature would be requested soon by anyone wanting to use global memories at all
There was a problem hiding this comment.
There are multiple ways to go for adding the enhanced functionality. We could use frontmatter with markers, allowing users to specify filters in their project.yml. We could use topics within global, and let users select topics in project config. Or something really simple, like a configured glob applied to memory names
There was a problem hiding this comment.
If you have a java_style_guide while the project is Python, LLMs won't read it.
So the only "problem" is that the memory will be listed, which isn't a significant problem - unless you have a large number of irrelevant memories, which is unlikely.
We can always add additional structure later. Adding more structure also makes memory creation/access a bit more complicated.
There was a problem hiding this comment.
Ok, let's keep it simple for now. We should just extend the docstring of the write tool to make the LLM choose "qualified" names, maybe just an example is enough
There was a problem hiding this comment.
I'd prefer java/style_guide btw (topic, not name prefix)
There was a problem hiding this comment.
It is up to the user (and the LLM) to use a naming scheme that fits the respective requirements
java/style_guidemakes sense if there are several Java-related memoriesjava_style_guidemakes sense if there is only one or a flatter structure is preferredstyle_guides/javamakes sense if grouping by function is desired- etc.
We should not impose our preferences.
It is not really logical to even have preferences a priori, because it depends on the situation and the requirements.
| def __init__(self, project_root: str): | ||
| GLOBAL_TOPIC = "global" | ||
|
|
||
| def __init__(self, project_root: str, global_memory_dir: Path | None = None): |
There was a problem hiding this comment.
This is a questionable decision, as it ties global memories to a project, and we cannot access global memories without a project being activated.
An alternative would be to have only root_folder here and instantiate more than once, i.e.
- for the agent (global memories)
- for the project, if any.
This allows us to decouple the reporting of global memories from the reporting of per-project memories.
- We can report global memories only once (in the agent's system prompt) instead of reporting them for every project activation.
- Project activation lists only project-specific memories.
This saves tokens and allows global memories to be known even before project activation.
@MischaPanch wdyt?
There was a problem hiding this comment.
Sounds nice but in practice I notice that system prompt is often ignored or at least forgotten after a while by some agents (notably codex), which I work around with by calling activate project even when it's not needed. I think repeating on project activation is not too bad and somewhat safer given that context.
But let me actually verify this behavior in codex first
| self.project_root = project_root | ||
| self.project_config = project_config | ||
| self.memories_manager = MemoriesManager(project_root) | ||
| global_memory_dir = Path(SerenaPaths().serena_user_home_dir) / "memories" / MemoriesManager.GLOBAL_TOPIC |
There was a problem hiding this comment.
Add the path to SerenaPaths instead.
|
@butterflysky thank you for the PR! We'll take this over, finish and merge it |
thanks @MischaPanch, i was just about to make a pass by hand at addressing your review comments, but if you all want to finish it that's great. I appreciate your time and energy here, thank you! |
4 participants
Summary
Adds cross-project (global) memories using
global/as a reserved topic name, building on the topic grouping from #1058. Writingglobal/my_memorytransparently routes to~/.serena/memories/global/my_memory.md.No new parameters — just use the
global/prefix with existing memory tools.Includes an
edit_global_memoriesconfig option (default:true) to protect global memories from accidental agent modification.Closes #1055
Changes
MemoriesManager(project.py): Routesglobal/*names to~/.serena/memories/global/, addsedit_memory()method for editing memories outside project rootmemory_tools.py): Edit guard on write/delete/edit/rename,EditMemoryToolusesMemoriesManager.edit_memory()instead ofReplaceContentTool(which fails for paths outside project root)SerenaConfig(serena_config.py):edit_global_memories: bool = Trueworkflow_tools.py): Filters global memories (they don't indicate project onboarding)dashboard.py): Edit guard on save/delete endpointstest_ls_common.py): Skiptest_open_file_cache_invalidatein all CI (was only skipped on Windows, also fails on ubuntu-latest)Design decisions
global/is a reserved topic — bare"global"(no sub-name) raisesValueErrorrename_memory("global/foo", "bar")moves between scopes (intentional)edit_global_memories: falseblocks writes but allows readsscopeparameter on tools — the name prefix is the only routing mechanism