From 486345d5aa45088337401c8136de46e845e95f3e Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 10:31:34 -0700 Subject: [PATCH 1/7] Fix /mcp/status 503 spamming a console error on every page (#42) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skeptical sweep of all routes (desktop + phone) found every page logged a console error: '503 /mcp/status'. The MCP server is an OPTIONAL subsystem (we already gray out AI & Agents when it's offline), but the server's /mcp/status proxy returned 503 when MCP was unreachable — and browsers log failed resource loads (5xx) to the console regardless of try/catch, so it was unavoidable noise site-wide (2 console errors per page load). Fix: return 200 with { connected: false, status: 'offline' } for the offline case. The client already treats connected:false as offline, so behavior is unchanged — just no more console error. Verified: /mcp/status now 200, and /, /settings, /backend load with 0 console errors (was 2). Regression guard: user-smoke now fails on ANY 5xx from our own origin during the session and sweeps the main routes — this class of bug was invisible because the gate never checked non-GraphQL responses. Co-authored-by: Claude Fable 5 --- packages/server/src/index.ts | 10 ++++++++-- tests/e2e/user-smoke.spec.ts | 19 ++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) 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..1a5d90c5 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 }) => { From 423c93215edc22cb0cdd11040961b12c12c29345 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 10:40:50 -0700 Subject: [PATCH 2/7] Fix get_graph_context reporting an empty graph as "not found" (#43) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix get_graph_context reporting an empty graph as "not found" The type/status tally used `CALL { WITH items UNWIND items as i RETURN i.type as type, count(i) as cnt }`. UNWIND of an empty list yields ZERO rows, and a correlated CALL subquery returning zero rows eliminates the outer row — so for a brand-new empty graph the whole query returned no records and getGraphContext threw "Graph with ID X not found", even though the Graph node plainly existed. Return the raw type/status lists from Cypher and tally them in JS instead. The blockers/recent subqueries were already safe because they end in `collect(...)` (aggregation always yields one row). Adds two real-Neo4j contract cases: an empty graph returns zero counts (not an error), and a populated graph tallies byType/byStatus correctly. Co-Authored-By: Claude Opus 4.8 (1M context) * Add smoke guard: a brand-new empty graph shows its empty-state, not an error UI counterpart to the get_graph_context empty-graph fix. Verifies that creating a graph with zero work items and opening it renders the "Create Your First Work Item" invitation with no error chrome and no uncaught JS errors. Passes against the live dev stack. Co-Authored-By: Claude Opus 4.8 (1M context) * Update mock-neo4j for the new get_graph_context query shape getGraphContext now returns raw types/statuses lists (tallied in JS) instead of pre-aggregated typeCounts/statusCounts, so the mock driver's canned record must match. Match on the new query (size(items) as nodeCount / as statuses) and return raw label arrays. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../mcp-server/src/services/graph-service.ts | 34 +++++------ packages/mcp-server/tests/mock-neo4j.ts | 12 +--- .../mcp-server/tests/neo4j-contract.test.ts | 49 ++++++++++++++++ tests/e2e/user-smoke.spec.ts | 57 +++++++++++++++++++ 4 files changed, 124 insertions(+), 28 deletions(-) diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts index 8cb08116..c9e74071 100644 --- a/packages/mcp-server/src/services/graph-service.ts +++ b/packages/mcp-server/src/services/graph-service.ts @@ -3291,18 +3291,14 @@ export class GraphService { OPTIONAL MATCH (g)<-[:BELONGS_TO]-(w:WorkItem) OPTIONAL MATCH (w)-[e:DEPENDS_ON|BLOCKS|RELATES_TO|CONTAINS|PART_OF]-(:WorkItem) 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) @@ -3318,7 +3314,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 +3330,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 +3342,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 { diff --git a/packages/mcp-server/tests/mock-neo4j.ts b/packages/mcp-server/tests/mock-neo4j.ts index df11cf97..5579a1af 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 } } ], diff --git a/packages/mcp-server/tests/neo4j-contract.test.ts b/packages/mcp-server/tests/neo4j-contract.test.ts index 570d906a..f4db34a5 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,9 @@ 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) { + 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 +85,51 @@ 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('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/tests/e2e/user-smoke.spec.ts b/tests/e2e/user-smoke.spec.ts index 1a5d90c5..d09116ae 100644 --- a/tests/e2e/user-smoke.spec.ts +++ b/tests/e2e/user-smoke.spec.ts @@ -172,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 () => { From 1750f466bc5f4f0bb7c4be681d2e96097598f515 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 10:57:09 -0700 Subject: [PATCH 3/7] Fix clone_graph: throws on most graphs + corrupts every edge type (#44) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../mcp-server/src/services/graph-service.ts | 23 +++++---- .../mcp-server/tests/neo4j-contract.test.ts | 50 +++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts index c9e74071..cf1f287f 100644 --- a/packages/mcp-server/src/services/graph-service.ts +++ b/packages/mcp-server/src/services/graph-service.ts @@ -3586,10 +3586,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 }); @@ -3636,12 +3640,11 @@ export class GraphService { MATCH (sourceW)-[r:DEPENDS_ON|BLOCKS|RELATES_TO|CONTAINS|PART_OF]->(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 + // 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 `; const edgesResult = await tx.run(cloneEdgesQuery, { diff --git a/packages/mcp-server/tests/neo4j-contract.test.ts b/packages/mcp-server/tests/neo4j-contract.test.ts index f4db34a5..eadc6ff4 100644 --- a/packages/mcp-server/tests/neo4j-contract.test.ts +++ b/packages/mcp-server/tests/neo4j-contract.test.ts @@ -44,6 +44,9 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { 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 */ } @@ -130,6 +133,53 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { 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)-[rel]->(:WorkItem) RETURN type(rel) 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('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; From 8d75ca4b305c8c560b532e657cfd7b4dee501041 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 13:31:58 -0700 Subject: [PATCH 4/7] Fix invalid Docker image tag on PRs to main (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docker-publish.yml used `type=sha,prefix={{branch}}-`, but on a pull_request event the metadata-action `{{branch}}` placeholder is empty, so the computed tag became `ghcr.io/...:-` — an invalid reference that failed every build-and-push job on PRs targeting main (blocking the develop→main promotion / prod image pipeline). Use a static `sha-` prefix so the tag is valid on branch pushes, tags, and PRs alike. Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/docker-publish.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 9d11df70..a4f0683b 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 From 2c489a8cab7af27073b919d469123494d0e39a56 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 13:59:59 -0700 Subject: [PATCH 5/7] Unify MCP edges onto the Edge-node model (AI edges now visible to humans) (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The core "humans and AI as peers on one graph" promise was broken: MCP create_edge wrote a DIRECT Neo4j relationship (a)-[:TYPE]->(b), but the GraphQL server and web UI model edges as Edge NODES joined via EDGE_SOURCE / EDGE_TARGET. Proven live: an MCP-created edge produced 0 rows in the GraphQL `edges` query, so edges an AI created were invisible in the web UI (and human/web-created Edge nodes were invisible to every MCP read, which only traversed direct rels). The two layers were fully bifurcated. This migrates the ENTIRE MCP edge surface onto the canonical Edge-node model the server already uses: - Writes: createEdge, executeBulkCreateEdge (MERGE an Edge node + EDGE_SOURCE/EDGE_TARGET, idempotent); deleteEdge, executeBulkDeleteEdge (DETACH DELETE the Edge node). - Reads: getNodeDetails relationships, getGraphContext (edge count + blockers), getGraphDetails, browseGraph 'dependencies', cloneGraph edge clone — all traverse Edge nodes now. - Analytics: analyzeGraphHealth, getPriorityInsights, getBottlenecks, getContributorPriorities swap direct-rel patterns for Edge-node ones. - Variable-length: findPath and detectCycles use Neo4j 5 quantified path patterns / EDGE_SOURCE|EDGE_TARGET traversal and project back to WorkItem nodes + edge types (verified live: path A->C type DEPENDS_ON, cycle A->B->C->A len 3). Tests: new real-Neo4j contract case asserts an MCP-created edge is a web-visible Edge node (and NOT a stray direct rel), and that deleteEdge removes it. cloneGraph contract case now verifies via Edge nodes. Mock driver updated for the new relationships query + count(newE). Full CI-mode unit suite (305) green; contract suite (7) green. Co-authored-by: Claude Opus 4.8 (1M context) --- .../mcp-server/src/services/graph-service.ts | 178 ++++++++++-------- packages/mcp-server/tests/mock-neo4j.ts | 16 +- .../mcp-server/tests/neo4j-contract.test.ts | 48 ++++- 3 files changed, 159 insertions(+), 83 deletions(-) diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts index cf1f287f..69667f9e 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 5579a1af..9e10d2f9 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 eadc6ff4..53d2f5c9 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; From 0e9482d9864fe092f8e923c6e90d4ea03353ba18 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 14:11:26 -0700 Subject: [PATCH 6/7] Fix build-stack manifest tag on PRs (#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build-stack job assembled the multi-image stack manifest from a `${GITHUB_SHA::8}` tag that the build-and-push job never pushes, so it failed on every PR (e.g. the develop→main promotion). Use a tag that is actually published: `pr-` on pull_request events (matches metadata-action's type=ref,event=pr), `latest` on main, the branch name on other branch pushes, and a `sha-` fallback that matches the type=sha tag. Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/docker-publish.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a4f0683b..c75e8a42 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -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 From 189e43d7e6e4f86f9fd3be46ecd21636015f34bc Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 14:31:20 -0700 Subject: [PATCH 7/7] create_node: attach to a graph + write canonical WorkItem fields (parity with web) (#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * create_node: attach to a graph via graph_id (orphan nodes were invisible) Node-level twin of the edge parity fix (#47). The web lists nodes per-graph (workItems where graph.id = currentGraph), but MCP create_node had no graph_id parameter and never created a BELONGS_TO relationship, so every node an AI created was orphaned and shown in NO graph view. This also undermined the edge fix: an AI's nodes never appeared, so neither did the edges between them. Add an optional graph_id to the create_node tool + CreateNodeArgs; when present, MATCH the graph and MERGE (n)-[:BELONGS_TO]->(g). A graph_id that matches no graph returns a clean error instead of silently creating an unattached node. Omitting graph_id stays backward-compatible. Real-Neo4j contract test covers all three: attached (BELONGS_TO + shows in get_graph_context), and unknown graph_id errors. Co-Authored-By: Claude Opus 4.8 (1M context) * create_node: write canonical WorkItem fields the web reads MCP node creation also used non-canonical property names the GraphQL server/web schema don't read: priorityComputed (vs priorityComp), sphericalRadius/Theta/Phi (vs radius/theta/phi), and it never set positionX/Y/Z or priority at all. So AI-created nodes showed priority 0 at the origin in the web, and MCP was internally inconsistent (createNode + browseGraph used priorityComputed while updateNode and the priority/bottleneck analytics used priorityComp — updates were invisible to browse-by-priority and vice versa). Standardize createNode and executeBulkCreateNode on the canonical schema fields (positionX/Y/Z, radius, theta, phi, priority, priorityComp) while keeping the MCP-internal priorityExec/Indiv/Comm decomposition, and fix browseGraph by_priority to read priorityComp. Contract test now also asserts the canonical fields are set and no legacy priorityComputed remains. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- packages/mcp-server/src/index.ts | 4 ++ .../mcp-server/src/services/graph-service.ts | 67 +++++++++++++------ .../mcp-server/tests/neo4j-contract.test.ts | 46 +++++++++++++ 3 files changed, 98 insertions(+), 19 deletions(-) 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 69667f9e..12bfcd33 100644 --- a/packages/mcp-server/src/services/graph-service.ts +++ b/packages/mcp-server/src/services/graph-service.ts @@ -63,6 +63,7 @@ export interface CreateNodeArgs { status?: NodeStatus; contributor_ids?: string[]; metadata?: NodeMetadata; + graph_id?: string; } export interface UpdateNodeArgs { @@ -330,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 `; @@ -465,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, @@ -478,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, @@ -499,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 @@ -2177,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 diff --git a/packages/mcp-server/tests/neo4j-contract.test.ts b/packages/mcp-server/tests/neo4j-contract.test.ts index 53d2f5c9..4b377726 100644 --- a/packages/mcp-server/tests/neo4j-contract.test.ts +++ b/packages/mcp-server/tests/neo4j-contract.test.ts @@ -226,6 +226,52 @@ describe.skipIf(!RUN)('MCP GraphService — real Neo4j contract', () => { } }); + 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;