Fix clone_graph: throws on most graphs and corrupts every edge type#44
Merged
Conversation
Two real, AI-reachable bugs in the clone_graph MCP tool: 1. ParameterMissing(teamId): Neo4j does not persist null-valued properties, so a graph created without a teamId has NO teamId property. cloneGraph read it back as `undefined` and passed it straight to the driver, which rejects undefined param values — so clone threw for essentially every graph created via the API. Coalesce teamId (and type/isShared/settings defensively) to non-undefined values. 2. Edge types silently collapsed to DEPENDS_ON: the edge-clone query matched every relationship type but hard-coded `CREATE (a)-[:DEPENDS_ON ...]->(b)`, rewriting BLOCKS / RELATES_TO / CONTAINS / PART_OF into DEPENDS_ON on clone. Use apoc.create.relationship(newW, type(r), ...) to preserve the real type (APOC 5.x ships with the project's Neo4j). Adds a real-Neo4j contract case: clone a graph with BLOCKS + RELATES_TO edges and assert all nodes copy and both edge types survive. The existing mock unit tests passed through both bugs — they can't reproduce real-DB param/relationship-type behavior, which is exactly why the contract test exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Found during the skeptical live-app audit by exercising the
clone_graphMCP tool against a real Neo4j. It has two real, AI-reachable bugs:1.
clone_graphthrowsParameterMissing: teamIdfor most graphsNeo4j does not store
null-valued properties, so a graph created without ateamIdsimply has noteamIdproperty.cloneGraphread it back asundefinedand passed it as a query parameter — and the driver rejectsundefinedparam values. So clone threw for essentially every graph created via the MCP API (createGraphstoresteamId: null→ property absent).Fix: coalesce
teamId(andtype/isShared/settingsdefensively) to non-undefinedvalues with?? null.2. Every cloned edge becomes
DEPENDS_ONThe edge-clone query matched all relationship types but hard-coded
CREATE (newW)-[:DEPENDS_ON {...}]->(newTargetW), so aBLOCKS/RELATES_TO/CONTAINS/PART_OFedge was silently rewritten toDEPENDS_ONon clone — corrupting the graph's dependency semantics.Fix:
CALL apoc.create.relationship(newW, type(r), {...}, newTargetW)to preserve the real type. (APOC 5.26 ships with the project's Neo4j.)Verification
Before (probe):
clone_graphthrewParameterMissing(teamId).After (probe): clones 3 nodes + 2 edges, types preserved as
BLOCKSandRELATES_TO.Tests
New real-Neo4j contract case (
MCP Real-Neo4j ContractCI job): clone a graph containing aBLOCKSand aRELATES_TOedge → assert all nodes copy and both edge types survive (not collapsed toDEPENDS_ON).The existing mock unit tests passed straight through both bugs — they can't reproduce real-DB null-property/relationship-type behavior, which is exactly why the contract test exists.
🤖 Generated with Claude Code