diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 9d11df70..c75e8a42 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -61,7 +61,7 @@ jobs: type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=raw,value=latest,enable={{is_default_branch}} - type=sha,prefix={{branch}}- + type=sha,prefix=sha- - name: Build and push Docker image uses: docker/build-push-action@v5 @@ -102,12 +102,19 @@ jobs: - name: Determine tag id: tag run: | - if [[ "${{ github.ref }}" == "refs/heads/main" ]]; then + # Must match a tag the build-and-push job actually pushed (see the + # docker/metadata-action tags above), or imagetools can't find the + # per-image manifests. On PRs metadata pushes `pr-`; on main + # it pushes `latest`. The old `${GITHUB_SHA::8}` fallback was never a + # pushed tag, so build-stack failed on every PR. + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + echo "tag=pr-${{ github.event.number }}" >> $GITHUB_OUTPUT + elif [[ "${{ github.ref }}" == "refs/heads/main" ]]; then echo "tag=latest" >> $GITHUB_OUTPUT elif [[ "${{ github.ref }}" == refs/heads/* ]]; then echo "tag=${GITHUB_REF#refs/heads/}" | sed 's/\//-/g' >> $GITHUB_OUTPUT else - echo "tag=${GITHUB_SHA::8}" >> $GITHUB_OUTPUT + echo "tag=sha-${GITHUB_SHA::7}" >> $GITHUB_OUTPUT fi - name: Create and push manifest for stack diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index bf91054f..1a49e03a 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -116,6 +116,10 @@ const tools: Tool[] = [ items: { type: 'string' }, description: 'Array of contributor IDs' }, + graph_id: { + type: 'string', + description: 'ID of the graph/project this node belongs to. Strongly recommended: the web UI lists nodes per-graph, so a node created without a graph_id is not shown in any graph view. Use list_graphs / create_graph to obtain one.' + }, metadata: { type: 'object', description: 'Additional metadata for the node' diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts index 8cb08116..12bfcd33 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, @@ -64,6 +63,7 @@ export interface CreateNodeArgs { status?: NodeStatus; contributor_ids?: string[]; metadata?: NodeMetadata; + graph_id?: string; } export interface UpdateNodeArgs { @@ -331,12 +331,12 @@ export class GraphService { case 'by_priority': const minPriority = filters.min_priority || 0; - countQuery = `MATCH (n:WorkItem) WHERE n.priorityComputed >= $min_priority RETURN count(n) as total`; + countQuery = `MATCH (n:WorkItem) WHERE n.priorityComp >= $min_priority RETURN count(n) as total`; query = ` MATCH (n:WorkItem) - WHERE n.priorityComputed >= $min_priority + WHERE n.priorityComp >= $min_priority RETURN n - ORDER BY n.priorityComputed DESC + ORDER BY n.priorityComp DESC SKIP $offset LIMIT $limit `; @@ -349,9 +349,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 `; @@ -466,11 +466,17 @@ export class GraphService { // Generate truly unique ID to prevent race conditions const id = generateUniqueNodeId(); const now = new Date().toISOString(); - + const graphId = args.graph_id ? sanitizeNodeId(args.graph_id) : null; + // Use write consistency to prevent read-after-write issues return await withWriteConsistency(id, 'CREATE', async () => { + // Attach the node to its graph via BELONGS_TO when a graph_id is given. + // Without this a node is orphaned — the web UI lists nodes per-graph + // (workItems where graph.id = currentGraph), so an unattached node an AI + // creates is invisible to humans. const query = ` + ${graphId ? 'MATCH (g:Graph {id: $graphId})' : ''} CREATE (n:WorkItem { id: $id, title: $title, @@ -479,19 +485,24 @@ export class GraphService { status: $status, createdAt: $now, updatedAt: $now, - priorityExecutive: 0, - priorityIndividual: 0, - priorityCommunity: 0, - priorityComputed: 0, - sphericalRadius: 1.0, - sphericalTheta: 0, - sphericalPhi: 0, + positionX: 0, + positionY: 0, + positionZ: 0, + radius: 1.0, + theta: 0, + phi: 0, + priority: 0, + priorityComp: 0, + priorityExec: 0, + priorityIndiv: 0, + priorityComm: 0, metadata: $metadata }) + ${graphId ? 'MERGE (n)-[:BELONGS_TO]->(g)' : ''} RETURN n `; - const params = { + const params: Record = { id, title, description, @@ -500,8 +511,21 @@ export class GraphService { now, metadata: JSON.stringify(metadata) }; + if (graphId) params.graphId = graphId; const result = await session.run(query, params); + + // A graph_id that matches no graph yields zero rows (the MATCH fails) — + // surface that instead of silently creating nothing. + if (graphId && result.records.length === 0) { + return { + content: [{ + type: 'text', + text: JSON.stringify({ error: `Graph with id '${graphId}' not found`, graph_id: graphId }, null, 2) + }], + isError: true + }; + } const rawNode = result.records[0].get('n').properties; // Parse metadata back from JSON string for data integrity @@ -884,24 +908,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 +962,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 +1020,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 +1083,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 +1143,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 +1192,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 +1228,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 +1260,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 +1641,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 +1709,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 +1740,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 +1868,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 +1887,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, @@ -2168,13 +2202,17 @@ export class GraphService { status: $status, createdAt: $now, updatedAt: $now, - priorityExecutive: 0, - priorityIndividual: 0, - priorityCommunity: 0, - priorityComputed: 0, - sphericalRadius: 1.0, - sphericalTheta: 0, - sphericalPhi: 0, + positionX: 0, + positionY: 0, + positionZ: 0, + radius: 1.0, + theta: 0, + phi: 0, + priority: 0, + priorityComp: 0, + priorityExec: 0, + priorityIndiv: 0, + priorityComm: 0, metadata: $metadata }) RETURN n.id as id @@ -2227,17 +2265,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 +2294,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 +2556,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 +3274,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,24 +3330,21 @@ 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 - CALL { - WITH items - UNWIND items as i - RETURN i.type as type, count(i) as cnt - } - WITH g, items, edgeCount, collect({type: type, count: cnt}) as typeCounts - CALL { - WITH items - UNWIND items as i - RETURN i.status as status, count(i) as cnt - } - WITH g, items, edgeCount, typeCounts, collect({status: status, count: cnt}) as statusCounts + // Return the raw type/status lists and tally them in JS. A + // CALL { UNWIND items ... } subquery returns ZERO rows for an empty + // graph (UNWIND of an empty list), which dropped the whole result and + // made get_graph_context wrongly report a brand-new empty graph as + // "not found". + WITH g, items, edgeCount, + [x IN items | x.type] as types, + [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 @@ -3318,7 +3356,7 @@ export class GraphService { ORDER BY rw.updatedAt DESC LIMIT 5 RETURN collect({id: rw.id, title: rw.title, status: rw.status, type: rw.type, updatedAt: rw.updatedAt}) as recent } - RETURN g, size(items) as nodeCount, edgeCount, typeCounts, statusCounts, blockers, recent + RETURN g, size(items) as nodeCount, edgeCount, types, statuses, blockers, recent `; const result = await session.run(query, { graphId: args.graphId }); @@ -3334,8 +3372,8 @@ export class GraphService { const record = result.records[0]; const g = record.get('g').properties; - const typeCounts = (record.get('typeCounts') || []) as Array<{ type: string; count: unknown }>; - const statusCounts = (record.get('statusCounts') || []) as Array<{ status: string; count: unknown }>; + const types = (record.get('types') || []) as Array; + const statuses = (record.get('statuses') || []) as Array; const blockers = (record.get('blockers') || []) as Array<{ id: string; title: string; blocksCount: unknown }>; const recent = (record.get('recent') || []) as Array<{ id: string; @@ -3346,12 +3384,12 @@ export class GraphService { }>; const byType: Record = {}; - for (const t of typeCounts) { - if (t.type) byType[t.type] = toNum(t.count); + for (const t of types) { + if (t) byType[t] = (byType[t] ?? 0) + 1; } const byStatus: Record = {}; - for (const s of statusCounts) { - if (s.status) byStatus[s.status] = toNum(s.count); + for (const s of statuses) { + if (s) byStatus[s] = (byStatus[s] ?? 0) + 1; } return { @@ -3590,10 +3628,14 @@ export class GraphService { const newGraph = await tx.run(createGraphQuery, { newName: args.newName, description: `Cloned from: ${sourceGraph.name}`, - type: sourceGraph.type, - teamId: args.teamId || sourceGraph.teamId, - isShared: sourceGraph.isShared, - settings: sourceGraph.settings, + type: sourceGraph.type ?? 'PROJECT', + // Neo4j does not store null-valued properties, so a graph created + // without a teamId has NO teamId property — reading it back yields + // `undefined`, and the driver rejects an undefined param value with + // ParameterMissing. Coalesce to null so clone works for every graph. + teamId: args.teamId ?? sourceGraph.teamId ?? null, + isShared: sourceGraph.isShared ?? false, + settings: sourceGraph.settings ?? '{}', sourceGraphId: args.sourceGraphId }); @@ -3633,19 +3675,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 - CREATE (newW)-[newR:DEPENDS_ON { - type: r.type, - weight: r.weight, - metadata: r.metadata - }]->(newTargetW) - RETURN count(newR) 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 df11cf97..9e10d2f9 100644 --- a/packages/mcp-server/tests/mock-neo4j.ts +++ b/packages/mcp-server/tests/mock-neo4j.ts @@ -226,7 +226,7 @@ export function createMockDriver(): Driver { } // Handle compact graph context query (get_graph_context, AI-6) - if (query.includes('typeCounts') && query.includes('statusCounts')) { + if (query.includes('size(items) as nodeCount') && query.includes('as statuses')) { if (params?.graphId === 'missing-graph-id') { return { records: [] }; } @@ -241,14 +241,8 @@ export function createMockDriver(): Driver { }, nodeCount: { toNumber: () => 12 }, edgeCount: { toNumber: () => 7 }, - typeCounts: [ - { type: 'TASK', count: { toNumber: () => 8 } }, - { type: 'BUG', count: { toNumber: () => 4 } } - ], - statusCounts: [ - { status: 'IN_PROGRESS', count: { toNumber: () => 5 } }, - { status: 'BLOCKED', count: { toNumber: () => 2 } } - ], + types: ['TASK', 'TASK', 'TASK', 'TASK', 'TASK', 'TASK', 'TASK', 'TASK', 'BUG', 'BUG', 'BUG', 'BUG'], + statuses: ['IN_PROGRESS', 'IN_PROGRESS', 'IN_PROGRESS', 'IN_PROGRESS', 'IN_PROGRESS', 'BLOCKED', 'BLOCKED'], blockers: [ { id: 'node-1', title: 'Fix auth', blocksCount: { toNumber: () => 3 } } ], @@ -307,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 }, @@ -328,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 570d906a..4b377726 100644 --- a/packages/mcp-server/tests/neo4j-contract.test.ts +++ b/packages/mcp-server/tests/neo4j-contract.test.ts @@ -29,6 +29,7 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { let driver: Driver; let svc: GraphService; const createdNodes: string[] = []; + const createdGraphs: string[] = []; beforeAll(async () => { driver = neo4j.driver(URI, neo4j.auth.basic(USER, PASS)); @@ -42,6 +43,12 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { if (createdNodes.length) { await session?.run('MATCH (n:WorkItem) WHERE n.id IN $ids DETACH DELETE n', { ids: createdNodes }); } + if (createdGraphs.length) { + // Also remove any WorkItems that belong to these graphs (clone creates + // brand-new node ids we don't otherwise track). + await session?.run('MATCH (g:Graph) WHERE g.id IN $ids OPTIONAL MATCH (g)<-[:BELONGS_TO]-(w:WorkItem) DETACH DELETE w', { ids: createdGraphs }); + await session?.run('MATCH (g:Graph) WHERE g.id IN $ids DETACH DELETE g', { ids: createdGraphs }); + } } catch { /* ignore */ } await session?.close(); await driver?.close(); @@ -81,6 +88,190 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { expect(delEdge).toMatch(/delet|success|true|removed/); }); + it('getGraphContext on a brand-new EMPTY graph returns zero counts, not "not found"', async () => { + // Regression: the type/status tally used `CALL { UNWIND items ... }`, and + // UNWIND of an empty list yields ZERO rows, which dropped the whole result + // row — so an existing empty graph was reported as "not found". + const created = parse(await svc.createGraph({ name: `Contract Empty ${Date.now()}`, type: 'PROJECT' } as any)); + const graphId = created.graph.id; + expect(graphId, 'createGraph persists and returns an id').toBeTruthy(); + createdGraphs.push(graphId); + + const ctx = parse(await svc.getGraphContext({ graphId } as any)).context; + expect(ctx.graph.id, 'the empty graph is found by id').toBe(graphId); + expect(ctx.counts.nodes, 'empty graph has zero nodes').toBe(0); + expect(ctx.counts.edges, 'empty graph has zero edges').toBe(0); + expect(ctx.counts.byType, 'no type tallies on an empty graph').toEqual({}); + expect(ctx.counts.byStatus, 'no status tallies on an empty graph').toEqual({}); + expect(Array.isArray(ctx.topBlockers) && ctx.topBlockers.length, 'no blockers').toBe(0); + expect(Array.isArray(ctx.recentActivity) && ctx.recentActivity.length, 'no recent activity').toBe(0); + }); + + it('getGraphContext tallies type/status once a graph has items', async () => { + const g = parse(await svc.createGraph({ name: `Contract Populated ${Date.now()}`, type: 'PROJECT' } as any)); + const graphId = g.graph.id; + createdGraphs.push(graphId); + + // Attach two TASK/IN_PROGRESS items to this graph + const session = driver.session(); + try { + for (const i of [1, 2]) { + const id = parse(await svc.createNode({ title: `Pop ${i} ${Date.now()}`, type: 'TASK', status: 'IN_PROGRESS' } as any)).node.id; + createdNodes.push(id); + await session.run( + 'MATCH (w:WorkItem {id: $id}), (g:Graph {id: $gid}) MERGE (w)-[:BELONGS_TO]->(g)', + { id, gid: graphId } + ); + } + } finally { + await session.close(); + } + + const ctx = parse(await svc.getGraphContext({ graphId } as any)).context; + expect(ctx.counts.nodes, 'two items counted').toBe(2); + expect(ctx.counts.byType.TASK, 'both items tallied under TASK').toBe(2); + expect(ctx.counts.byStatus.IN_PROGRESS, 'both items tallied under IN_PROGRESS').toBe(2); + }); + + it('cloneGraph copies nodes AND preserves each edge type (not all DEPENDS_ON)', async () => { + // Regressions this guards: + // 1. clone threw ParameterMissing(teamId) because Neo4j never stores a + // null teamId, so reading it back yields undefined. + // 2. clone hard-coded :DEPENDS_ON, silently rewriting BLOCKS/RELATES_TO/… + const src = parse(await svc.createGraph({ name: `Contract Clone Src ${Date.now()}`, type: 'PROJECT' } as any)); + const srcId = src.graph.id; + createdGraphs.push(srcId); + + const session = driver.session(); + const mk = async (title: string) => { + const id = parse(await svc.createNode({ title, type: 'TASK', status: 'PROPOSED' } as any)).node.id; + createdNodes.push(id); + await session.run('MATCH (w:WorkItem {id: $id}), (g:Graph {id: $gid}) MERGE (w)-[:BELONGS_TO]->(g)', { id, gid: srcId }); + return id; + }; + let a: string, b: string, c: string; + try { + a = await mk(`Clone A ${Date.now()}`); + b = await mk(`Clone B ${Date.now()}`); + c = await mk(`Clone C ${Date.now()}`); + } finally { + await session.close(); + } + await svc.createEdge({ source_id: a, target_id: b, type: 'BLOCKS' } as any); + await svc.createEdge({ source_id: b, target_id: c, type: 'RELATES_TO' } as any); + + const cloned = parse(await svc.cloneGraph({ sourceGraphId: srcId, newName: `Contract Clone Dst ${Date.now()}` } as any)); + const dstId = cloned.newGraph.id; + createdGraphs.push(dstId); + expect(cloned.newGraph.clonedNodes, 'all three nodes cloned').toBe(3); + expect(cloned.newGraph.clonedEdges, 'both edges cloned').toBe(2); + + // The cloned edges must keep their real types, not all become DEPENDS_ON + const s2 = driver.session(); + try { + const r = await s2.run( + '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(); + expect(types, 'cloned relationship types are preserved').toEqual(['BLOCKS', 'RELATES_TO']); + } finally { + await s2.close(); + } + }); + + 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('createNode with graph_id attaches via BELONGS_TO (otherwise the node is invisible per-graph)', async () => { + // The web lists nodes per-graph (workItems where graph.id = currentGraph), + // so a node created without a graph link is invisible to humans. createNode + // must attach to the given graph. + const g = parse(await svc.createGraph({ name: `Contract Attach ${Date.now()}`, type: 'PROJECT' } as any)); + const graphId = g.graph.id; + createdGraphs.push(graphId); + + const created = parse(await svc.createNode({ title: `Attached ${Date.now()}`, type: 'TASK', graph_id: graphId } as any)); + const id = created.node.id; + createdNodes.push(id); + + const session = driver.session(); + try { + const r = await session.run( + 'MATCH (g:Graph {id: $gid})<-[:BELONGS_TO]-(w:WorkItem {id: $id}) RETURN count(*) AS c', + { gid: graphId, id } + ); + expect(r.records[0].get('c').toNumber(), 'node is linked to its graph via BELONGS_TO').toBe(1); + + // The node must carry the CANONICAL schema fields the web reads + // (positionX/Y/Z, radius, theta, phi, priority, priorityComp) — not the + // old MCP-only names (priorityComputed / sphericalRadius) the web ignores. + const fields = await session.run( + 'MATCH (w:WorkItem {id: $id}) RETURN w.positionX AS px, w.radius AS radius, w.priority AS priority, w.priorityComp AS pc, w.priorityComputed AS legacy', + { id } + ); + const rec = fields.records[0]; + expect(rec.get('px'), 'positionX is set (web reads it)').not.toBeNull(); + expect(rec.get('radius'), 'radius is set').not.toBeNull(); + expect(rec.get('priority'), 'priority is set').not.toBeNull(); + expect(rec.get('pc'), 'priorityComp is set (canonical)').not.toBeNull(); + expect(rec.get('legacy'), 'no legacy priorityComputed property').toBeNull(); + } finally { + await session.close(); + } + + // get_graph_context should now count this node + const ctx = parse(await svc.getGraphContext({ graphId } as any)).context; + expect(ctx.counts.nodes, 'attached node shows in the graph context').toBeGreaterThanOrEqual(1); + + // A non-existent graph_id is a clean error, not a silent orphan + const bad = await svc.createNode({ title: 'Bad Graph', type: 'TASK', graph_id: 'no-such-graph' } as any); + expect((bad as { isError?: boolean }).isError, 'unknown graph_id errors').toBe(true); + }); + 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; diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index c0269cb2..2b118702 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -569,14 +569,20 @@ async function startServer() { ...mcpStatus }); } else { - res.status(503).json({ + // MCP being offline is a NORMAL, expected state (the MCP server is an + // optional subsystem). Report it with 200 + connected:false so the + // browser doesn't log a failed-resource error on every page that polls + // this; the client already treats connected:false as "offline". + res.json({ connected: false, + status: 'offline', error: 'MCP server returned an error' }); } } catch (error) { - res.status(503).json({ + res.json({ connected: false, + status: 'offline', error: error instanceof Error ? error.message : 'Connection failed' }); } diff --git a/tests/e2e/user-smoke.spec.ts b/tests/e2e/user-smoke.spec.ts index 87b410bd..d09116ae 100644 --- a/tests/e2e/user-smoke.spec.ts +++ b/tests/e2e/user-smoke.spec.ts @@ -14,9 +14,17 @@ test.describe('user smoke: the app works from a user point of view @smoke', () = test('login → graph renders nodes AND edges → no errors anywhere', async ({ page }) => { const pageErrors: string[] = []; const gqlErrors: string[] = []; + const serverErrors: string[] = []; page.on('pageerror', (e) => pageErrors.push(e.message)); page.on('response', async (res) => { - if (!res.url().includes('graphql')) return; + // Any 5xx from our own origin is a server fault the user shouldn't hit — + // e.g. /mcp/status used to 503 on every page when MCP was offline (a + // NORMAL state), logging a console error site-wide. + const url = res.url(); + if (res.status() >= 500 && (url.includes('localhost:4127') || url.includes('localhost:3127') || url.includes('/api/'))) { + serverErrors.push(`${res.status()} ${url.replace(/^https?:\/\/[^/]+/, '')}`); + } + if (!url.includes('graphql')) return; try { const body = await res.json(); if (body?.errors?.length) { @@ -77,6 +85,15 @@ test.describe('user smoke: the app works from a user point of view @smoke', () = // 5) No uncaught JS errors expect(pageErrors, `uncaught page errors: ${pageErrors[0] ?? ''}`).toEqual([]); + + // 6) Visit the main routes and confirm none of them produce a 5xx from our + // own origin (catches optional-subsystem endpoints returning 503 for a + // normal "offline" state, which logs a console error site-wide). + for (const route of ['/settings', '/backend', '/ontology', '/']) { + await page.goto(route).catch(() => {}); + await page.waitForTimeout(2500); + } + expect(serverErrors, `server 5xx responses during the session: ${serverErrors[0] ?? ''}`).toEqual([]); }); test('grow flow stays healthy: + → empty space → connected named node @smoke', async ({ page }) => { @@ -155,6 +172,63 @@ test.describe('user smoke: the app works from a user point of view @smoke', () = }, name); }); + // A brand-new EMPTY graph (the very first thing a user sees after "Create + // Graph") must render its empty-state invitation, NOT crash or show error + // chrome. UI counterpart to the get_graph_context "empty graph reported as + // not found" bug — the empty case is a first-class state. + test('a brand-new empty graph shows the empty-state, not an error @smoke', async ({ page }) => { + const pageErrors: string[] = []; + page.on('pageerror', (e) => pageErrors.push(e.message)); + + await login(page, TEST_USERS.ADMIN); + await page.waitForTimeout(2000); + + const graphId = await page.evaluate(async () => { + const token = localStorage.getItem('authToken') ?? ''; + const post = (query: string, variables?: unknown) => + fetch('/api/graphql', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, + body: JSON.stringify({ query, variables }), + }).then((r) => r.json()); + const me = await post('{ me { id } }'); + const userId = me.data.me.id; + const g = await post( + `mutation($input: [GraphCreateInput!]!) { createGraphs(input: $input) { graphs { id } } }`, + { input: [{ name: `Empty Smoke ${Date.now()}`, type: 'PROJECT', status: 'ACTIVE', createdBy: userId, isShared: true }] } + ); + return g.data.createGraphs.graphs[0].id as string; + }); + expect(graphId, 'empty graph created').toBeTruthy(); + + try { + await page.evaluate((gid) => localStorage.setItem('currentGraphId', gid), graphId); + await page.reload(); + await page.waitForTimeout(6000); + + await expect( + page.locator('text=/Create Your First Work Item|Transform Your Vision/i').first(), + 'empty graph shows its create-first-item invitation' + ).toBeVisible({ timeout: 10000 }); + + const errorBadges = await page + .locator('.graph-container') + .locator('text=/^Error$|not found|failed to load|connection lost/i') + .count(); + expect(errorBadges, 'no error chrome on an empty graph').toBe(0); + expect(pageErrors, `uncaught page errors on empty graph: ${pageErrors[0] ?? ''}`).toEqual([]); + } finally { + await page.evaluate(async (gid) => { + const token = localStorage.getItem('authToken') ?? ''; + await fetch('/api/graphql', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, + body: JSON.stringify({ query: `mutation($id: ID!) { deleteGraphs(where: { id: $id }) { nodesDeleted } }`, variables: { id: gid } }), + }); + }, graphId); + } + }); + test('data integrity: no orphan edges in the database @smoke', async ({ page }) => { await login(page, TEST_USERS.ADMIN); const orphans = await page.evaluate(async () => {