-
-
Notifications
You must be signed in to change notification settings - Fork 280
feat: add multi-project support with scope enforcement #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add ability to manage multiple indexed projects in the knowledge graph: - list_projects: List all indexed projects - delete_project: Remove specific project while preserving others - wipe_database: Complete database clear with confirmation - index_repository: Now only clears current project data This enables multiple codebases to coexist in the same database, with each project's data isolated by qualified_name prefix. Based on PR vitali87#178 by PatD42, refactored to comply with project coding standards (no docstrings, centralized constants, TypedDicts). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add security safeguards to prevent cross-project data access: - list_projects: Only shows current project when scoped - delete_project: Blocked if targeting different project - wipe_database: Disabled entirely in scoped mode Scope enforcement is enabled by default (enforce_scope=True). This matches the safety model implemented in graphiti. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary of ChangesHello @LongSunnyDay, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the MCP tools by integrating comprehensive multi-project support. It allows for the seamless management of multiple distinct codebases within a single Memgraph instance, ensuring data integrity and preventing conflicts. Key to this is the introduction of new project-management tools and a robust scope enforcement mechanism that isolates project data and restricts operations to the current project by default, thereby improving safety and usability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively introduces multi-project support with a strong emphasis on safety through scope enforcement. The new tools (list_projects, delete_project, wipe_database) are well-designed, incorporating crucial safety checks for destructive actions. The modification of index_repository to operate on a per-project basis is a significant enhancement. The changes are consistently applied across the codebase, including constants, logging, type definitions, and services. I have one suggestion to improve code clarity and remove a minor redundancy.
|
Thanks @LongSunnyDay ! I'm a bit overloaded at work and didn't had time to adapt to comments guidelines. |
Greptile Summaryadded multi-project support by introducing three new MCP tools ( Key Changes:
Critical Issues:
Architecture:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant MCPToolsRegistry
participant MemgraphIngestor
participant Memgraph
Note over User,Memgraph: Multi-Project Index Operation
User->>MCPToolsRegistry: index_repository()
MCPToolsRegistry->>MCPToolsRegistry: get project_name from path
MCPToolsRegistry->>MemgraphIngestor: delete_project(project_name)
MemgraphIngestor->>Memgraph: MATCH (n) WHERE n.qualified_name STARTS WITH prefix
MemgraphIngestor->>Memgraph: OR (n:Project AND n.name = project_name)
MemgraphIngestor->>Memgraph: DETACH DELETE n
Note right of Memgraph: Only current project deleted,<br/>other projects preserved
MCPToolsRegistry->>MCPToolsRegistry: GraphUpdater.run()
MCPToolsRegistry-->>User: Success message
Note over User,Memgraph: List Projects (Scoped)
User->>MCPToolsRegistry: list_projects()
MCPToolsRegistry->>MemgraphIngestor: list_projects()
MemgraphIngestor->>Memgraph: MATCH (p:Project) RETURN p.name
Memgraph-->>MemgraphIngestor: all_projects[]
MemgraphIngestor-->>MCPToolsRegistry: all_projects[]
alt enforce_scope = true
MCPToolsRegistry->>MCPToolsRegistry: filter to current_project only
end
MCPToolsRegistry-->>User: filtered projects[]
Note over User,Memgraph: Delete Project (Scoped)
User->>MCPToolsRegistry: delete_project(project_name)
alt enforce_scope = true
MCPToolsRegistry->>MCPToolsRegistry: check project_name == current_project
alt project_name != current_project
MCPToolsRegistry-->>User: Scope violation error
end
end
MCPToolsRegistry->>MemgraphIngestor: delete_project(project_name)
MemgraphIngestor->>Memgraph: DETACH DELETE matching nodes
MCPToolsRegistry-->>User: Success message
Note over User,Memgraph: Wipe Database (Blocked)
User->>MCPToolsRegistry: wipe_database(confirm=true)
alt enforce_scope = true
MCPToolsRegistry-->>User: Blocked - use delete_project instead
else enforce_scope = false
MCPToolsRegistry->>MemgraphIngestor: clean_database()
MemgraphIngestor->>Memgraph: DELETE ALL
MCPToolsRegistry-->>User: All projects wiped
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (3)
-
codebase_rag/services/graph_service.py, line 179 (link)logic: query misses nodes without
qualified_namepropertynodes with missing
qualified_namewon't match theSTARTS WITHfilter and will be orphaned.File,Folder, and similar structural nodes may lack this property. useEXISTS(n.qualified_name)to check property existence -
codebase_rag/tests/test_mcp_query_and_index.py, line 360-363 (link)logic: test assertions outdated
index_repositorynow callsdelete_projectinstead ofclean_database. update assertions to verifydelete_projectis called with correct project names -
codebase_rag/services/graph_service.py, line 174-184 (link)logic: incomplete project deletion -
FileandFoldernodes orphanedFolderandFilenodes lackqualified_nameproperty (only havepath/name), so theSTARTS WITHfilter won't match them. only nodes withqualified_nameand theProjectnode itself are deleteduse graph traversal to delete all nodes reachable from the project:
7 files reviewed, 3 comments
- Use self.current_project instead of recalculating project_name in index_repository (removes redundant Path().resolve().name call) - Fix delete_project() to use graph traversal instead of qualified_name prefix matching, which was missing File/Folder nodes that use 'path' - Update tests to verify delete_project is called with correct project names instead of checking for clean_database calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Hi @LongSunnyDay, Thanks for this PR! We've just merged PR #233 which implements the core multi-project support functionality: What's now in main:
Regarding scope enforcement (the unique part of this PR): We're considering whether scope enforcement adds enough value to justify the complexity. Some concerns:
Do you still see value in the scope enforcement feature given these concerns? If so, what use cases do you have in mind where the existing safeguards wouldn't be sufficient? If not, we can close this PR as superseded by #233. |
Summary
This PR adds multi-project support to the MCP tools, allowing multiple codebases to coexist in the same Memgraph database without interference.
New MCP Tools
list_projects- List all indexed projects (scoped to current project by default)delete_project- Remove a specific project while preserving otherswipe_database- Complete database clear (disabled in scoped mode for safety)Key Changes
index_repositorynow only clears the current project's data, preserving other projectsqualified_nameprefix matchingScope Enforcement
list_projectsdelete_projectwipe_databaseconfirm=trueFiles Changed
constants.py- AddedMCPToolName,MCPParamName,MCPSchemaTypeentrieslogs.py- Added log constants for new operationstool_descriptions.py- Added tool and param descriptionstypes_defs.py- AddedListProjectsResultDict,DeleteProjectResultDictgraph_service.py- Addedlist_projects(),delete_project()methodsmcp/tools.py- Added new tools with scope enforcementTesting
All tests pass:
wipe_databaseblocked in scoped modeRelated
Based on PR #178 by @PatD42, refactored to comply with project coding standards.
Test plan
list_projectsreturns correct projectsdelete_projectonly deletes specified projectindex_repositorypreserves other projectswipe_databaseis blocked in scoped mode🤖 Generated with Claude Code