diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts index cf1f287..69667f9 100644 --- a/packages/mcp-server/src/services/graph-service.ts +++ b/packages/mcp-server/src/services/graph-service.ts @@ -15,7 +15,6 @@ import { Neo4jParams, Neo4jNode, Neo4jContributor, - Neo4jPathSegment, AnalysisResults, WorkloadData, CapacityAnalysis, @@ -349,9 +348,9 @@ export class GraphService { } query = ` MATCH (n:WorkItem {id: $node_id}) - OPTIONAL MATCH (n)-[r1:DEPENDS_ON]->(dep:WorkItem) - OPTIONAL MATCH (dependent:WorkItem)-[r2:DEPENDS_ON]->(n) - RETURN n, + OPTIONAL MATCH (n)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(dep:WorkItem) + OPTIONAL MATCH (dependent:WorkItem)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(n) + RETURN n, COLLECT(DISTINCT dep) as dependencies, COLLECT(DISTINCT dependent) as dependents `; @@ -884,24 +883,31 @@ export class GraphService { }; } - // Create the relationship + // Edges are first-class Edge NODES (e)-[:EDGE_SOURCE]->(source), + // (e)-[:EDGE_TARGET]->(target) — the SAME model the GraphQL server and + // web UI use. (Previously MCP created a direct (source)-[:TYPE]->(target) + // relationship, which the web never rendered, so AI-created edges were + // invisible to humans.) MERGE on the source/target/type triple keeps it + // idempotent like the old code. const createQuery = ` MATCH (source:WorkItem {id: $source_id}) MATCH (target:WorkItem {id: $target_id}) - MERGE (source)-[r:${type} { - weight: $weight, - metadata: $metadata, - createdAt: $now - }]->(target) + MERGE (source)<-[:EDGE_SOURCE]-(r:Edge {type: $type})-[:EDGE_TARGET]->(target) + ON CREATE SET r.id = 'edge-' + randomUUID(), + r.weight = $weight, + r.metadata = $metadata, + r.createdAt = datetime() + ON MATCH SET r.weight = $weight, + r.metadata = $metadata RETURN r, source, target `; const params = { source_id, target_id, + type, weight, - metadata: JSON.stringify(metadata), - now: new Date().toISOString() + metadata: JSON.stringify(metadata) }; const result = await session.run(createQuery, params); @@ -931,13 +937,14 @@ export class GraphService { const { source_id, target_id, type } = args; const deleteQuery = ` - MATCH (source:WorkItem {id: $source_id})-[r:${type}]->(target:WorkItem {id: $target_id}) - DELETE r + MATCH (source:WorkItem {id: $source_id})<-[:EDGE_SOURCE]-(r:Edge {type: $type})-[:EDGE_TARGET]->(target:WorkItem {id: $target_id}) + WITH r, source, target + DETACH DELETE r RETURN source, target `; - const result = await session.run(deleteQuery, { source_id, target_id }); - + const result = await session.run(deleteQuery, { source_id, target_id, type }); + if (result.records.length === 0) { return { content: [{ @@ -988,20 +995,25 @@ export class GraphService { RETURN n, COLLECT(DISTINCT c) as contributors `; - // Count relationships + // Count relationships — edges are Edge NODES joined via EDGE_SOURCE / + // EDGE_TARGET (the same model the web uses), not direct WorkItem rels. const relCountQuery = ` MATCH (n:WorkItem {id: $node_id}) - OPTIONAL MATCH (n)-[rel]-(related:WorkItem) - RETURN count(DISTINCT rel) as totalRelationships + OPTIONAL MATCH (n)<-[:EDGE_SOURCE|EDGE_TARGET]-(e:Edge) + RETURN count(DISTINCT e) as totalRelationships `; // Get relationships with pagination const relationshipsQuery = ` MATCH (n:WorkItem {id: $node_id}) - OPTIONAL MATCH (n)-[rel]-(related:WorkItem) - RETURN rel, related, - CASE WHEN startNode(rel) = n THEN 'outgoing' ELSE 'incoming' END as direction - ORDER BY type(rel), related.title + OPTIONAL MATCH (n)<-[se:EDGE_SOURCE|EDGE_TARGET]-(e:Edge) + OPTIONAL MATCH (e)-[:EDGE_SOURCE]->(src:WorkItem) + OPTIONAL MATCH (e)-[:EDGE_TARGET]->(tgt:WorkItem) + WITH e, + CASE WHEN type(se) = 'EDGE_SOURCE' THEN 'outgoing' ELSE 'incoming' END as direction, + CASE WHEN type(se) = 'EDGE_SOURCE' THEN tgt ELSE src END as related + RETURN e as rel, related, direction + ORDER BY e.type, related.title SKIP $relationships_offset LIMIT $relationships_limit `; @@ -1046,12 +1058,12 @@ export class GraphService { const relationships = relationshipsResult.records .filter(record => record.get('rel') !== null && record.get('related') !== null) .map(record => { - const rel = record.get('rel'); + const rel = record.get('rel'); // an Edge NODE const related = record.get('related'); const direction = record.get('direction'); return { - type: rel.type, + type: rel.properties.type, direction, target_node: related.properties, relationship_properties: rel.properties @@ -1106,15 +1118,22 @@ export class GraphService { const limitInt = typeof limit === 'number' ? Math.floor(limit) : parseInt(String(limit), 10) || 10; const offsetInt = typeof offset === 'number' ? Math.floor(offset) : parseInt(String(offset), 10) || 0; - // Count query to get total number of paths + // Edges are Edge NODES, so each logical hop between two WorkItems is two + // physical hops (WorkItem)<-[:EDGE_SOURCE]-(:Edge)-[:EDGE_TARGET]->(WorkItem). + // Traverse the EDGE_SOURCE/EDGE_TARGET structure (depth doubled) and + // project the path back to its WorkItem nodes and Edge nodes. + const physicalDepth = maxDepthInt * 2; const countQuery = ` - MATCH path = allShortestPaths((start:WorkItem {id: $start_id})-[*1..${maxDepthInt}]-(end:WorkItem {id: $end_id})) + MATCH path = allShortestPaths((start:WorkItem {id: $start_id})-[:EDGE_SOURCE|EDGE_TARGET*1..${physicalDepth}]-(end:WorkItem {id: $end_id})) RETURN count(path) as total `; const query = ` - MATCH path = allShortestPaths((start:WorkItem {id: $start_id})-[*1..${maxDepthInt}]-(end:WorkItem {id: $end_id})) - RETURN path, length(path) as pathLength + MATCH path = allShortestPaths((start:WorkItem {id: $start_id})-[:EDGE_SOURCE|EDGE_TARGET*1..${physicalDepth}]-(end:WorkItem {id: $end_id})) + WITH path, + [x IN nodes(path) WHERE x:WorkItem | x] as wnodes, + [x IN nodes(path) WHERE x:Edge | x] as wedges + RETURN wnodes, wedges, size(wedges) as pathLength ORDER BY pathLength ASC SKIP $offset LIMIT $limit @@ -1148,20 +1167,12 @@ export class GraphService { } const paths = result.records.map(record => { - const path = record.get('path'); const pathLength = record.get('pathLength')?.toNumber?.() || 0; - - const nodes = path.segments.map((segment: Neo4jPathSegment, index: number) => { - if (index === 0) { - return [segment.start.properties, segment.end.properties]; - } else { - return segment.end.properties; - } - }).flat(); - const relationships = path.segments.map((segment: Neo4jPathSegment) => ({ - type: segment.relationship.type, - properties: segment.relationship.properties + const nodes = (record.get('wnodes') || []).map((n: Neo4jNode) => n.properties); + const relationships = (record.get('wedges') || []).map((e: Neo4jNode) => ({ + type: e.properties.type, + properties: e.properties })); return { nodes, relationships, length: pathLength }; @@ -1192,16 +1203,23 @@ export class GraphService { const limit = Math.floor(Number(args.limit || args.max_cycles || 10)); const offset = Math.floor(Number(args.offset || 0)); + // DEPENDS_ON edges are Edge NODES, so a logical hop is + // (a)<-[:EDGE_SOURCE]-(:Edge)-[:EDGE_TARGET]->(b). Use a quantified path + // pattern (Neo4j 5) to find cycles of 2..10 logical hops, then keep only + // the WorkItem nodes for the result. + const cyclePattern = `(n:WorkItem) ((:WorkItem)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(:WorkItem)){2,10} (n)`; + // Count query for total cycles const countQuery = ` - MATCH path = (n:WorkItem)-[:DEPENDS_ON*2..10]->(n) + MATCH path = ${cyclePattern} RETURN count(path) as total `; // Main query with pagination using parameterized queries const query = ` - MATCH path = (n:WorkItem)-[:DEPENDS_ON*2..10]->(n) - RETURN path, length(path) as cycleLength + MATCH path = ${cyclePattern} + WITH path, [x IN nodes(path) WHERE x:WorkItem | x] as cycleNodes + RETURN cycleNodes, size(cycleNodes) - 1 as cycleLength ORDER BY cycleLength ASC SKIP $offset LIMIT $limit @@ -1217,17 +1235,8 @@ export class GraphService { }); const cycles = result.records.map(record => { - const path = record.get('path'); const cycleLength = record.get('cycleLength')?.toNumber?.() || 0; - - const nodes = path.segments.map((segment: Neo4jPathSegment, index: number) => { - if (index === 0) { - return [segment.start.properties, segment.end.properties]; - } else { - return segment.end.properties; - } - }).flat(); - + const nodes = (record.get('cycleNodes') || []).map((n: Neo4jNode) => n.properties); return { nodes, length: cycleLength }; }); @@ -1607,9 +1616,9 @@ export class GraphService { count(n) as total_nodes, collect(DISTINCT n.type) as node_types, collect(DISTINCT n.status) as statuses, - avg(size((n)-[:DEPENDS_ON]->())) as avg_dependencies, - max(size((n)-[:DEPENDS_ON]->())) as max_dependencies, - count(CASE WHEN size((n)-[:DEPENDS_ON]->()) = 0 THEN 1 END) as isolated_nodes + avg(COUNT { (n)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(:WorkItem) }) as avg_dependencies, + max(COUNT { (n)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(:WorkItem) }) as max_dependencies, + count(CASE WHEN COUNT { (n)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(:WorkItem) } = 0 THEN 1 END) as isolated_nodes `; try { @@ -1675,7 +1684,7 @@ export class GraphService { const query = ` MATCH (n:WorkItem) ${whereClause} - OPTIONAL MATCH (n)-[:DEPENDS_ON]->(dep:WorkItem) + OPTIONAL MATCH (n)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(dep:WorkItem) WITH n, collect(dep) as dependencies RETURN count(n) as total_nodes, @@ -1706,7 +1715,7 @@ export class GraphService { const query = ` MATCH (n:WorkItem) ${whereClause} - OPTIONAL MATCH (n)<-[:DEPENDS_ON]-(dependent:WorkItem) + OPTIONAL MATCH (n)<-[:EDGE_TARGET]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_SOURCE]->(dependent:WorkItem) WITH n, count(dependent) as dependent_count WHERE dependent_count > 3 RETURN @@ -1834,7 +1843,7 @@ export class GraphService { const bottleneckQuery = ` MATCH (n:WorkItem) ${whereClause} - OPTIONAL MATCH (n)<-[:DEPENDS_ON]-(dependent:WorkItem) + OPTIONAL MATCH (n)<-[:EDGE_TARGET]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_SOURCE]->(dependent:WorkItem) WITH n, collect(dependent) as dependents, count(dependent) as dependent_count WHERE dependent_count > 0 RETURN @@ -1853,7 +1862,7 @@ export class GraphService { const blockedChainQuery = ` MATCH (blocked:WorkItem {status: 'BLOCKED'}) ${whereClause ? whereClause.replace('n.teamId', 'blocked.teamId') : ''} - OPTIONAL MATCH (blocked)-[:DEPENDS_ON]->(blocker:WorkItem) + OPTIONAL MATCH (blocked)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(blocker:WorkItem) WHERE blocker.status IN ['PROPOSED', 'PLANNED', 'IN_PROGRESS'] RETURN blocked.id as blocked_id, @@ -2227,17 +2236,20 @@ export class GraphService { const query = ` MATCH (source:WorkItem {id: $source_id}) MATCH (target:WorkItem {id: $target_id}) - CREATE (source)-[r:${type} { - weight: $weight, - metadata: $metadata, - createdAt: datetime() - }]->(target) + MERGE (source)<-[:EDGE_SOURCE]-(r:Edge {type: $type})-[:EDGE_TARGET]->(target) + ON CREATE SET r.id = 'edge-' + randomUUID(), + r.weight = $weight, + r.metadata = $metadata, + r.createdAt = datetime() + ON MATCH SET r.weight = $weight, + r.metadata = $metadata RETURN r `; const result = await sessionOrTx.run(query, { source_id, target_id, + type, weight, metadata: JSON.stringify(metadata) }); @@ -2253,12 +2265,12 @@ export class GraphService { const { source_id, target_id, type } = params; const query = ` - MATCH (source:WorkItem {id: $source_id})-[r:${type}]->(target:WorkItem {id: $target_id}) - DELETE r + MATCH (source:WorkItem {id: $source_id})<-[:EDGE_SOURCE]-(r:Edge {type: $type})-[:EDGE_TARGET]->(target:WorkItem {id: $target_id}) + DETACH DELETE r RETURN count(r) as deleted_count `; - const result = await sessionOrTx.run(query, { source_id, target_id }); + const result = await sessionOrTx.run(query, { source_id, target_id, type }); const deletedCount = result.records[0]?.get('deleted_count').toNumber() || 0; if (deletedCount === 0) { @@ -2515,7 +2527,7 @@ export class GraphService { const query = ` MATCH (c:Contributor {id: $contributorId})-[:CONTRIBUTES_TO]->(w:WorkItem) WHERE w.status IN $statusFilter - OPTIONAL MATCH (w)-[:DEPENDS_ON]->(dep:WorkItem) + OPTIONAL MATCH (w)<-[:EDGE_SOURCE]-(:Edge {type: 'DEPENDS_ON'})-[:EDGE_TARGET]->(dep:WorkItem) RETURN w, count(dep) as dependencyCount, collect(DISTINCT dep.title)[0..3] as sampleDependencies @@ -3233,7 +3245,7 @@ export class GraphService { const query = ` MATCH (g:Graph {id: $graphId}) OPTIONAL MATCH (g)<-[:BELONGS_TO]-(w:WorkItem) - OPTIONAL MATCH (w)-[e:DEPENDS_ON|BLOCKS|RELATES_TO|CONTAINS|PART_OF]-(:WorkItem) + OPTIONAL MATCH (w)<-[:EDGE_SOURCE]-(e:Edge) RETURN g, count(DISTINCT w) as nodeCount, count(DISTINCT e) as edgeCount, @@ -3289,7 +3301,8 @@ export class GraphService { const query = ` MATCH (g:Graph {id: $graphId}) OPTIONAL MATCH (g)<-[:BELONGS_TO]-(w:WorkItem) - OPTIONAL MATCH (w)-[e:DEPENDS_ON|BLOCKS|RELATES_TO|CONTAINS|PART_OF]-(:WorkItem) + // Edges are Edge NODES; count those whose source is in this graph. + OPTIONAL MATCH (w)<-[:EDGE_SOURCE]-(e:Edge) WITH g, collect(DISTINCT w) as items, count(DISTINCT e) as edgeCount // Return the raw type/status lists and tally them in JS. A // CALL { UNWIND items ... } subquery returns ZERO rows for an empty @@ -3301,8 +3314,8 @@ export class GraphService { [x IN items | x.status] as statuses CALL { WITH g - OPTIONAL MATCH (g)<-[:BELONGS_TO]-(b:WorkItem)-[r:BLOCKS]->(:WorkItem) - WITH b, count(r) as blocksCount + OPTIONAL MATCH (g)<-[:BELONGS_TO]-(b:WorkItem)<-[:EDGE_SOURCE]-(e:Edge {type: 'BLOCKS'})-[:EDGE_TARGET]->(:WorkItem) + WITH b, count(e) as blocksCount WHERE b IS NOT NULL AND blocksCount > 0 ORDER BY blocksCount DESC LIMIT 5 RETURN collect({id: b.id, title: b.title, blocksCount: blocksCount}) as blockers @@ -3633,18 +3646,23 @@ export class GraphService { // Clone edges if requested if (args.includeEdges !== false && clonedNodes > 0) { + // Edges are Edge NODES — clone each source Edge node (with its + // real type) between the corresponding cloned WorkItems. const cloneEdgesQuery = ` MATCH (newG:Graph {id: $newGraphId})<-[:BELONGS_TO]-(newW:WorkItem) MATCH (sourceG:Graph {id: $sourceGraphId})<-[:BELONGS_TO]-(sourceW:WorkItem) WHERE sourceW.id = newW.originalId - MATCH (sourceW)-[r:DEPENDS_ON|BLOCKS|RELATES_TO|CONTAINS|PART_OF]->(targetW:WorkItem)-[:BELONGS_TO]->(sourceG) + MATCH (sourceW)<-[:EDGE_SOURCE]-(e:Edge)-[:EDGE_TARGET]->(targetW:WorkItem)-[:BELONGS_TO]->(sourceG) MATCH (newG)<-[:BELONGS_TO]-(newTargetW:WorkItem) WHERE newTargetW.originalId = targetW.id - // Preserve the real relationship type — the previous code - // hard-coded :DEPENDS_ON, silently rewriting every BLOCKS / - // RELATES_TO / CONTAINS / PART_OF edge into a DEPENDS_ON on clone. - CALL apoc.create.relationship(newW, type(r), { weight: r.weight, metadata: r.metadata }, newTargetW) YIELD rel - RETURN count(rel) as edgeCount + CREATE (newW)<-[:EDGE_SOURCE]-(newE:Edge { + id: 'edge-' + randomUUID(), + type: e.type, + weight: e.weight, + metadata: e.metadata, + createdAt: datetime() + })-[:EDGE_TARGET]->(newTargetW) + RETURN count(newE) as edgeCount `; const edgesResult = await tx.run(cloneEdgesQuery, { diff --git a/packages/mcp-server/tests/mock-neo4j.ts b/packages/mcp-server/tests/mock-neo4j.ts index 5579a1a..9e10d2f 100644 --- a/packages/mcp-server/tests/mock-neo4j.ts +++ b/packages/mcp-server/tests/mock-neo4j.ts @@ -301,8 +301,20 @@ export function createMockDriver(): Driver { }; } + // Handle getNodeDetails relationships query — edges are Edge NODES, so + // 'rel' is an Edge node (with .properties.type), 'related' a WorkItem. + if (query.includes('e as rel') && query.includes('EDGE_SOURCE|EDGE_TARGET')) { + return { + records: [createMockRecord({ + rel: { properties: { id: 'edge-1', type: 'DEPENDS_ON', weight: 1, metadata: '{}' } }, + related: { properties: { id: 'related-1', title: 'Related Node', type: 'TASK', status: 'ACTIVE' } }, + direction: 'outgoing' + })] + }; + } + // Handle clone operations - return 0 for nodeCount/edgeCount - if (query.includes('count(newW)') || query.includes('count(newR)')) { + if (query.includes('count(newW)') || query.includes('count(newR)') || query.includes('count(newE)')) { return { records: [createMockRecord({ nodeCount: { toNumber: () => 0 }, @@ -322,7 +334,7 @@ export function createMockDriver(): Driver { beginTransaction: () => ({ run: async (query: string, params?: any) => { // Handle clone operations within transaction - if (query.includes('count(newW)') || query.includes('count(newR)')) { + if (query.includes('count(newW)') || query.includes('count(newR)') || query.includes('count(newE)')) { return { records: [createMockRecord({ nodeCount: { toNumber: () => 0 }, diff --git a/packages/mcp-server/tests/neo4j-contract.test.ts b/packages/mcp-server/tests/neo4j-contract.test.ts index eadc6ff..53d2f5c 100644 --- a/packages/mcp-server/tests/neo4j-contract.test.ts +++ b/packages/mcp-server/tests/neo4j-contract.test.ts @@ -170,7 +170,7 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { const s2 = driver.session(); try { const r = await s2.run( - 'MATCH (g:Graph {id: $gid})<-[:BELONGS_TO]-(:WorkItem)-[rel]->(:WorkItem) RETURN type(rel) AS t ORDER BY t', + 'MATCH (g:Graph {id: $gid})<-[:BELONGS_TO]-(:WorkItem)<-[:EDGE_SOURCE]-(e:Edge)-[:EDGE_TARGET]->(:WorkItem) RETURN e.type AS t ORDER BY t', { gid: dstId } ); const types = r.records.map((rec) => rec.get('t')).sort(); @@ -180,6 +180,52 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { } }); + it('createEdge is human-visible: produces an Edge NODE (EDGE_SOURCE/EDGE_TARGET), not a direct rel', async () => { + // The whole point of the unify: an edge an AI creates via MCP must be the + // SAME representation the GraphQL server/web render — an Edge node — so it + // shows up for humans. Previously MCP wrote a direct (a)-[:TYPE]->(b) rel + // that the web's `edges` query (which returns Edge nodes) never saw. + const a = parse(await svc.createNode({ title: `Edge Vis A ${Date.now()}`, type: 'TASK' } as any)).node.id; + const b = parse(await svc.createNode({ title: `Edge Vis B ${Date.now()}`, type: 'TASK' } as any)).node.id; + createdNodes.push(a, b); + + await svc.createEdge({ source_id: a, target_id: b, type: 'BLOCKS' } as any); + + const session = driver.session(); + try { + // The Edge node exists exactly as the web GraphQL `edges` query expects + const edgeNode = await session.run( + 'MATCH (e:Edge)-[:EDGE_SOURCE]->(s:WorkItem {id: $a}) MATCH (e)-[:EDGE_TARGET]->(t:WorkItem {id: $b}) RETURN e.type AS type, e.id AS id', + { a, b } + ); + expect(edgeNode.records.length, 'an Edge NODE is created (web-visible)').toBe(1); + expect(edgeNode.records[0].get('type'), 'edge keeps its type').toBe('BLOCKS'); + expect(edgeNode.records[0].get('id'), 'edge has an id like server-created edges').toBeTruthy(); + + // And NOT a stray direct relationship (the old, web-invisible form) + const directRel = await session.run( + 'MATCH (:WorkItem {id: $a})-[r:BLOCKS]->(:WorkItem {id: $b}) RETURN count(r) AS c', + { a, b } + ); + expect(directRel.records[0].get('c').toNumber(), 'no legacy direct relationship').toBe(0); + } finally { + await session.close(); + } + + // deleteEdge removes the Edge node cleanly (no orphan Edge left behind) + await svc.deleteEdge({ source_id: a, target_id: b, type: 'BLOCKS' } as any); + const s2 = driver.session(); + try { + const after = await s2.run( + 'MATCH (e:Edge)-[:EDGE_SOURCE]->(:WorkItem {id: $a}) RETURN count(e) AS c', + { a } + ); + expect(after.records[0].get('c').toNumber(), 'edge node removed on delete').toBe(0); + } finally { + await s2.close(); + } + }); + it('browseGraph returns well-formed data over a real DB', async () => { const browsed = parse(await svc.browseGraph({ query_type: 'all_nodes', limit: 25 } as any)); const arr = browsed.nodes ?? browsed.results ?? browsed.workItems;