From 796153bdb1fdff7d62a4e9aacec2432bb3580238 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 26 May 2026 14:55:27 -0700 Subject: [PATCH 01/11] feat(mcp): add patch_dashboard, get_dashboard_tile, search_dashboards tools (HDX-4139) Add three new MCP dashboard tools for more granular operations: - hyperdx_get_dashboard_tile: retrieve a single tile by tileId without loading the full dashboard - hyperdx_patch_dashboard: update dashboard name/tags and/or replace a single tile by tileId in one call, preserving all other tiles and falling back to existing layout when omitted - hyperdx_search_dashboards: search dashboards by name (case-insensitive) and/or tags Additional improvements: - Fix empty parameter schema on patch_dashboard and search_dashboards caused by Zod .refine() wrapping (moved cross-field validation to handler body so inputSchema stays a plain z.object) - Add 'prefer patch_dashboard' hint to save_dashboard description - Document Lucene substring matching limitation prominently in tool descriptions and query guide prompt (field:value is ilike not equality, field:val* is prefix-within-substring not true prefix) - Split monolithic dashboards.test.ts (3040 lines) into 8 focused test files under __tests__/dashboards/ --- packages/api/jest.config.js | 2 +- .../api/src/mcp/__tests__/dashboards.test.ts | 2509 ----------------- .../dashboards/deleteDashboard.test.ts | 38 + .../__tests__/dashboards/getDashboard.test.ts | 65 + .../dashboards/getDashboardTile.test.ts | 83 + .../dashboards/patchDashboard.test.ts | 322 +++ .../mcp/__tests__/dashboards/prompts.test.ts | 576 ++++ .../__tests__/dashboards/queryTile.test.ts | 98 + .../dashboards/saveDashboard.test.ts | 1774 ++++++++++++ .../dashboards/searchDashboards.test.ts | 151 + .../api/src/mcp/__tests__/dashboards/setup.ts | 81 + .../api/src/mcp/prompts/dashboards/content.ts | 46 +- .../mcp/tools/dashboards/getDashboardTile.ts | 83 + .../api/src/mcp/tools/dashboards/index.ts | 6 + .../mcp/tools/dashboards/patchDashboard.ts | 349 +++ .../src/mcp/tools/dashboards/saveDashboard.ts | 3 +- .../api/src/mcp/tools/dashboards/schemas.ts | 39 +- .../mcp/tools/dashboards/searchDashboards.ts | 85 + packages/api/src/mcp/tools/query/schemas.ts | 11 +- 19 files changed, 3793 insertions(+), 2528 deletions(-) delete mode 100644 packages/api/src/mcp/__tests__/dashboards.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/deleteDashboard.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/getDashboard.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/getDashboardTile.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/prompts.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/queryTile.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts create mode 100644 packages/api/src/mcp/__tests__/dashboards/setup.ts create mode 100644 packages/api/src/mcp/tools/dashboards/getDashboardTile.ts create mode 100644 packages/api/src/mcp/tools/dashboards/patchDashboard.ts create mode 100644 packages/api/src/mcp/tools/dashboards/searchDashboards.ts diff --git a/packages/api/jest.config.js b/packages/api/jest.config.js index 8fc45f0f9d..bfb2f9c832 100644 --- a/packages/api/jest.config.js +++ b/packages/api/jest.config.js @@ -10,7 +10,7 @@ module.exports = { testEnvironment: 'node', verbose: true, rootDir: './src', - testMatch: ['**/__tests__/*.test.ts?(x)'], + testMatch: ['**/__tests__/**/*.test.ts?(x)'], testTimeout: 30000, moduleNameMapper: { '@/(.*)$': '/$1', diff --git a/packages/api/src/mcp/__tests__/dashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards.test.ts deleted file mode 100644 index 6ed04e2ff9..0000000000 --- a/packages/api/src/mcp/__tests__/dashboards.test.ts +++ /dev/null @@ -1,2509 +0,0 @@ -import { MetricsDataType, SourceKind } from '@hyperdx/common-utils/dist/types'; -import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import mongoose from 'mongoose'; - -import * as config from '@/config'; -import { - DEFAULT_DATABASE, - DEFAULT_TRACES_TABLE, - getLoggedInAgent, - getServer, -} from '@/fixtures'; -import Connection from '@/models/connection'; -import Dashboard from '@/models/dashboard'; -import { Source } from '@/models/source'; -import type { ExternalDashboardTileWithId } from '@/utils/zod'; - -import { - buildCreateDashboardPrompt, - buildDashboardExamplesPrompt, - buildQueryGuidePrompt, -} from '../prompts/dashboards/content'; -import { buildSourceSummary } from '../prompts/dashboards/helpers'; -import { McpContext } from '../tools/types'; -import { callTool, createTestClient, getFirstText } from './mcpTestUtils'; - -describe('MCP Dashboard Tools', () => { - const server = getServer(); - let team: any; - let user: any; - let traceSource: any; - let connection: any; - let client: Client; - - beforeAll(async () => { - await server.start(); - }); - - beforeEach(async () => { - const result = await getLoggedInAgent(server); - team = result.team; - user = result.user; - - connection = await Connection.create({ - team: team._id, - name: 'Default', - host: config.CLICKHOUSE_HOST, - username: config.CLICKHOUSE_USER, - password: config.CLICKHOUSE_PASSWORD, - }); - - traceSource = await Source.create({ - kind: SourceKind.Trace, - team: team._id, - from: { - databaseName: DEFAULT_DATABASE, - tableName: DEFAULT_TRACES_TABLE, - }, - timestampValueExpression: 'Timestamp', - connection: connection._id, - name: 'Traces', - }); - - const context: McpContext = { - teamId: team._id.toString(), - userId: user._id.toString(), - }; - client = await createTestClient(context); - }); - - afterEach(async () => { - await client?.close(); - await server.clearDBs(); - }); - - afterAll(async () => { - await server.stop(); - }); - - describe('hyperdx_get_dashboard', () => { - it('should list all dashboards when no id provided', async () => { - await new Dashboard({ - name: 'Dashboard 1', - tiles: [], - team: team._id, - tags: ['tag1'], - }).save(); - await new Dashboard({ - name: 'Dashboard 2', - tiles: [], - team: team._id, - tags: ['tag2'], - }).save(); - - const result = await callTool(client, 'hyperdx_get_dashboard', {}); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output).toHaveLength(2); - expect(output[0]).toHaveProperty('id'); - expect(output[0]).toHaveProperty('name'); - expect(output[0]).toHaveProperty('tags'); - }); - - it('should get dashboard detail when id is provided', async () => { - const dashboard = await new Dashboard({ - name: 'My Dashboard', - tiles: [], - team: team._id, - tags: ['test'], - }).save(); - - const result = await callTool(client, 'hyperdx_get_dashboard', { - id: dashboard._id.toString(), - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.id).toBe(dashboard._id.toString()); - expect(output.name).toBe('My Dashboard'); - expect(output.tags).toEqual(['test']); - expect(output.tiles).toEqual([]); - }); - - it('should return error for non-existent dashboard id', async () => { - const fakeId = '000000000000000000000000'; - const result = await callTool(client, 'hyperdx_get_dashboard', { - id: fakeId, - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - }); - - describe('hyperdx_save_dashboard', () => { - it('should create a new dashboard with tiles', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'New MCP Dashboard', - tiles: [ - { - name: 'Line Chart', - x: 0, - y: 0, - w: 12, - h: 4, - config: { - displayType: 'line', - sourceId, - select: [{ aggFn: 'count', where: '' }], - }, - }, - ], - tags: ['mcp-test'], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.id).toBeDefined(); - expect(output.name).toBe('New MCP Dashboard'); - expect(output.tiles).toHaveLength(1); - expect(output.tiles[0].config.displayType).toBe('line'); - expect(output.tags).toEqual(['mcp-test']); - - // Verify in database - const dashboard = await Dashboard.findById(output.id); - expect(dashboard).not.toBeNull(); - expect(dashboard?.name).toBe('New MCP Dashboard'); - }); - - it('should create a dashboard with a markdown tile', async () => { - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Markdown Dashboard', - tiles: [ - { - name: 'Notes', - config: { - displayType: 'markdown', - markdown: '# Hello World', - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles).toHaveLength(1); - expect(output.tiles[0].config.displayType).toBe('markdown'); - }); - - it('should update an existing dashboard', async () => { - const sourceId = traceSource._id.toString(); - - // Create first - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Original Name', - tiles: [ - { - name: 'Tile 1', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - const created = JSON.parse(getFirstText(createResult)); - - // Update - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'Updated Name', - tiles: [ - { - name: 'Updated Tile', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - tags: ['updated'], - }); - - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.id).toBe(created.id); - expect(updated.name).toBe('Updated Name'); - expect(updated.tiles).toHaveLength(1); - expect(updated.tiles[0].name).toBe('Updated Tile'); - expect(updated.tiles[0].config.displayType).toBe('table'); - }); - - it('should return error for missing source ID', async () => { - const fakeSourceId = '000000000000000000000000'; - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Bad Dashboard', - tiles: [ - { - name: 'Bad Tile', - config: { - displayType: 'line', - sourceId: fakeSourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('source'); - }); - - it('should return error when updating non-existent dashboard', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - id: '000000000000000000000000', - name: 'Ghost Dashboard', - tiles: [ - { - name: 'Tile', - config: { - displayType: 'line', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - - it('should create a dashboard with multiple tile types', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Multi-tile Dashboard', - tiles: [ - { - name: 'Line', - x: 0, - y: 0, - w: 12, - h: 4, - config: { - displayType: 'line', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - { - name: 'Table', - x: 0, - y: 4, - w: 12, - h: 4, - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - groupBy: 'SpanName', - groupByColumnsOnLeft: true, - }, - }, - { - name: 'Number', - x: 0, - y: 8, - w: 6, - h: 3, - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - { - name: 'Pie', - x: 6, - y: 8, - w: 6, - h: 3, - config: { - displayType: 'pie', - sourceId, - select: [{ aggFn: 'count' }], - groupBy: 'SpanName', - }, - }, - { - name: 'Notes', - x: 0, - y: 11, - w: 12, - h: 2, - config: { displayType: 'markdown', markdown: '# Dashboard Notes' }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles).toHaveLength(5); - - const tableTile = output.tiles.find( - (t: { name: string }) => t.name === 'Table', - ); - expect(tableTile).toBeDefined(); - expect(tableTile.config.groupByColumnsOnLeft).toBe(true); - }); - - it('should create a dashboard with a raw SQL tile', async () => { - const connectionId = connection._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'SQL Dashboard', - tiles: [ - { - name: 'Raw SQL', - config: { - configType: 'sql', - displayType: 'table', - connectionId, - sqlTemplate: 'SELECT 1 AS value LIMIT 1', - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles).toHaveLength(1); - }); - - it('should create a dashboard with a heatmap tile on a Trace source', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Heatmap Dashboard', - tiles: [ - { - name: 'Latency Heatmap', - x: 0, - y: 0, - w: 12, - h: 4, - config: { - displayType: 'heatmap', - sourceId, - select: [ - { - valueExpression: 'Duration', - }, - ], - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles).toHaveLength(1); - expect(output.tiles[0].config.displayType).toBe('heatmap'); - expect(output.tiles[0].config.select[0]).toMatchObject({ - valueExpression: 'Duration', - }); - }); - - it('should round-trip every MCP-specific heatmap field through save, get, update, and re-get', async () => { - const sourceId = traceSource._id.toString(); - - const createConfig = { - displayType: 'heatmap' as const, - sourceId, - select: [ - { - valueExpression: 'Duration', - countExpression: 'count()', - heatmapScaleType: 'log' as const, - }, - ], - where: 'level:error', - whereLanguage: 'lucene' as const, - numberFormat: { output: 'duration' as const, factor: 1e-9 }, - }; - // Mutate select + where on update; numberFormat, whereLanguage, - // and sourceId carry forward via the spread. Re-asserting against - // updatedConfig catches a regression where PUT silently drops any - // carried-forward field. - const updatedConfig = { - ...createConfig, - select: [ - { - valueExpression: "SpanAttributes['http.duration']", - heatmapScaleType: 'linear' as const, - }, - ], - where: 'level:error AND service:checkout', - }; - - const saveResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Heatmap Full Round-Trip', - tiles: [ - { - name: 'Latency Heatmap', - x: 0, - y: 0, - w: 12, - h: 4, - config: createConfig, - }, - ], - }); - expect(saveResult.isError).toBeFalsy(); - const saved = JSON.parse(getFirstText(saveResult)); - expect(saved.tiles).toHaveLength(1); - expect(saved.tiles[0].config).toMatchObject(createConfig); - - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: saved.id, - }); - expect(getResult.isError).toBeFalsy(); - const fetched = JSON.parse(getFirstText(getResult)); - expect(fetched.tiles[0].config).toMatchObject(createConfig); - - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: saved.id, - name: 'Heatmap Full Round-Trip', - tiles: [ - { - ...fetched.tiles[0], - config: { ...fetched.tiles[0].config, ...updatedConfig }, - }, - ], - }); - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.tiles[0].config).toMatchObject(updatedConfig); - - const getAfterUpdate = await callTool(client, 'hyperdx_get_dashboard', { - id: saved.id, - }); - expect(getAfterUpdate.isError).toBeFalsy(); - const refetched = JSON.parse(getFirstText(getAfterUpdate)); - expect(refetched.tiles[0].config).toMatchObject(updatedConfig); - }); - - it('should reject heatmap tile with empty valueExpression at the schema layer', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Bad Heatmap', - tiles: [ - { - name: 'Heatmap', - config: { - displayType: 'heatmap', - sourceId, - select: [{ valueExpression: '' }], - }, - }, - ], - }); - - expect(result.isError).toBe(true); - }); - - it.each([ - { output: 'duration', factor: 1e-9 }, - { output: 'data_rate' }, - { output: 'throughput' }, - ] as const)( - 'should round-trip numberFormat output "$output" through save and get', - async numberFormat => { - const sourceId = traceSource._id.toString(); - - const saveResult = await callTool(client, 'hyperdx_save_dashboard', { - name: `NumberFormat ${numberFormat.output}`, - tiles: [ - { - name: 'Number Tile', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - numberFormat, - }, - }, - { - name: 'Line Tile', - config: { - displayType: 'line', - sourceId, - select: [ - { aggFn: 'count' }, - { - aggFn: 'avg', - valueExpression: 'Duration', - numberFormat, - }, - ], - }, - }, - ], - }); - - expect(saveResult.isError).toBeFalsy(); - const saved = JSON.parse(getFirstText(saveResult)); - - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: saved.id, - }); - expect(getResult.isError).toBeFalsy(); - const fetched = JSON.parse(getFirstText(getResult)); - - const numberTile = fetched.tiles.find( - (t: { name: string }) => t.name === 'Number Tile', - ); - expect(numberTile.config.numberFormat).toEqual(numberFormat); - - const lineTile = fetched.tiles.find( - (t: { name: string }) => t.name === 'Line Tile', - ); - expect(lineTile.config.select[1].numberFormat).toEqual(numberFormat); - }, - ); - - it('should reject numberFormat with an unknown output value', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Bad NumberFormat', - tiles: [ - { - name: 'Number Tile', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - numberFormat: { output: 'not_a_real_output' }, - }, - }, - ], - }); - - expect(result.isError).toBe(true); - }); - - it('should reject heatmap tile on a non-Trace source', async () => { - // Create a Log source so the heatmap source-kind gate has - // something to reject. The schema accepts the tile shape (the - // sourceId is valid), so this exercises the runtime check that - // the REST POST path also runs. - const logSource = await Source.create({ - kind: SourceKind.Log, - team: team._id, - from: { - databaseName: DEFAULT_DATABASE, - tableName: 'otel_logs', - }, - timestampValueExpression: 'Timestamp', - connection: connection._id, - name: 'Logs', - }); - - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Heatmap on Log Source', - tiles: [ - { - name: 'Heatmap', - config: { - displayType: 'heatmap', - sourceId: logSource._id.toString(), - select: [{ valueExpression: 'Duration' }], - }, - }, - ], - }); - - expect(result.isError).toBe(true); - const text = getFirstText(result); - expect(text).toContain('Trace source'); - expect(text).toContain(logSource._id.toString()); - }); - - // Exercises the update-side source-kind gate via filterChangedHeatmapTiles - // (displayType changed to heatmap on an existing tile). - it('should reject update that changes a tile to heatmap on a non-Trace source', async () => { - const logSource = await Source.create({ - kind: SourceKind.Log, - team: team._id, - from: { databaseName: DEFAULT_DATABASE, tableName: 'otel_logs' }, - timestampValueExpression: 'Timestamp', - connection: connection._id, - name: 'Logs', - }); - - const created = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Line on Log Source', - tiles: [ - { - name: 'Line', - config: { - displayType: 'line', - sourceId: logSource._id.toString(), - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - expect(created.isError).toBeFalsy(); - const saved = JSON.parse(getFirstText(created)); - - const update = await callTool(client, 'hyperdx_save_dashboard', { - id: saved.id, - name: 'Line on Log Source', - tiles: [ - { - ...saved.tiles[0], - config: { - ...saved.tiles[0].config, - displayType: 'heatmap', - select: [{ valueExpression: 'Duration' }], - }, - }, - ], - }); - expect(update.isError).toBe(true); - const text = getFirstText(update); - expect(text).toContain('Trace source'); - expect(text).toContain(logSource._id.toString()); - }); - - // Exercises the update-side source-kind gate via filterChangedHeatmapTiles - // (sourceId changed on an existing heatmap tile). - it('should reject update that changes a heatmap tile source to a non-Trace source', async () => { - const logSource = await Source.create({ - kind: SourceKind.Log, - team: team._id, - from: { databaseName: DEFAULT_DATABASE, tableName: 'otel_logs' }, - timestampValueExpression: 'Timestamp', - connection: connection._id, - name: 'Logs', - }); - - const created = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Heatmap re-pointed at Log', - tiles: [ - { - name: 'Heatmap', - config: { - displayType: 'heatmap', - sourceId: traceSource._id.toString(), - select: [{ valueExpression: 'Duration' }], - }, - }, - ], - }); - expect(created.isError).toBeFalsy(); - const saved = JSON.parse(getFirstText(created)); - - const update = await callTool(client, 'hyperdx_save_dashboard', { - id: saved.id, - name: 'Heatmap re-pointed at Log', - tiles: [ - { - ...saved.tiles[0], - config: { - ...saved.tiles[0].config, - sourceId: logSource._id.toString(), - }, - }, - ], - }); - expect(update.isError).toBe(true); - const text = getFirstText(update); - expect(text).toContain('Trace source'); - expect(text).toContain(logSource._id.toString()); - }); - - // Asserts each tile's config survives the serializer/deserializer cycle - // independently when mixed with other displayTypes on the same dashboard. - it('should round-trip a heatmap alongside line and number tiles in one dashboard', async () => { - const sourceId = traceSource._id.toString(); - - const heatmapConfig = { - displayType: 'heatmap' as const, - sourceId, - select: [ - { valueExpression: 'Duration', heatmapScaleType: 'log' as const }, - ], - }; - const lineConfig = { - displayType: 'line' as const, - sourceId, - select: [{ aggFn: 'count' as const }], - groupBy: "SpanAttributes['service.name']", - }; - const numberConfig = { - displayType: 'number' as const, - sourceId, - select: [{ aggFn: 'count' as const }], - numberFormat: { output: 'number' as const, average: true }, - }; - - const save = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Mixed Tile Round-Trip', - tiles: [ - { name: 'Heatmap Tile', config: heatmapConfig }, - { name: 'Line Tile', config: lineConfig }, - { name: 'Number Tile', config: numberConfig }, - ], - }); - expect(save.isError).toBeFalsy(); - const saved = JSON.parse(getFirstText(save)); - expect(saved.tiles).toHaveLength(3); - - const byName: Record = - Object.fromEntries( - saved.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), - ); - expect(byName['Heatmap Tile'].config).toMatchObject(heatmapConfig); - expect(byName['Line Tile'].config).toMatchObject(lineConfig); - expect(byName['Number Tile'].config).toMatchObject(numberConfig); - - const fetched = JSON.parse( - getFirstText( - await callTool(client, 'hyperdx_get_dashboard', { id: saved.id }), - ), - ); - const fetchedByName: Record = - Object.fromEntries( - fetched.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), - ); - expect(fetchedByName['Heatmap Tile'].config).toMatchObject(heatmapConfig); - expect(fetchedByName['Line Tile'].config).toMatchObject(lineConfig); - expect(fetchedByName['Number Tile'].config).toMatchObject(numberConfig); - }); - }); - - describe('hyperdx_save_dashboard - containers and tabs', () => { - // Mirrors the v2 external-API "Containers and tabs" describe block so - // the MCP path enforces the same 5 cross-field rules: container id - // unique, tab id unique within a container, tile.containerId - // resolves, tile.tabId resolves to a tab on that container, and - // tile.tabId requires tile.containerId. - const buildTile = ( - sourceId: string, - overrides: Record, - ) => ({ - name: 'Tile', - x: 0, - y: 0, - w: 6, - h: 3, - config: { - displayType: 'line' as const, - sourceId, - select: [{ aggFn: 'count' as const, where: '' }], - }, - ...overrides, - }); - - it('should round-trip containers, tabs, and tile containerId/tabId on create and update', async () => { - const sourceId = traceSource._id.toString(); - const containers = [ - { - id: 'service-health', - title: 'Service Health', - collapsed: false, - collapsible: true, - bordered: true, - tabs: [ - { id: 'errors', title: 'Errors' }, - { id: 'latency', title: 'Latency' }, - ], - }, - { - id: 'overview', - title: 'Overview', - collapsed: true, - }, - ]; - - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Containers MCP Round-Trip', - tiles: [ - buildTile(sourceId, { - name: 'In Group, Tab A', - containerId: 'service-health', - tabId: 'errors', - }), - buildTile(sourceId, { - name: 'In Group, Tab B', - containerId: 'service-health', - tabId: 'latency', - }), - // Tile inside a tabbed container without a tabId renders in - // the container shell rather than under a tab. Guards that the - // schema does not accidentally require tabId for every tile in - // a tabbed container. - buildTile(sourceId, { - name: 'In Tabbed Group, No Tab', - containerId: 'service-health', - }), - buildTile(sourceId, { - name: 'In Plain Group', - containerId: 'overview', - }), - buildTile(sourceId, { name: 'Ungrouped' }), - ], - tags: ['containers-mcp'], - containers, - }); - - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - expect(created.containers).toEqual(containers); - - // Typed as ExternalDashboardTileWithId so a typo on .containerId - // would fail typecheck instead of silently asserting undefined. - const createdTilesByName: Record = - Object.fromEntries( - created.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), - ); - expect(createdTilesByName['In Group, Tab A']).toMatchObject({ - containerId: 'service-health', - tabId: 'errors', - }); - expect(createdTilesByName['In Group, Tab B']).toMatchObject({ - containerId: 'service-health', - tabId: 'latency', - }); - expect(createdTilesByName['In Tabbed Group, No Tab']).toMatchObject({ - containerId: 'service-health', - }); - expect( - createdTilesByName['In Tabbed Group, No Tab'].tabId, - ).toBeUndefined(); - expect(createdTilesByName['In Plain Group']).toMatchObject({ - containerId: 'overview', - }); - expect(createdTilesByName['In Plain Group'].tabId).toBeUndefined(); - expect(createdTilesByName.Ungrouped.containerId).toBeUndefined(); - expect(createdTilesByName.Ungrouped.tabId).toBeUndefined(); - - // Verify a fresh GET via the get tool also returns the structure. - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: created.id, - }); - const fetched = JSON.parse(getFirstText(getResult)); - expect(fetched.containers).toEqual(containers); - - // Update: rename a tab, drop the second container, re-home tiles. - const updatedContainers = [ - { - id: 'service-health', - title: 'Service Health', - collapsed: true, - tabs: [ - { id: 'errors', title: 'Error Rate' }, - { id: 'latency', title: 'Latency' }, - ], - }, - ]; - const reHomedUngrouped = { - ...createdTilesByName.Ungrouped, - containerId: 'service-health', - tabId: 'errors', - }; - const droppedContainerTile = { - ...createdTilesByName['In Plain Group'], - containerId: undefined, - tabId: undefined, - }; - - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'Containers MCP Round-Trip', - tiles: [ - createdTilesByName['In Group, Tab A'], - createdTilesByName['In Group, Tab B'], - createdTilesByName['In Tabbed Group, No Tab'], - droppedContainerTile, - reHomedUngrouped, - ], - tags: ['containers-mcp'], - containers: updatedContainers, - }); - - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.containers).toEqual(updatedContainers); - const updatedTilesByName: Record = - Object.fromEntries( - updated.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), - ); - expect(updatedTilesByName['In Plain Group'].containerId).toBeUndefined(); - expect(updatedTilesByName.Ungrouped).toMatchObject({ - containerId: 'service-health', - tabId: 'errors', - }); - expect(updatedTilesByName['In Tabbed Group, No Tab'].containerId).toBe( - 'service-health', - ); - expect( - updatedTilesByName['In Tabbed Group, No Tab'].tabId, - ).toBeUndefined(); - }); - - it('should reject duplicate container ids', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Duplicate Container Ids', - tiles: [buildTile(sourceId, {})], - containers: [ - { id: 'dupe', title: 'A', collapsed: false }, - { id: 'dupe', title: 'B', collapsed: false }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('Container IDs must be unique'); - }); - - it('should reject duplicate tab ids within a container', async () => { - // Structural rules (duplicate container ids, duplicate tab ids) fire - // through the body schema, which is reported as a stringified issue - // array. Cross-tile-ref rules (covered below) fire through - // collectTileContainerRefIssues and produce plain prose. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Duplicate Tab Ids', - tiles: [buildTile(sourceId, {})], - containers: [ - { - id: 'service-health', - title: 'Service Health', - collapsed: false, - tabs: [ - { id: 'errors', title: 'Errors' }, - { id: 'errors', title: 'Errors Two' }, - ], - }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('Duplicate tab id'); - expect(getFirstText(result)).toContain('errors'); - expect(getFirstText(result)).toContain('service-health'); - }); - - it('should reject a tile that supplies tabId without containerId', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Tab Without Container', - tiles: [buildTile(sourceId, { tabId: 'errors' })], - containers: [ - { - id: 'service-health', - title: 'Service Health', - collapsed: false, - tabs: [{ id: 'errors', title: 'Errors' }], - }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain( - 'tabId requires containerId to be set', - ); - }); - - it('should reject a tile that references an unknown containerId', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Unknown Container', - tiles: [buildTile(sourceId, { containerId: 'does-not-exist' })], - containers: [{ id: 'real', title: 'Real', collapsed: false }], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain( - 'unknown containerId "does-not-exist"', - ); - }); - - it('should reject a tile that references an unknown tabId within a container', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Unknown Tab', - tiles: [ - buildTile(sourceId, { - containerId: 'service-health', - tabId: 'ghost', - }), - ], - containers: [ - { - id: 'service-health', - title: 'Service Health', - collapsed: false, - tabs: [{ id: 'errors', title: 'Errors' }], - }, - ], - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('unknown tabId "ghost"'); - }); - - it('should preserve existing containers on update when body omits the field', async () => { - // Exercises the `effectiveContainers = parsedContainers ?? existingDashboard.containers ?? []` - // fallback in the MCP update handler. Without it, a PUT that - // updates only `tiles` would reject because the tile's - // containerId reference would resolve against an empty array. - const sourceId = traceSource._id.toString(); - const containers = [ - { - id: 'service-health', - title: 'Service Health', - collapsed: false, - tabs: [{ id: 'errors', title: 'Errors' }], - }, - ]; - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'PUT-without-containers fallback', - tiles: [ - buildTile(sourceId, { - name: 'In Group', - containerId: 'service-health', - tabId: 'errors', - }), - ], - containers, - }); - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'PUT-without-containers fallback', - tiles: created.tiles, - // containers intentionally omitted - }); - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.containers).toEqual(containers); - expect(updated.tiles[0]).toMatchObject({ - containerId: 'service-health', - tabId: 'errors', - }); - }); - - it('should wipe persisted containers when update body sets containers: []', async () => { - // Mirror the v2 behavior: an explicit empty array clears the - // persisted containers, and the response normalizes [] back to - // absent on read. - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Wipe containers', - tiles: [buildTile(sourceId, { name: 'Tile' })], - containers: [{ id: 'overview', title: 'Overview', collapsed: false }], - }); - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - expect(created.containers).toHaveLength(1); - - const wipedTile = { - ...created.tiles[0], - containerId: undefined, - tabId: undefined, - }; - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'Wipe containers', - tiles: [wipedTile], - containers: [], - }); - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.containers).toBeUndefined(); - - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: created.id, - }); - const fetched = JSON.parse(getFirstText(getResult)); - expect(fetched.containers).toBeUndefined(); - }); - - // Bounds mirror DASHBOARD_CONTAINER_ID_MAX (256) on the tile-level - // containerId / tabId. 256 chars must accept; 257 must reject. - // 257 trips the inputSchema's `.max(256)` and surfaces back as the - // MCP SDK's "Input validation error" envelope. - it('should accept a 256-char tile.containerId and reject 257', async () => { - const sourceId = traceSource._id.toString(); - const idAtMax = 'c'.repeat(256); - const okResult = await callTool(client, 'hyperdx_save_dashboard', { - name: '256-char containerId', - tiles: [buildTile(sourceId, { containerId: idAtMax })], - containers: [{ id: idAtMax, title: 'Max', collapsed: false }], - }); - expect(okResult.isError).toBeFalsy(); - - const idTooLong = 'c'.repeat(257); - const tooLongResult = await callTool(client, 'hyperdx_save_dashboard', { - name: '257-char containerId', - tiles: [buildTile(sourceId, { containerId: idTooLong })], - containers: [{ id: idTooLong, title: 'Too long', collapsed: false }], - }); - expect(tooLongResult.isError).toBe(true); - expect(getFirstText(tooLongResult)).toContain( - 'String must contain at most 256 character(s)', - ); - expect(getFirstText(tooLongResult)).toContain('containerId'); - }); - }); - - describe('hyperdx_save_dashboard - dashboard filters', () => { - // The MCP input schema delegates to createDashboardBodySchema / - // updateDashboardBodySchema for the filter shape. These tests guard - // that the MCP path lights up the same filter round-trip the v2 REST - // path already covers: filters are saved verbatim, get back with an - // assigned id, survive an update, and reject obvious bad inputs. - - const traceTile = (sourceId: string) => ({ - name: 'Volume', - x: 0, - y: 0, - w: 6, - h: 3, - config: { - displayType: 'line' as const, - sourceId, - select: [{ aggFn: 'count' as const, where: '' }], - }, - }); - - it('should round-trip filters on create, get, and update', async () => { - const sourceId = traceSource._id.toString(); - - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Service Detail (MCP filter round-trip)', - tiles: [traceTile(sourceId)], - filters: [ - { - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }, - { - type: 'QUERY_EXPRESSION', - name: 'Environment', - expression: 'deployment.environment', - sourceId, - where: "deployment.environment = 'production'", - whereLanguage: 'sql', - }, - ], - }); - - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - expect(Array.isArray(created.filters)).toBe(true); - expect(created.filters).toHaveLength(2); - // The body schema assigns an id to each filter on create. Capture - // them so the update payload can include the same ids and the - // filter array round-trips identically (instead of being treated - // as wholesale replacement with new ids). - const [serviceFilter, envFilter] = created.filters; - expect(serviceFilter).toMatchObject({ - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }); - expect(typeof serviceFilter.id).toBe('string'); - expect(serviceFilter.id.length).toBeGreaterThan(0); - expect(envFilter).toMatchObject({ - type: 'QUERY_EXPRESSION', - name: 'Environment', - expression: 'deployment.environment', - sourceId, - where: "deployment.environment = 'production'", - whereLanguage: 'sql', - }); - - // Fetch and assert the same shape. - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: created.id, - }); - const fetched = JSON.parse(getFirstText(getResult)); - expect(fetched.filters).toEqual(created.filters); - - // Update: rename the first filter and drop the second. The first - // filter keeps its id, the second is dropped (not preserved). - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'Service Detail (MCP filter round-trip)', - tiles: [traceTile(sourceId)], - filters: [ - { - id: serviceFilter.id, - type: 'QUERY_EXPRESSION', - name: 'Service (renamed)', - expression: 'ServiceName', - sourceId, - }, - ], - }); - - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.filters).toHaveLength(1); - expect(updated.filters[0]).toMatchObject({ - id: serviceFilter.id, - type: 'QUERY_EXPRESSION', - name: 'Service (renamed)', - expression: 'ServiceName', - sourceId, - }); - }); - - it('should accept a create payload with stray filter ids (copy-paste round-trip)', async () => { - // An LLM that copies a filter out of hyperdx_get_dashboard and - // back into hyperdx_save_dashboard for a NEW dashboard ships an - // `id` on the filter. The body schema for create is - // `.strict()` and rejects `id`, so saveDashboard normalizes by - // stripping ids before validation. Without that normalization - // this payload would 4xx with a confusing strict-validation - // error. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Create with stray filter id', - tiles: [traceTile(sourceId)], - filters: [ - { - id: '999999999999999999999999', // simulates a copied id - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }, - ], - }); - expect(result.isError).toBeFalsy(); - const dashboard = JSON.parse(getFirstText(result)); - expect(dashboard.filters).toHaveLength(1); - // The new dashboard gets a fresh id; the copied one is discarded. - expect(dashboard.filters[0].id).not.toBe('999999999999999999999999'); - expect(dashboard.filters[0]).toMatchObject({ - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }); - }); - - it('should assign ids to new filters added during update (no-id round-trip)', async () => { - // An LLM updating a dashboard to ADD a brand-new filter ships - // a filter without an `id`. The body schema for update requires - // id, so saveDashboard fills one in. Without that normalization - // this payload would 4xx complaining that id is missing. - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Update adds new filter', - tiles: [traceTile(sourceId)], - filters: [ - { - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }, - ], - }); - const created = JSON.parse(getFirstText(createResult)); - const existingFilterId = created.filters[0].id; - - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'Update adds new filter', - tiles: [traceTile(sourceId)], - filters: [ - // Existing filter, preserved by id. - { - id: existingFilterId, - type: 'QUERY_EXPRESSION', - name: 'Service', - expression: 'ServiceName', - sourceId, - }, - // Brand-new filter, no id. saveDashboard assigns one. - { - type: 'QUERY_EXPRESSION', - name: 'Environment', - expression: 'deployment.environment', - sourceId, - }, - ], - }); - expect(updateResult.isError).toBeFalsy(); - const updated = JSON.parse(getFirstText(updateResult)); - expect(updated.filters).toHaveLength(2); - const envFilter = updated.filters.find( - (f: { name: string }) => f.name === 'Environment', - ); - expect(envFilter).toBeDefined(); - expect(typeof envFilter.id).toBe('string'); - expect(envFilter.id.length).toBeGreaterThan(0); - expect(envFilter.id).not.toBe(existingFilterId); - }); - - it('should round-trip a table tile that uses a having clause', async () => { - // mcpTableTileSchema exposes `having` so the service_detail - // example's "Top Error Messages" pattern (groupBy StatusMessage - // with having: "StatusMessage != ''") survives MCP authoring. - // Without `having` on the schema, Zod would strip it and the - // saved tile would include empty-message rows instead of the - // filtered set the prompt promises. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Top Error Messages', - tiles: [ - { - name: 'Top Error Messages', - x: 0, - y: 0, - w: 12, - h: 6, - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count', alias: 'Count' }], - groupBy: 'StatusMessage', - having: "StatusMessage != ''", - orderBy: 'Count DESC', - groupByColumnsOnLeft: true, - }, - }, - ], - }); - expect(result.isError).toBeFalsy(); - const dashboard = JSON.parse(getFirstText(result)); - expect(dashboard.tiles[0].config.having).toBe("StatusMessage != ''"); - }); - }); - - describe('hyperdx_save_dashboard - table onClick linking', () => { - // Mirrors the v2 external-API onClick tests so the MCP path - // enforces the same drill-down rules: row-click can target /search - // for a log/trace source or another dashboard, by concrete ID or - // by templated name, with optional whereTemplate/filters. - it('should round-trip a table tile with a search onClick by ID', async () => { - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick search by id', - tiles: [ - { - name: 'Errors by Service', - x: 0, - y: 0, - w: 12, - h: 6, - config: { - displayType: 'table', - sourceId, - groupBy: "ResourceAttributes['service.name']", - select: [{ aggFn: 'count' }], - onClick: { - type: 'search', - target: { mode: 'id', id: sourceId }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: "{{ResourceAttributes['service.name']}}", - }, - ], - }, - }, - }, - ], - }); - - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - expect(created.tiles[0].config.onClick).toEqual({ - type: 'search', - target: { mode: 'id', id: sourceId }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: "{{ResourceAttributes['service.name']}}", - }, - ], - }); - - const getResult = await callTool(client, 'hyperdx_get_dashboard', { - id: created.id, - }); - const fetched = JSON.parse(getFirstText(getResult)); - expect(fetched.tiles[0].config.onClick).toEqual( - created.tiles[0].config.onClick, - ); - }); - - it('should round-trip a table tile with a dashboard onClick by ID', async () => { - const sourceId = traceSource._id.toString(); - // Create the target dashboard the onClick will link to so the - // server-side `getMissingOnClickDashboards` check resolves. - const targetDashboard = await new Dashboard({ - name: 'Service Detail', - tiles: [], - team: team._id, - }).save(); - - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick dashboard by id', - tiles: [ - { - name: 'Top Services', - x: 0, - y: 0, - w: 12, - h: 6, - config: { - displayType: 'table', - sourceId, - groupBy: "ResourceAttributes['service.name']", - select: [{ aggFn: 'count' }], - onClick: { - type: 'dashboard', - target: { - mode: 'id', - id: targetDashboard._id.toString(), - }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: "{{ResourceAttributes['service.name']}}", - }, - ], - }, - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles[0].config.onClick).toEqual({ - type: 'dashboard', - target: { mode: 'id', id: targetDashboard._id.toString() }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: "{{ResourceAttributes['service.name']}}", - }, - ], - }); - }); - - it('should round-trip a templated dashboard onClick (mode=template)', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick dashboard by template', - tiles: [ - { - name: 'Service Picker', - x: 0, - y: 0, - w: 12, - h: 6, - config: { - displayType: 'table', - sourceId, - groupBy: 'SpanName', - select: [{ aggFn: 'count' }], - onClick: { - type: 'dashboard', - target: { - mode: 'template', - template: '{{TargetDashboard}}', - }, - whereLanguage: 'lucene', - whereTemplate: - "service.name:{{ResourceAttributes['service.name']}}", - }, - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles[0].config.onClick).toEqual({ - type: 'dashboard', - target: { mode: 'template', template: '{{TargetDashboard}}' }, - whereLanguage: 'lucene', - whereTemplate: "service.name:{{ResourceAttributes['service.name']}}", - }); - }); - - it('should round-trip onClick on a raw SQL table tile', async () => { - const connectionId = connection._id.toString(); - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'SQL onClick', - tiles: [ - { - name: 'Raw SQL Table', - x: 0, - y: 0, - w: 12, - h: 6, - config: { - configType: 'sql', - displayType: 'table', - connectionId, - sqlTemplate: - 'SELECT ServiceName, count() AS c FROM otel_traces GROUP BY ServiceName LIMIT 50', - onClick: { - type: 'search', - target: { mode: 'id', id: sourceId }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: '{{ServiceName}}', - }, - ], - }, - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles[0].config.onClick).toEqual({ - type: 'search', - target: { mode: 'id', id: sourceId }, - whereLanguage: 'sql', - filters: [ - { - kind: 'expressionTemplate', - expression: 'ServiceName', - template: '{{ServiceName}}', - }, - ], - }); - }); - - it('should reject a table tile onClick referencing a non-existent dashboard', async () => { - const sourceId = traceSource._id.toString(); - const ghostDashboardId = new mongoose.Types.ObjectId().toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick missing dashboard', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - onClick: { - type: 'dashboard', - target: { mode: 'id', id: ghostDashboardId }, - whereLanguage: 'sql', - }, - }, - }, - ], - }); - - expect(result.isError).toBe(true); - const text = getFirstText(result); - expect(text).toContain('onClick dashboard'); - expect(text).toContain(ghostDashboardId); - }); - - it('should reject a table tile onClick search target with a non-log/trace source', async () => { - // /search only renders log/trace sources; targeting a metric - // source must be rejected at save time, matching the REST POST - // path. Create the metric source inline so this test is - // self-contained. - const metricSource = await Source.create({ - kind: SourceKind.Metric, - team: team._id, - from: { - databaseName: DEFAULT_DATABASE, - tableName: '', - }, - metricTables: { - [MetricsDataType.Gauge.toLowerCase()]: 'otel_metrics_gauge', - [MetricsDataType.Sum.toLowerCase()]: 'otel_metrics_sum', - [MetricsDataType.Histogram.toLowerCase()]: 'otel_metrics_histogram', - }, - timestampValueExpression: 'TimeUnix', - connection: connection._id, - name: 'Metrics', - }); - const sourceId = traceSource._id.toString(); - - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick metric source', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - onClick: { - type: 'search', - target: { - mode: 'id', - id: metricSource._id.toString(), - }, - whereLanguage: 'sql', - }, - }, - }, - ], - }); - - expect(result.isError).toBe(true); - const text = getFirstText(result); - expect(text).toContain('log or trace'); - expect(text).toContain(metricSource._id.toString()); - }); - - it('should reject a table tile onClick with an invalid target.id', async () => { - // target.id must be a valid ObjectId. Bad shape is caught by the - // input schema and surfaces as the SDK's "Input validation error" - // envelope rather than a custom 400 message. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick bad object id', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - onClick: { - type: 'dashboard', - target: { mode: 'id', id: 'not-a-valid-object-id' }, - whereLanguage: 'sql', - }, - }, - }, - ], - }); - - expect(result.isError).toBe(true); - }); - - it('should accept onClick without whereLanguage (it is optional at the schema layer)', async () => { - // SearchConditionLanguageSchema is `.optional()` in common-utils, - // so omitting whereLanguage is allowed. Pin that behavior so a - // future tightening of the schema doesn't quietly change it - // without updating the docs that callers rely on. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick no whereLanguage', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - onClick: { - type: 'search', - target: { mode: 'id', id: sourceId }, - // whereLanguage intentionally omitted - }, - }, - }, - ], - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.tiles[0].config.onClick).toMatchObject({ - type: 'search', - target: { mode: 'id', id: sourceId }, - }); - }); - - it('should reject an unknown onClick.type', async () => { - // The schema is a discriminated union on `type`; a bogus type - // must be rejected up front. - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick bad type', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - onClick: { - type: 'navigate-to-mars', - target: { mode: 'id', id: sourceId }, - whereLanguage: 'sql', - }, - }, - }, - ], - }); - - expect(result.isError).toBe(true); - }); - - it('should validate onClick on update too', async () => { - // Round-trip a tile with no onClick, then update it to add an - // onClick pointing at a non-existent dashboard. The PUT path - // mirrors POST: missing dashboards are rejected with the same - // message. - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'OnClick update validation', - tiles: [ - { - name: 'Top Services', - config: { - displayType: 'table', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - expect(createResult.isError).toBeFalsy(); - const created = JSON.parse(getFirstText(createResult)); - - const ghostDashboardId = new mongoose.Types.ObjectId().toString(); - const updateResult = await callTool(client, 'hyperdx_save_dashboard', { - id: created.id, - name: 'OnClick update validation', - tiles: [ - { - ...created.tiles[0], - config: { - ...created.tiles[0].config, - onClick: { - type: 'dashboard', - target: { mode: 'id', id: ghostDashboardId }, - whereLanguage: 'sql', - }, - }, - }, - ], - }); - - expect(updateResult.isError).toBe(true); - const text = getFirstText(updateResult); - expect(text).toContain('onClick dashboard'); - expect(text).toContain(ghostDashboardId); - }); - }); - - describe('hyperdx_delete_dashboard', () => { - it('should delete an existing dashboard', async () => { - const dashboard = await new Dashboard({ - name: 'To Delete', - tiles: [], - team: team._id, - }).save(); - - const result = await callTool(client, 'hyperdx_delete_dashboard', { - id: dashboard._id.toString(), - }); - - expect(result.isError).toBeFalsy(); - const output = JSON.parse(getFirstText(result)); - expect(output.deleted).toBe(true); - expect(output.id).toBe(dashboard._id.toString()); - - // Verify deleted from database - const found = await Dashboard.findById(dashboard._id); - expect(found).toBeNull(); - }); - - it('should return error for non-existent dashboard', async () => { - const result = await callTool(client, 'hyperdx_delete_dashboard', { - id: '000000000000000000000000', - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - }); - - describe('hyperdx_query_tile', () => { - it('should return error for non-existent dashboard', async () => { - const result = await callTool(client, 'hyperdx_query_tile', { - dashboardId: '000000000000000000000000', - tileId: 'some-tile-id', - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - - it('should return error for non-existent tile', async () => { - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Tile Query Test', - tiles: [ - { - name: 'My Tile', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - const dashboard = JSON.parse(getFirstText(createResult)); - - const result = await callTool(client, 'hyperdx_query_tile', { - dashboardId: dashboard.id, - tileId: 'non-existent-tile-id', - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('Tile not found'); - }); - - it('should return error for invalid time range', async () => { - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Time Range Test', - tiles: [ - { - name: 'Tile', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - const dashboard = JSON.parse(getFirstText(createResult)); - - const result = await callTool(client, 'hyperdx_query_tile', { - dashboardId: dashboard.id, - tileId: dashboard.tiles[0].id, - startTime: 'not-a-date', - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('Invalid'); - }); - - it('should execute query for a valid tile', async () => { - const sourceId = traceSource._id.toString(); - const createResult = await callTool(client, 'hyperdx_save_dashboard', { - name: 'Query Tile Test', - tiles: [ - { - name: 'Count Tile', - config: { - displayType: 'number', - sourceId, - select: [{ aggFn: 'count' }], - }, - }, - ], - }); - const dashboard = JSON.parse(getFirstText(createResult)); - - const result = await callTool(client, 'hyperdx_query_tile', { - dashboardId: dashboard.id, - tileId: dashboard.tiles[0].id, - startTime: new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(), - endTime: new Date().toISOString(), - }); - - // Should succeed (may have empty results since no data inserted) - expect(result.isError).toBeFalsy(); - expect(result.content).toHaveLength(1); - }); - }); - - describe('buildQueryGuidePrompt', () => { - it('documents heatmap in tile-type, constraints, mistakes, and aggFn sections', () => { - const prompt = buildQueryGuidePrompt(); - // Ensure each section the bot called out has a heatmap entry so - // the LLM cannot skip the constraints when producing a heatmap - // tile through hyperdx_save_dashboard. - const sections = [ - '== AGGREGATION FUNCTIONS (aggFn) ==', - '== PER-TILE TYPE CONSTRAINTS ==', - '== COMMON MISTAKES ==', - ]; - for (const heading of sections) { - const idx = prompt.indexOf(heading); - expect(idx).toBeGreaterThan(-1); - // Each section's body extends until the next == heading or the - // end of the string. Checking for the substring "heatmap" in - // that slice is enough to assert the heatmap entry exists. - const next = prompt.indexOf('\n== ', idx + heading.length); - const body = prompt.slice(idx, next === -1 ? prompt.length : next); - expect(body.toLowerCase()).toContain('heatmap'); - } - }); - - it('documents the dashboard filter and per-series numberFormat sections', () => { - const prompt = buildQueryGuidePrompt(); - // Sections must exist AND carry the substantive content. Heading- - // only assertions let a future contributor empty out a section - // and silently pass review. - const sliceSection = (heading: string) => { - const idx = prompt.indexOf(heading); - if (idx === -1) return ''; - const next = prompt.indexOf('\n== ', idx + heading.length); - return prompt.slice(idx, next === -1 ? prompt.length : next); - }; - const filters = sliceSection('== DASHBOARD FILTERS =='); - expect(filters).toContain('QUERY_EXPRESSION'); - expect(filters).toContain('expression'); - expect(filters).toContain('sourceId'); - const numberFormat = sliceSection('== NUMBER FORMAT =='); - expect(numberFormat).toContain('factor: 0.000000001'); - expect(numberFormat).toContain('duration'); - expect(numberFormat.toLowerCase()).toContain('per-series'); - }); - - it('declares the MCP metric-authoring gap rather than referencing fields that do not exist', () => { - // The MCP select-item schema does not carry metricName / metricType, - // so any prompt that tells the model to author a metric tile via - // the builder path is teaching it to ship JSON that gets silently - // stripped. Guard with an explicit assertion so a future diff that - // restores metric-tile guidance fails this test loudly. - const prompt = buildQueryGuidePrompt(); - expect(prompt).not.toMatch(/metricName \+ metricType/); - expect(prompt).not.toMatch(/exactly 1 select item with metricName/); - const constraintsIdx = prompt.indexOf('== PER-TILE TYPE CONSTRAINTS =='); - const constraintsBody = prompt.slice(constraintsIdx); - // The constraints section closes with an explicit note that builder - // tiles on a metric source are not reliable today, with a fallback - // recipe to raw SQL. Anchor on the phrase so a future diff that - // drops the gap-acknowledgement fails loudly. - expect(constraintsBody).toMatch( - /Authoring builder tiles on a metric source is not reliable/, - ); - expect(constraintsBody).toMatch(/MCP select-item shape does not carry/); - }); - - it('documents table-tile onClick linking features', () => { - // Lock down the documentation for row-click drill-downs so a - // future refactor can't quietly drop the section the LLM relies - // on to wire up onClick correctly. - const prompt = buildQueryGuidePrompt(); - const idx = prompt.indexOf('== TABLE TILE LINKING (config.onClick) =='); - expect(idx).toBeGreaterThan(-1); - const next = prompt.indexOf('\n== ', idx + 1); - const section = prompt.slice(idx, next === -1 ? prompt.length : next); - - // Both destination types are mentioned. - expect(section).toContain('type: "search"'); - expect(section).toContain('type: "dashboard"'); - // Both target modes are mentioned. - expect(section).toContain('mode: "id"'); - expect(section).toContain('mode: "template"'); - // Templating fields are mentioned. - expect(section).toContain('whereTemplate'); - expect(section).toContain('filters'); - expect(section).toContain('expressionTemplate'); - // The two server-side validation error messages are quoted so - // the LLM can recognize and recover from them. - expect(section).toContain('onClick search source IDs'); - expect(section).toContain('onClick dashboard IDs'); - }); - - it('documents onClick pitfalls under common mistakes', () => { - const prompt = buildQueryGuidePrompt(); - const idx = prompt.indexOf('== COMMON MISTAKES =='); - expect(idx).toBeGreaterThan(-1); - const section = prompt.slice(idx); - // Mention the four failure modes we hit during validation. - expect(section.toLowerCase()).toContain('onclick on a non-table tile'); - expect(section.toLowerCase()).toContain('non-log/trace source'); - expect(section.toLowerCase()).toContain('missing wherelanguage'); - expect(section.toLowerCase()).toContain("isn't in the table"); - }); - - it('documents the missing-alias mistake explicitly', () => { - // Claude built three number tiles without alias on the quantile() - // select item, even though rule 2 of the design checklist says - // ALIAS EVERY SELECT ITEM. Spelling out the mistake under COMMON - // MISTAKES gives the model a second touchpoint to catch it. - const prompt = buildQueryGuidePrompt(); - const idx = prompt.indexOf('== COMMON MISTAKES =='); - const section = prompt.slice(idx); - expect(section).toMatch(/Missing alias on a select item/); - // Must call out that number tiles need the alias too; this is the - // exact case Claude got wrong. - expect(section).toMatch(/number tiles too/); - }); - - it('documents the Lucene map-attribute gotcha across all operators including simple equality', () => { - // The original rule covered only comparison/wildcard. Claude's - // second pass showed simple equality (db.system:mongodb) also - // fails to translate to SpanAttributes['db.system']. Broaden the - // gotcha to "ALL Lucene operations on map-attribute paths". - const prompt = buildQueryGuidePrompt(); - const luceneIdx = prompt.indexOf('== LUCENE FILTER SYNTAX =='); - const luceneBody = prompt.slice(luceneIdx); - expect(luceneBody).toMatch(/NOT reliably translated to bracket access/); - expect(luceneBody).toMatch(/http\.status_code:>=500/); // operator - expect(luceneBody).toMatch(/http\.route:\*/); // wildcard - expect(luceneBody).toMatch(/db\.system:mongodb/); // simple equality - expect(luceneBody).toMatch(/use SQL with bracket access/); - - // Also a top-level entry in COMMON MISTAKES for discoverability. - const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); - const mistakesBody = prompt.slice(mistakesIdx); - expect(mistakesBody).toMatch( - /Lucene on a map-attribute path \(any operation\)/, - ); - }); - - it('documents the Lucene fuzzy-substring behavior on top-level columns', () => { - // Lucene field:value translates to ilike(field, '%value%'), not - // equality. Claude's second pass hit this on SpanKind:Server, - // which matched broader strings than intended. The note has to - // be in the Lucene section so a model reading top-down sees it - // before writing a SpanKind filter. - const prompt = buildQueryGuidePrompt(); - const luceneIdx = prompt.indexOf('== LUCENE FILTER SYNTAX =='); - const luceneBody = prompt.slice(luceneIdx); - expect(luceneBody).toMatch(/FUZZY-MATCH NOTE/); - expect(luceneBody).toMatch(/ilike\(field, '%value%'\)/); - // The canonical fix is SQL with =. - expect(luceneBody).toMatch(/SpanKind = 'Server'/); - - // And a COMMON MISTAKES entry covering enum-like columns. - const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); - const mistakesBody = prompt.slice(mistakesIdx); - expect(mistakesBody).toMatch( - /Lucene field:value on enum-like top-level columns/, - ); - }); - - it('documents the empty-string trap for map-attribute groupBy', () => { - // Map columns return '' (not NULL) when a key is unset on a row. - // A groupBy on such a key produces an empty-string bucket alongside - // the real values, which is visual noise on tables and pies. The - // standard fix is where: " != ''" alongside the groupBy. - const prompt = buildQueryGuidePrompt(); - const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); - const mistakesBody = prompt.slice(mistakesIdx); - expect(mistakesBody).toMatch( - /Forgetting that map-attribute values are often empty strings/, - ); - expect(mistakesBody).toMatch(/db\.collection\.name'\] != ''/); - }); - - it('documents the groupBy alias workaround for map-attribute drill-downs', () => { - // Builder tiles don't expose a groupBy alias today; map-attribute - // groupBys produce result columns named like the raw expression, - // which onClick template lookups can't reference. The workaround - // (author the tile as raw SQL with AS) needs to be findable. - const prompt = buildQueryGuidePrompt(); - const idx = prompt.indexOf( - '== GROUPBY ALIASES AND ROW-CLICK TEMPLATES ==', - ); - expect(idx).toBeGreaterThan(-1); - const section = prompt.slice(idx); - expect(section).toMatch(/do NOT currently expose a groupBy alias/); - expect(section).toMatch(/configType: "sql", displayType: "table"/); - expect(section).toMatch(/AS Route/); - }); - - it('points at ClickStack and ClickHouse docs in REFERENCES', () => { - // Without external doc links, the LLM has to guess at Lucene - // operators and ClickHouse function names. Anchor the canonical - // URLs in the prompt so it can cite them and so a future doc - // restructure can be caught here. - const prompt = buildQueryGuidePrompt(); - const idx = prompt.indexOf('== REFERENCES =='); - expect(idx).toBeGreaterThan(-1); - const section = prompt.slice(idx); - expect(section).toContain( - 'https://clickhouse.com/docs/use-cases/observability/clickstack/search', - ); - expect(section).toContain('https://clickhouse.com/docs/sql-reference'); - expect(section).toContain( - 'https://clickhouse.com/docs/use-cases/observability/clickstack/dashboards/sql-visualizations', - ); - }); - - it('strengthens the validate-every-tile rule under common mistakes', () => { - // Saving validates input shape, not query semantics. The prompt - // has to push for query_tile on every tile, not "at least one". - const prompt = buildQueryGuidePrompt(); - const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); - const section = prompt.slice(mistakesIdx); - expect(section).toMatch( - /on EVERY tile after hyperdx_save_dashboard, not just one/, - ); - }); - - it('flags the metric source builder gap', () => { - // Builder tiles on metric sources currently save but render with - // "Both table name and UUID are empty". Claude went 100% raw SQL - // across 21 metric tiles for this reason; the prompt has to make - // the workaround obvious so the model doesn't try the builder - // first and fail silently. - const prompt = buildQueryGuidePrompt(); - expect(prompt).toMatch(/Both table name and UUID are empty/); - expect(prompt).toMatch(/use a raw SQL tile/); - expect(prompt).toMatch(/otel_metrics_gauge/); - }); - - it('contains no em-dashes or en-dashes used as em-dashes', () => { - const prompt = buildQueryGuidePrompt(); - expect(prompt).not.toMatch(/—/); // [prose-lint: allow] - expect(prompt).not.toMatch(/ – /); // [prose-lint: allow] - }); - }); - - describe('buildSourceSummary', () => { - it('contains no em-dashes or en-dashes used as em-dashes', () => { - // buildSourceSummary is the source-list block that gets prepended - // to create_dashboard. The first iteration of this PR shipped with - // em-dashes here that the content.ts snapshot test missed because - // it calls the builders directly. Guard the helper output too. - const summary = buildSourceSummary( - [ - { - _id: '000000000000000000000001', - name: 'Traces', - kind: 'trace', - connection: '000000000000000000000002', - }, - { - _id: '000000000000000000000003', - name: 'Logs', - kind: 'log', - connection: '000000000000000000000002', - }, - ], - [{ _id: '000000000000000000000002', name: 'Default' }], - ); - expect(summary).not.toMatch(/—/); // [prose-lint: allow] - expect(summary).not.toMatch(/ – /); // [prose-lint: allow] - // Sanity: the helper still emits the source list, so the assertion - // above isn't trivially passing on an empty string. - expect(summary).toContain('Traces'); - expect(summary).toContain('AVAILABLE SOURCES'); - }); - }); - - describe('buildCreateDashboardPrompt', () => { - it('includes the design checklist and adapt-do-not-copy note', () => { - const prompt = buildCreateDashboardPrompt( - 'sources summary', - '000000000000000000000001', - '000000000000000000000002', - ); - expect(prompt).toContain('== DESIGN CHECKLIST =='); - // Each rule on the checklist exists at the same numbered position - // so a future contributor cannot silently drop one. Anchor on - // line start (with a trailing space) so digit-and-dot substrings - // like "0.000000001" elsewhere in the prompt cannot satisfy the - // assertion accidentally. - const checklistIdx = prompt.indexOf('== DESIGN CHECKLIST =='); - const adaptIdx = prompt.indexOf('== ADAPT, DO NOT COPY =='); - const checklistBody = prompt.slice(checklistIdx, adaptIdx); - // Thirteen rules: the original ten plus GROUP BY HAS NO ALIAS HOOK - // (rule 3), VALIDATE EVERY TILE AFTER SAVE (rule 12), and NO - // TITLE-RECAP MARKDOWN TILE (rule 13). Each came out of a live - // verification pass after watching Claude reliably ignore the soft - // "should" formulations or hit a schema gap the earlier checklist - // did not call out. - for (let i = 1; i <= 13; i++) { - expect(checklistBody).toMatch(new RegExp(`^${i}\\. `, 'm')); - } - expect(prompt).toContain('ADAPT, DO NOT COPY'); - // Rule 9 documents the replace-not-merge semantic on update; an - // agent that ignores this can silently drop tiles / filters / containers - // on a subsequent save call. - expect(checklistBody).toMatch(/UPDATE IS REPLACE, NOT MERGE/); - // Rule 2 must cover EVERY select item (including number tiles), not - // just aggregations on tables. The phrasing matters: Claude's first - // pass at this dropped aliases on number tiles because it read rule 2 - // as table-specific. The stronger phrasing pulls them in. - expect(checklistBody).toMatch(/ALIAS EVERY SELECT ITEM/); - // Rule 3 calls out the schema gap on groupBy: the chart config takes - // a single expression string with no alias field, so a table grouped - // by SpanAttributes['http.route'] renders arrayElement(...) as the - // column header. Naming the limit explicitly lets Claude pick a - // top-level column (SpanName, ServiceName) instead of grouping on - // the raw Map expression. - expect(checklistBody).toMatch(/GROUP BY HAS NO ALIAS HOOK/); - // Rule 10 must be a hard requirement at 5+ tiles, not a soft hint. - // Without the imperative, Claude built five dashboards averaging ten - // tiles each with zero containers. - expect(checklistBody).toMatch(/REQUIRED at five or more tiles/); - // Rule 11 + 12 close the post-save loop and the title-recap markdown - // habit Claude landed on every starter dashboard. - expect(checklistBody).toMatch(/VALIDATE EVERY TILE AFTER SAVE/); - expect(checklistBody).toMatch(/NO TITLE-RECAP MARKDOWN TILE/); - // Rule 2 now carries a concrete number-tile example. Claude's - // second pass still dropped aliases on every builder number tile, - // because the text rule was abstract; a copy-pasteable shape next - // to the rule is what makes it stick. - expect(checklistBody).toMatch( - /Number tile, correct:.*alias: "Server Requests"/, - ); - // Rule 10 now carries a concrete containers shape. Claude's - // second pass built five dashboards with 9-10 tiles each and zero - // containers despite the REQUIRED phrasing; a copy-pasteable shape - // inside the rule itself is the next-level push. - expect(checklistBody).toMatch(/Concrete shape \(copy this directly/); - expect(checklistBody).toMatch(/id: "kpis"/); - expect(checklistBody).toMatch(/id: "trends"/); - expect(checklistBody).toMatch(/id: "errors"/); - }); - - it('walks the workflow through six steps including read-existing and group-into-containers', () => { - // The workflow is the first thing a model anchors on. Claude - // skipped both "read existing dashboards before designing" and - // "group tiles into containers before saving" because neither was - // a numbered workflow step; both lived only in the checklist. The - // workflow now lists six steps with those two explicitly. - const prompt = buildCreateDashboardPrompt( - 'summary', - 'trace_src', - 'log_src', - ); - const wfIdx = prompt.indexOf('== WORKFLOW =='); - const wfEnd = prompt.indexOf('\n== ', wfIdx + 1); - const wf = prompt.slice(wfIdx, wfEnd); - for (let i = 1; i <= 6; i++) { - expect(wf).toMatch(new RegExp(`^${i}\\. `, 'm')); - } - // Step 2 must point at hyperdx_get_dashboard with no id. - expect(wf).toMatch(/hyperdx_get_dashboard \(no id\)/); - // Step 4 must call out grouping tiles into containers before save. - expect(wf).toMatch(/group them into 2-4 containers/); - // Step 6 must call out EVERY tile, not just one. - expect(wf).toMatch(/EVERY tile \(not just one\)/); - }); - - it('warns explicitly against title-recap markdown tiles in the tile type guide', () => { - // The TILE TYPE GUIDE entry for markdown is where a model reading - // top-down first decides whether to add an "About this dashboard" - // tile. The entry has to actively discourage it, not just describe - // the tile type, because every model trained on dashboard examples - // reaches for a title tile by default. - const prompt = buildCreateDashboardPrompt( - 'summary', - 'trace_src', - 'log_src', - ); - const guideIdx = prompt.indexOf('== TILE TYPE GUIDE =='); - const guideEnd = prompt.indexOf('\n== ', guideIdx + 1); - const guideBody = prompt.slice(guideIdx, guideEnd); - // The markdown line must say "skip" / "sparingly" / "do not" / - // "no headings", not just "use for notes". Look for the most - // load-bearing phrase. - expect(guideBody).toMatch(/Skip markdown tiles for starter dashboards/); - }); - - it('explains the 15-minute default time window UX trap', () => { - // Empty tiles on a fresh dashboard are usually a time-window problem, - // not a query problem. The prompt has to surface this so Claude - // tells the user (rather than silently padding individual tile time - // ranges to make a 24-hour view fit a 15-minute dashboard frame). - const prompt = buildCreateDashboardPrompt( - 'summary', - 'trace_src', - 'log_src', - ); - expect(prompt).toMatch(/DEFAULT TIME WINDOW/); - expect(prompt).toMatch(/15-minute default window/); - }); - - it('contains no em-dashes or en-dashes used as em-dashes', () => { - const prompt = buildCreateDashboardPrompt('sources summary', '', ''); - expect(prompt).not.toMatch(/—/); // [prose-lint: allow] - // En-dash flanked by spaces reads as an em-dash substitute and is - // disallowed by the voice rules. Numeric ranges (e.g. "1-20") are - // not in the checklist anyway, but guard against them just in case. - expect(prompt).not.toMatch(/ – /); // [prose-lint: allow] - }); - - it("mentions row-click linking in the table tile's description", () => { - // The create prompt is the LLM's primary entry point for new - // dashboards. Surface onClick on the TILE TYPE GUIDE so the - // model considers wiring it up on overview tables without the - // user having to ask. - const prompt = buildCreateDashboardPrompt( - 'summary', - 'trace_src', - 'log_src', - ); - const guideIdx = prompt.indexOf('== TILE TYPE GUIDE =='); - expect(guideIdx).toBeGreaterThan(-1); - const guideEnd = prompt.indexOf('\n== ', guideIdx + 1); - const guideBody = prompt.slice(guideIdx, guideEnd); - // The table line must hint at onClick and point at the - // consolidated TABLE TILE LINKING section in the query guide. - expect(guideBody).toContain('onClick'); - expect(guideBody).toContain('TABLE TILE LINKING'); - }); - }); - - describe('buildDashboardExamplesPrompt', () => { - it('exposes exactly the four verified trace/log examples plus infrastructure_sql', () => { - const all = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - ); - // Order matters here only to keep the LLM\'s navigation stable - // across releases. If the order changes intentionally, update the - // snapshot. - const patternLineMatch = all.match(/Available patterns: ([^\n]+)/); - expect(patternLineMatch).toBeTruthy(); - const patternList = patternLineMatch?.[1].split(', '); - expect(patternList).toEqual([ - 'service_inventory', - 'service_detail', - 'log_analytics', - 'backend_dependencies', - 'drilldown_links', - 'infrastructure_sql', - ]); - }); - - it('renders each example with a leading "When to use" header', () => { - const all = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - ); - for (const heading of [ - '== SERVICE INVENTORY ==', - '== SERVICE DETAIL ==', - '== LOG ANALYTICS ==', - '== BACKEND DEPENDENCIES ==', - '== INFRASTRUCTURE (Raw SQL) ==', - ]) { - expect(all).toContain(heading); - } - // Each non-SQL example should explain WHEN to reach for it. The - // SQL example does too, but the heading differs slightly. - const nonSqlSections = all - .split('== ') - .filter(s => - [ - 'SERVICE INVENTORY', - 'SERVICE DETAIL', - 'LOG ANALYTICS', - 'BACKEND DEPENDENCIES', - ].some(h => s.startsWith(h)), - ); - for (const section of nonSqlSections) { - expect(section.toLowerCase()).toContain('when to use'); - } - }); - - it('returns a single example when filtered by pattern', () => { - const single = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - 'service_inventory', - ); - expect(single).toContain('== SERVICE INVENTORY =='); - expect(single).not.toContain('== SERVICE DETAIL =='); - expect(single).not.toContain('== LOG ANALYTICS =='); - }); - - it('wires service_inventory row-click into the service_detail dashboard', () => { - // The service_inventory pattern's main RED table should carry an - // onClick that drills into the partner "Service Detail" dashboard - // by name template, with a ServiceName filter rendered from the - // clicked row. Without this wiring, an agent following the canonical - // workflow would have to invent the drill-down each time and - // sometimes get the field shape wrong. - const single = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - 'service_inventory', - ); - // The onClick target points at the partner pattern by name. - expect(single).toContain( - 'target: { mode: "template", template: "Service Detail" }', - ); - // The onClick filter expression matches service_detail's filter expression, - // so the destination dashboard's "Service" dropdown auto-populates. - expect(single).toContain('expression: "ServiceName"'); - // The template references the row's groupBy column (ServiceName). - expect(single).toContain('template: "{{ServiceName}}"'); - }); - - it('exposes a drilldown_links example pattern with onClick wiring', () => { - // Drew's drilldown_links example carries a self-contained - // onClick walkthrough (search drill-down + dashboard drill-down). - // Keep it discoverable so an agent asked to wire up drill-downs - // can opt into this example by pattern name. - const all = buildDashboardExamplesPrompt( - 'trace_src', - 'log_src', - 'conn_id', - ); - expect(all).toContain('drilldown_links'); - - const filtered = buildDashboardExamplesPrompt( - 'trace_src', - 'log_src', - 'conn_id', - 'drilldown_links', - ); - expect(filtered).toContain('ROW-CLICK DRILL-DOWN LINKS'); - expect(filtered).toContain('onClick'); - expect(filtered).toContain('type: "search"'); - expect(filtered).toContain('type: "dashboard"'); - expect(filtered).toContain('expressionTemplate'); - }); - - it('falls back to showing all examples when pattern is unknown', () => { - const fallback = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - 'made_up_pattern', - ); - expect(fallback).toContain('No example found for pattern'); - // Listing every example name in the fallback gives the LLM a way - // to recover by re-requesting one of the known patterns. - expect(fallback).toContain('service_inventory'); - expect(fallback).toContain('service_detail'); - }); - - it('contains no em-dashes in any example body', () => { - const all = buildDashboardExamplesPrompt( - '000000000000000000000001', - '000000000000000000000002', - '000000000000000000000003', - ); - expect(all).not.toMatch(/—/); // [prose-lint: allow] - }); - }); -}); diff --git a/packages/api/src/mcp/__tests__/dashboards/deleteDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/deleteDashboard.test.ts new file mode 100644 index 0000000000..0e37bcd08c --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/deleteDashboard.test.ts @@ -0,0 +1,38 @@ +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_delete_dashboard', () => { + const ctx = setupDashboardTests(); + + it('should delete an existing dashboard', async () => { + const dashboard = await new Dashboard({ + name: 'To Delete', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_delete_dashboard', { + id: dashboard._id.toString(), + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.deleted).toBe(true); + expect(output.id).toBe(dashboard._id.toString()); + + // Verify deleted from database + const found = await Dashboard.findById(dashboard._id); + expect(found).toBeNull(); + }); + + it('should return error for non-existent dashboard', async () => { + const result = await callTool(ctx.client!, 'hyperdx_delete_dashboard', { + id: '000000000000000000000000', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/getDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/getDashboard.test.ts new file mode 100644 index 0000000000..ebb41ffe85 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/getDashboard.test.ts @@ -0,0 +1,65 @@ +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_get_dashboard', () => { + const ctx = setupDashboardTests(); + + // Inside each test, use ctx.team, ctx.traceSource, ctx.connection, ctx.client + // Replace bare `team` with `ctx.team`, `client` with `ctx.client!`, etc. + + it('should list all dashboards when no id provided', async () => { + await new Dashboard({ + name: 'Dashboard 1', + tiles: [], + team: ctx.team._id, + tags: ['tag1'], + }).save(); + await new Dashboard({ + name: 'Dashboard 2', + tiles: [], + team: ctx.team._id, + tags: ['tag2'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard', {}); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(2); + expect(output[0]).toHaveProperty('id'); + expect(output[0]).toHaveProperty('name'); + expect(output[0]).toHaveProperty('tags'); + }); + + it('should get dashboard detail when id is provided', async () => { + const dashboard = await new Dashboard({ + name: 'My Dashboard', + tiles: [], + team: ctx.team._id, + tags: ['test'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: dashboard._id.toString(), + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.id).toBe(dashboard._id.toString()); + expect(output.name).toBe('My Dashboard'); + expect(output.tags).toEqual(['test']); + expect(output.tiles).toEqual([]); + }); + + it('should return error for non-existent dashboard id', async () => { + const fakeId = '000000000000000000000000'; + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: fakeId, + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/getDashboardTile.test.ts b/packages/api/src/mcp/__tests__/dashboards/getDashboardTile.test.ts new file mode 100644 index 0000000000..36b3bb1a51 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/getDashboardTile.test.ts @@ -0,0 +1,83 @@ +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_get_dashboard_tile', () => { + const ctx = setupDashboardTests(); + + it('should return a single tile by tileId', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Tile Test Dashboard', + tiles: [ + { + name: 'Line Chart', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Number Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const dashboardId = created.id; + const tileId = created.tiles[0].id; + + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard_tile', { + dashboardId, + tileId, + }); + + expect(result.isError).toBeFalsy(); + const tile = JSON.parse(getFirstText(result)); + expect(tile.id).toBe(tileId); + expect(tile.name).toBe('Line Chart'); + expect(tile.config.displayType).toBe('line'); + }); + + it('should return error for non-existent tileId', async () => { + const dashboard = await new Dashboard({ + name: 'Empty Dashboard', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard_tile', { + dashboardId: dashboard._id.toString(), + tileId: 'nonexistent-tile', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Tile not found'); + }); + + it('should return error for non-existent dashboard', async () => { + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard_tile', { + dashboardId: '000000000000000000000000', + tileId: 'some-tile', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); + + it('should return error for invalid dashboard ID', async () => { + const result = await callTool(ctx.client!, 'hyperdx_get_dashboard_tile', { + dashboardId: 'not-valid', + tileId: 'some-tile', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Invalid dashboard ID'); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts new file mode 100644 index 0000000000..1e72c98b09 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts @@ -0,0 +1,322 @@ +import Dashboard from '@/models/dashboard'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_patch_dashboard', () => { + const ctx = setupDashboardTests(); + + it('should patch a tile config without affecting other tiles', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Patch Test Dashboard', + tiles: [ + { + name: 'Tile A', + x: 0, + y: 0, + w: 12, + h: 4, + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Tile B', + x: 0, + y: 4, + w: 12, + h: 4, + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const dashboardId = created.id; + const tileAId = created.tiles[0].id; + + // Patch only tile A: change its name and displayType + const patchResult = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId, + tileId: tileAId, + tile: { + name: 'Patched Tile A', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + }); + + expect(patchResult.isError).toBeFalsy(); + const patchOutput = JSON.parse(getFirstText(patchResult)); + expect(patchOutput.patchedTile.name).toBe('Patched Tile A'); + expect(patchOutput.patchedTile.config.displayType).toBe('table'); + + // Verify the full dashboard: tile B should be untouched + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: dashboardId, + }); + const dashboard = JSON.parse(getFirstText(getResult)); + expect(dashboard.tiles).toHaveLength(2); + + const tileA = dashboard.tiles.find( + (t: ExternalDashboardTileWithId) => t.id === tileAId, + ); + expect(tileA.name).toBe('Patched Tile A'); + expect(tileA.config.displayType).toBe('table'); + + const tileB = dashboard.tiles.find( + (t: ExternalDashboardTileWithId) => t.id !== tileAId, + ); + expect(tileB.name).toBe('Tile B'); + expect(tileB.config.displayType).toBe('number'); + }); + + it('should preserve existing layout when not specified in patch', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Layout Test', + tiles: [ + { + name: 'Positioned Tile', + x: 6, + y: 10, + w: 8, + h: 5, + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const tileId = created.tiles[0].id; + + // Patch without specifying layout — layout should be preserved + const patchResult = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + tileId, + tile: { + name: 'Renamed Tile', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + }); + + expect(patchResult.isError).toBeFalsy(); + const patched = JSON.parse(getFirstText(patchResult)); + expect(patched.patchedTile.x).toBe(6); + expect(patched.patchedTile.y).toBe(10); + expect(patched.patchedTile.w).toBe(8); + expect(patched.patchedTile.h).toBe(5); + expect(patched.patchedTile.name).toBe('Renamed Tile'); + }); + + it('should update dashboard name only (no tile patch)', async () => { + const dashboard = await new Dashboard({ + name: 'Original Name', + tiles: [], + team: ctx.team._id, + tags: ['tag1'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: dashboard._id.toString(), + name: 'New Name', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.name).toBe('New Name'); + expect(output.tags).toEqual(['tag1']); + expect(output.patchedTile).toBeUndefined(); + + // Verify in DB + const updated = await Dashboard.findById(dashboard._id); + expect(updated?.name).toBe('New Name'); + }); + + it('should update tags only', async () => { + const dashboard = await new Dashboard({ + name: 'Tag Test', + tiles: [], + team: ctx.team._id, + tags: ['old'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: dashboard._id.toString(), + tags: ['new1', 'new2'], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tags).toEqual(['new1', 'new2']); + expect(output.name).toBe('Tag Test'); + }); + + it('should update name and patch tile in one call', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Combo Test', + tiles: [ + { + name: 'A Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + name: 'Combo Updated', + tileId: created.tiles[0].id, + tile: { + name: 'Updated Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.name).toBe('Combo Updated'); + expect(output.patchedTile.name).toBe('Updated Tile'); + }); + + it('should return error for non-existent tileId', async () => { + const dashboard = await new Dashboard({ + name: 'Tile Not Found', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: dashboard._id.toString(), + tileId: 'nonexistent', + tile: { + name: 'Ghost', + config: { displayType: 'markdown', markdown: 'hello' }, + }, + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Tile not found'); + }); + + it('should return error for non-existent dashboard', async () => { + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: '000000000000000000000000', + name: 'Ghost', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); + + it('should return error for missing source ID on patched tile', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Source Validation Test', + tiles: [ + { + name: 'Valid Tile', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + + const result = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + tileId: created.tiles[0].id, + tile: { + name: 'Bad Source', + config: { + displayType: 'line', + sourceId: '000000000000000000000000', + select: [{ aggFn: 'count' }], + }, + }, + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('source'); + }); + + it('should round-trip: patch then get_dashboard_tile', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Round Trip Test', + tiles: [ + { + name: 'Original', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const tileId = created.tiles[0].id; + + // Patch + await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + tileId, + tile: { + name: 'Patched', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + }); + + // Get the tile back + const getResult = await callTool( + ctx.client!, + 'hyperdx_get_dashboard_tile', + { + dashboardId: created.id, + tileId, + }, + ); + + expect(getResult.isError).toBeFalsy(); + const tile = JSON.parse(getFirstText(getResult)); + expect(tile.id).toBe(tileId); + expect(tile.name).toBe('Patched'); + expect(tile.config.displayType).toBe('number'); + expect(tile.config.select[0].aggFn).toBe('avg'); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/prompts.test.ts b/packages/api/src/mcp/__tests__/dashboards/prompts.test.ts new file mode 100644 index 0000000000..139bbbddf5 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/prompts.test.ts @@ -0,0 +1,576 @@ +import { + buildCreateDashboardPrompt, + buildDashboardExamplesPrompt, + buildQueryGuidePrompt, +} from '../../prompts/dashboards/content'; +import { buildSourceSummary } from '../../prompts/dashboards/helpers'; + +describe('MCP Dashboard Prompts', () => { + describe('buildQueryGuidePrompt', () => { + it('documents heatmap in tile-type, constraints, mistakes, and aggFn sections', () => { + const prompt = buildQueryGuidePrompt(); + // Ensure each section the bot called out has a heatmap entry so + // the LLM cannot skip the constraints when producing a heatmap + // tile through hyperdx_save_dashboard. + const sections = [ + '== AGGREGATION FUNCTIONS (aggFn) ==', + '== PER-TILE TYPE CONSTRAINTS ==', + '== COMMON MISTAKES ==', + ]; + for (const heading of sections) { + const idx = prompt.indexOf(heading); + expect(idx).toBeGreaterThan(-1); + // Each section's body extends until the next == heading or the + // end of the string. Checking for the substring "heatmap" in + // that slice is enough to assert the heatmap entry exists. + const next = prompt.indexOf('\n== ', idx + heading.length); + const body = prompt.slice(idx, next === -1 ? prompt.length : next); + expect(body.toLowerCase()).toContain('heatmap'); + } + }); + + it('documents the dashboard filter and per-series numberFormat sections', () => { + const prompt = buildQueryGuidePrompt(); + // Sections must exist AND carry the substantive content. Heading- + // only assertions let a future contributor empty out a section + // and silently pass review. + const sliceSection = (heading: string) => { + const idx = prompt.indexOf(heading); + if (idx === -1) return ''; + const next = prompt.indexOf('\n== ', idx + heading.length); + return prompt.slice(idx, next === -1 ? prompt.length : next); + }; + const filters = sliceSection('== DASHBOARD FILTERS =='); + expect(filters).toContain('QUERY_EXPRESSION'); + expect(filters).toContain('expression'); + expect(filters).toContain('sourceId'); + const numberFormat = sliceSection('== NUMBER FORMAT =='); + expect(numberFormat).toContain('factor: 0.000000001'); + expect(numberFormat).toContain('duration'); + expect(numberFormat.toLowerCase()).toContain('per-series'); + }); + + it('declares the MCP metric-authoring gap rather than referencing fields that do not exist', () => { + // The MCP select-item schema does not carry metricName / metricType, + // so any prompt that tells the model to author a metric tile via + // the builder path is teaching it to ship JSON that gets silently + // stripped. Guard with an explicit assertion so a future diff that + // restores metric-tile guidance fails this test loudly. + const prompt = buildQueryGuidePrompt(); + expect(prompt).not.toMatch(/metricName \+ metricType/); + expect(prompt).not.toMatch(/exactly 1 select item with metricName/); + const constraintsIdx = prompt.indexOf('== PER-TILE TYPE CONSTRAINTS =='); + const constraintsBody = prompt.slice(constraintsIdx); + // The constraints section closes with an explicit note that builder + // tiles on a metric source are not reliable today, with a fallback + // recipe to raw SQL. Anchor on the phrase so a future diff that + // drops the gap-acknowledgement fails loudly. + expect(constraintsBody).toMatch( + /Authoring builder tiles on a metric source is not reliable/, + ); + expect(constraintsBody).toMatch(/MCP select-item shape does not carry/); + }); + + it('documents table-tile onClick linking features', () => { + // Lock down the documentation for row-click drill-downs so a + // future refactor can't quietly drop the section the LLM relies + // on to wire up onClick correctly. + const prompt = buildQueryGuidePrompt(); + const idx = prompt.indexOf('== TABLE TILE LINKING (config.onClick) =='); + expect(idx).toBeGreaterThan(-1); + const next = prompt.indexOf('\n== ', idx + 1); + const section = prompt.slice(idx, next === -1 ? prompt.length : next); + + // Both destination types are mentioned. + expect(section).toContain('type: "search"'); + expect(section).toContain('type: "dashboard"'); + // Both target modes are mentioned. + expect(section).toContain('mode: "id"'); + expect(section).toContain('mode: "template"'); + // Templating fields are mentioned. + expect(section).toContain('whereTemplate'); + expect(section).toContain('filters'); + expect(section).toContain('expressionTemplate'); + // The two server-side validation error messages are quoted so + // the LLM can recognize and recover from them. + expect(section).toContain('onClick search source IDs'); + expect(section).toContain('onClick dashboard IDs'); + }); + + it('documents onClick pitfalls under common mistakes', () => { + const prompt = buildQueryGuidePrompt(); + const idx = prompt.indexOf('== COMMON MISTAKES =='); + expect(idx).toBeGreaterThan(-1); + const section = prompt.slice(idx); + // Mention the four failure modes we hit during validation. + expect(section.toLowerCase()).toContain('onclick on a non-table tile'); + expect(section.toLowerCase()).toContain('non-log/trace source'); + expect(section.toLowerCase()).toContain('missing wherelanguage'); + expect(section.toLowerCase()).toContain("isn't in the table"); + }); + + it('documents the missing-alias mistake explicitly', () => { + // Claude built three number tiles without alias on the quantile() + // select item, even though rule 2 of the design checklist says + // ALIAS EVERY SELECT ITEM. Spelling out the mistake under COMMON + // MISTAKES gives the model a second touchpoint to catch it. + const prompt = buildQueryGuidePrompt(); + const idx = prompt.indexOf('== COMMON MISTAKES =='); + const section = prompt.slice(idx); + expect(section).toMatch(/Missing alias on a select item/); + // Must call out that number tiles need the alias too; this is the + // exact case Claude got wrong. + expect(section).toMatch(/number tiles too/); + }); + + it('documents the Lucene map-attribute gotcha across all operators including simple equality', () => { + // The original rule covered only comparison/wildcard. Claude's + // second pass showed simple equality (db.system:mongodb) also + // fails to translate to SpanAttributes['db.system']. Broaden the + // gotcha to "ALL Lucene operations on map-attribute paths". + const prompt = buildQueryGuidePrompt(); + const luceneIdx = prompt.indexOf('== LUCENE FILTER SYNTAX =='); + const luceneBody = prompt.slice(luceneIdx); + expect(luceneBody).toMatch(/NOT reliably translated to bracket access/); + expect(luceneBody).toMatch(/http\.status_code:>=500/); // operator + expect(luceneBody).toMatch(/http\.route:\*/); // wildcard + expect(luceneBody).toMatch(/db\.system:mongodb/); // simple equality + expect(luceneBody).toMatch(/use SQL with bracket access/); + + // Also a top-level entry in COMMON MISTAKES for discoverability. + const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); + const mistakesBody = prompt.slice(mistakesIdx); + expect(mistakesBody).toMatch( + /Lucene on a map-attribute path \(any operation\)/, + ); + }); + + it('documents the Lucene fuzzy-substring behavior on top-level columns', () => { + // Lucene field:value translates to ilike(field, '%value%'), not + // equality. Claude's second pass hit this on SpanKind:Server, + // which matched broader strings than intended. The note has to + // be in the Lucene section so a model reading top-down sees it + // before writing a SpanKind filter. + const prompt = buildQueryGuidePrompt(); + const luceneIdx = prompt.indexOf('== LUCENE FILTER SYNTAX =='); + const luceneBody = prompt.slice(luceneIdx); + expect(luceneBody).toMatch(/SUBSTRING MATCHING/); + expect(luceneBody).toMatch(/ilike\(field, '%value%'\)/); + // The canonical fix is SQL with =. + expect(luceneBody).toMatch(/SpanKind = 'Server'/); + // Wildcard limitation is documented alongside substring matching. + expect(luceneBody).toMatch(/prefix-within-substring/); + expect(luceneBody).toMatch(/LIKE 'api%'/); + + // And a COMMON MISTAKES entry covering enum-like columns and wildcards. + const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); + const mistakesBody = prompt.slice(mistakesIdx); + expect(mistakesBody).toMatch(/Lucene field:value and field:value\*/); + }); + + it('documents the empty-string trap for map-attribute groupBy', () => { + // Map columns return '' (not NULL) when a key is unset on a row. + // A groupBy on such a key produces an empty-string bucket alongside + // the real values, which is visual noise on tables and pies. The + // standard fix is where: " != ''" alongside the groupBy. + const prompt = buildQueryGuidePrompt(); + const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); + const mistakesBody = prompt.slice(mistakesIdx); + expect(mistakesBody).toMatch( + /Forgetting that map-attribute values are often empty strings/, + ); + expect(mistakesBody).toMatch(/db\.collection\.name'\] != ''/); + }); + + it('documents the groupBy alias workaround for map-attribute drill-downs', () => { + // Builder tiles don't expose a groupBy alias today; map-attribute + // groupBys produce result columns named like the raw expression, + // which onClick template lookups can't reference. The workaround + // (author the tile as raw SQL with AS) needs to be findable. + const prompt = buildQueryGuidePrompt(); + const idx = prompt.indexOf( + '== GROUPBY ALIASES AND ROW-CLICK TEMPLATES ==', + ); + expect(idx).toBeGreaterThan(-1); + const section = prompt.slice(idx); + expect(section).toMatch(/do NOT currently expose a groupBy alias/); + expect(section).toMatch(/configType: "sql", displayType: "table"/); + expect(section).toMatch(/AS Route/); + }); + + it('points at ClickStack and ClickHouse docs in REFERENCES', () => { + // Without external doc links, the LLM has to guess at Lucene + // operators and ClickHouse function names. Anchor the canonical + // URLs in the prompt so it can cite them and so a future doc + // restructure can be caught here. + const prompt = buildQueryGuidePrompt(); + const idx = prompt.indexOf('== REFERENCES =='); + expect(idx).toBeGreaterThan(-1); + const section = prompt.slice(idx); + expect(section).toContain( + 'https://clickhouse.com/docs/use-cases/observability/clickstack/search', + ); + expect(section).toContain('https://clickhouse.com/docs/sql-reference'); + expect(section).toContain( + 'https://clickhouse.com/docs/use-cases/observability/clickstack/dashboards/sql-visualizations', + ); + }); + + it('strengthens the validate-every-tile rule under common mistakes', () => { + // Saving validates input shape, not query semantics. The prompt + // has to push for query_tile on every tile, not "at least one". + const prompt = buildQueryGuidePrompt(); + const mistakesIdx = prompt.indexOf('== COMMON MISTAKES =='); + const section = prompt.slice(mistakesIdx); + expect(section).toMatch( + /on EVERY tile after hyperdx_save_dashboard, not just one/, + ); + }); + + it('flags the metric source builder gap', () => { + // Builder tiles on metric sources currently save but render with + // "Both table name and UUID are empty". Claude went 100% raw SQL + // across 21 metric tiles for this reason; the prompt has to make + // the workaround obvious so the model doesn't try the builder + // first and fail silently. + const prompt = buildQueryGuidePrompt(); + expect(prompt).toMatch(/Both table name and UUID are empty/); + expect(prompt).toMatch(/use a raw SQL tile/); + expect(prompt).toMatch(/otel_metrics_gauge/); + }); + + it('contains no em-dashes or en-dashes used as em-dashes', () => { + const prompt = buildQueryGuidePrompt(); + expect(prompt).not.toMatch(/\u2014/); // [prose-lint: allow] + expect(prompt).not.toMatch(/ \u2013 /); // [prose-lint: allow] + }); + }); + + describe('buildSourceSummary', () => { + it('contains no em-dashes or en-dashes used as em-dashes', () => { + // buildSourceSummary is the source-list block that gets prepended + // to create_dashboard. The first iteration of this PR shipped with + // em-dashes here that the content.ts snapshot test missed because + // it calls the builders directly. Guard the helper output too. + const summary = buildSourceSummary( + [ + { + _id: '000000000000000000000001', + name: 'Traces', + kind: 'trace', + connection: '000000000000000000000002', + }, + { + _id: '000000000000000000000003', + name: 'Logs', + kind: 'log', + connection: '000000000000000000000002', + }, + ], + [{ _id: '000000000000000000000002', name: 'Default' }], + ); + expect(summary).not.toMatch(/\u2014/); // [prose-lint: allow] + expect(summary).not.toMatch(/ \u2013 /); // [prose-lint: allow] + // Sanity: the helper still emits the source list, so the assertion + // above isn't trivially passing on an empty string. + expect(summary).toContain('Traces'); + expect(summary).toContain('AVAILABLE SOURCES'); + }); + }); + + describe('buildCreateDashboardPrompt', () => { + it('includes the design checklist and adapt-do-not-copy note', () => { + const prompt = buildCreateDashboardPrompt( + 'sources summary', + '000000000000000000000001', + '000000000000000000000002', + ); + expect(prompt).toContain('== DESIGN CHECKLIST =='); + // Each rule on the checklist exists at the same numbered position + // so a future contributor cannot silently drop one. Anchor on + // line start (with a trailing space) so digit-and-dot substrings + // like "0.000000001" elsewhere in the prompt cannot satisfy the + // assertion accidentally. + const checklistIdx = prompt.indexOf('== DESIGN CHECKLIST =='); + const adaptIdx = prompt.indexOf('== ADAPT, DO NOT COPY =='); + const checklistBody = prompt.slice(checklistIdx, adaptIdx); + // Thirteen rules: the original ten plus GROUP BY HAS NO ALIAS HOOK + // (rule 3), VALIDATE EVERY TILE AFTER SAVE (rule 12), and NO + // TITLE-RECAP MARKDOWN TILE (rule 13). Each came out of a live + // verification pass after watching Claude reliably ignore the soft + // "should" formulations or hit a schema gap the earlier checklist + // did not call out. + for (let i = 1; i <= 13; i++) { + expect(checklistBody).toMatch(new RegExp(`^${i}\\. `, 'm')); + } + expect(prompt).toContain('ADAPT, DO NOT COPY'); + // Rule 9 documents the replace-not-merge semantic on update; an + // agent that ignores this can silently drop tiles / filters / containers + // on a subsequent save call. + expect(checklistBody).toMatch(/UPDATE IS REPLACE, NOT MERGE/); + // Rule 2 must cover EVERY select item (including number tiles), not + // just aggregations on tables. The phrasing matters: Claude's first + // pass at this dropped aliases on number tiles because it read rule 2 + // as table-specific. The stronger phrasing pulls them in. + expect(checklistBody).toMatch(/ALIAS EVERY SELECT ITEM/); + // Rule 3 calls out the schema gap on groupBy: the chart config takes + // a single expression string with no alias field, so a table grouped + // by SpanAttributes['http.route'] renders arrayElement(...) as the + // column header. Naming the limit explicitly lets Claude pick a + // top-level column (SpanName, ServiceName) instead of grouping on + // the raw Map expression. + expect(checklistBody).toMatch(/GROUP BY HAS NO ALIAS HOOK/); + // Rule 10 must be a hard requirement at 5+ tiles, not a soft hint. + // Without the imperative, Claude built five dashboards averaging ten + // tiles each with zero containers. + expect(checklistBody).toMatch(/REQUIRED at five or more tiles/); + // Rule 11 + 12 close the post-save loop and the title-recap markdown + // habit Claude landed on every starter dashboard. + expect(checklistBody).toMatch(/VALIDATE EVERY TILE AFTER SAVE/); + expect(checklistBody).toMatch(/NO TITLE-RECAP MARKDOWN TILE/); + // Rule 2 now carries a concrete number-tile example. Claude's + // second pass still dropped aliases on every builder number tile, + // because the text rule was abstract; a copy-pasteable shape next + // to the rule is what makes it stick. + expect(checklistBody).toMatch( + /Number tile, correct:.*alias: "Server Requests"/, + ); + // Rule 10 now carries a concrete containers shape. Claude's + // second pass built five dashboards with 9-10 tiles each and zero + // containers despite the REQUIRED phrasing; a copy-pasteable shape + // inside the rule itself is the next-level push. + expect(checklistBody).toMatch(/Concrete shape \(copy this directly/); + expect(checklistBody).toMatch(/id: "kpis"/); + expect(checklistBody).toMatch(/id: "trends"/); + expect(checklistBody).toMatch(/id: "errors"/); + }); + + it('walks the workflow through six steps including read-existing and group-into-containers', () => { + // The workflow is the first thing a model anchors on. Claude + // skipped both "read existing dashboards before designing" and + // "group tiles into containers before saving" because neither was + // a numbered workflow step; both lived only in the checklist. The + // workflow now lists six steps with those two explicitly. + const prompt = buildCreateDashboardPrompt( + 'summary', + 'trace_src', + 'log_src', + ); + const wfIdx = prompt.indexOf('== WORKFLOW =='); + const wfEnd = prompt.indexOf('\n== ', wfIdx + 1); + const wf = prompt.slice(wfIdx, wfEnd); + for (let i = 1; i <= 6; i++) { + expect(wf).toMatch(new RegExp(`^${i}\\. `, 'm')); + } + // Step 2 must point at hyperdx_get_dashboard with no id. + expect(wf).toMatch(/hyperdx_get_dashboard \(no id\)/); + // Step 4 must call out grouping tiles into containers before save. + expect(wf).toMatch(/group them into 2-4 containers/); + // Step 6 must call out EVERY tile, not just one. + expect(wf).toMatch(/EVERY tile \(not just one\)/); + }); + + it('warns explicitly against title-recap markdown tiles in the tile type guide', () => { + // The TILE TYPE GUIDE entry for markdown is where a model reading + // top-down first decides whether to add an "About this dashboard" + // tile. The entry has to actively discourage it, not just describe + // the tile type, because every model trained on dashboard examples + // reaches for a title tile by default. + const prompt = buildCreateDashboardPrompt( + 'summary', + 'trace_src', + 'log_src', + ); + const guideIdx = prompt.indexOf('== TILE TYPE GUIDE =='); + const guideEnd = prompt.indexOf('\n== ', guideIdx + 1); + const guideBody = prompt.slice(guideIdx, guideEnd); + // The markdown line must say "skip" / "sparingly" / "do not" / + // "no headings", not just "use for notes". Look for the most + // load-bearing phrase. + expect(guideBody).toMatch(/Skip markdown tiles for starter dashboards/); + }); + + it('explains the 15-minute default time window UX trap', () => { + // Empty tiles on a fresh dashboard are usually a time-window problem, + // not a query problem. The prompt has to surface this so Claude + // tells the user (rather than silently padding individual tile time + // ranges to make a 24-hour view fit a 15-minute dashboard frame). + const prompt = buildCreateDashboardPrompt( + 'summary', + 'trace_src', + 'log_src', + ); + expect(prompt).toMatch(/DEFAULT TIME WINDOW/); + expect(prompt).toMatch(/15-minute default window/); + }); + + it('contains no em-dashes or en-dashes used as em-dashes', () => { + const prompt = buildCreateDashboardPrompt('sources summary', '', ''); + expect(prompt).not.toMatch(/\u2014/); // [prose-lint: allow] + // En-dash flanked by spaces reads as an em-dash substitute and is + // disallowed by the voice rules. Numeric ranges (e.g. "1-20") are + // not in the checklist anyway, but guard against them just in case. + expect(prompt).not.toMatch(/ \u2013 /); // [prose-lint: allow] + }); + + it("mentions row-click linking in the table tile's description", () => { + // The create prompt is the LLM's primary entry point for new + // dashboards. Surface onClick on the TILE TYPE GUIDE so the + // model considers wiring it up on overview tables without the + // user having to ask. + const prompt = buildCreateDashboardPrompt( + 'summary', + 'trace_src', + 'log_src', + ); + const guideIdx = prompt.indexOf('== TILE TYPE GUIDE =='); + expect(guideIdx).toBeGreaterThan(-1); + const guideEnd = prompt.indexOf('\n== ', guideIdx + 1); + const guideBody = prompt.slice(guideIdx, guideEnd); + // The table line must hint at onClick and point at the + // consolidated TABLE TILE LINKING section in the query guide. + expect(guideBody).toContain('onClick'); + expect(guideBody).toContain('TABLE TILE LINKING'); + }); + }); + + describe('buildDashboardExamplesPrompt', () => { + it('exposes exactly the four verified trace/log examples plus infrastructure_sql', () => { + const all = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + ); + // Order matters here only to keep the LLM's navigation stable + // across releases. If the order changes intentionally, update the + // snapshot. + const patternLineMatch = all.match(/Available patterns: ([^\n]+)/); + expect(patternLineMatch).toBeTruthy(); + const patternList = patternLineMatch?.[1].split(', '); + expect(patternList).toEqual([ + 'service_inventory', + 'service_detail', + 'log_analytics', + 'backend_dependencies', + 'drilldown_links', + 'infrastructure_sql', + ]); + }); + + it('renders each example with a leading "When to use" header', () => { + const all = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + ); + for (const heading of [ + '== SERVICE INVENTORY ==', + '== SERVICE DETAIL ==', + '== LOG ANALYTICS ==', + '== BACKEND DEPENDENCIES ==', + '== INFRASTRUCTURE (Raw SQL) ==', + ]) { + expect(all).toContain(heading); + } + // Each non-SQL example should explain WHEN to reach for it. The + // SQL example does too, but the heading differs slightly. + const nonSqlSections = all + .split('== ') + .filter(s => + [ + 'SERVICE INVENTORY', + 'SERVICE DETAIL', + 'LOG ANALYTICS', + 'BACKEND DEPENDENCIES', + ].some(h => s.startsWith(h)), + ); + for (const section of nonSqlSections) { + expect(section.toLowerCase()).toContain('when to use'); + } + }); + + it('returns a single example when filtered by pattern', () => { + const single = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + 'service_inventory', + ); + expect(single).toContain('== SERVICE INVENTORY =='); + expect(single).not.toContain('== SERVICE DETAIL =='); + expect(single).not.toContain('== LOG ANALYTICS =='); + }); + + it('wires service_inventory row-click into the service_detail dashboard', () => { + // The service_inventory pattern's main RED table should carry an + // onClick that drills into the partner "Service Detail" dashboard + // by name template, with a ServiceName filter rendered from the + // clicked row. Without this wiring, an agent following the canonical + // workflow would have to invent the drill-down each time and + // sometimes get the field shape wrong. + const single = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + 'service_inventory', + ); + // The onClick target points at the partner pattern by name. + expect(single).toContain( + 'target: { mode: "template", template: "Service Detail" }', + ); + // The onClick filter expression matches service_detail's filter expression, + // so the destination dashboard's "Service" dropdown auto-populates. + expect(single).toContain('expression: "ServiceName"'); + // The template references the row's groupBy column (ServiceName). + expect(single).toContain('template: "{{ServiceName}}"'); + }); + + it('exposes a drilldown_links example pattern with onClick wiring', () => { + // Drew's drilldown_links example carries a self-contained + // onClick walkthrough (search drill-down + dashboard drill-down). + // Keep it discoverable so an agent asked to wire up drill-downs + // can opt into this example by pattern name. + const all = buildDashboardExamplesPrompt( + 'trace_src', + 'log_src', + 'conn_id', + ); + expect(all).toContain('drilldown_links'); + + const filtered = buildDashboardExamplesPrompt( + 'trace_src', + 'log_src', + 'conn_id', + 'drilldown_links', + ); + expect(filtered).toContain('ROW-CLICK DRILL-DOWN LINKS'); + expect(filtered).toContain('onClick'); + expect(filtered).toContain('type: "search"'); + expect(filtered).toContain('type: "dashboard"'); + expect(filtered).toContain('expressionTemplate'); + }); + + it('falls back to showing all examples when pattern is unknown', () => { + const fallback = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + 'made_up_pattern', + ); + expect(fallback).toContain('No example found for pattern'); + // Listing every example name in the fallback gives the LLM a way + // to recover by re-requesting one of the known patterns. + expect(fallback).toContain('service_inventory'); + expect(fallback).toContain('service_detail'); + }); + + it('contains no em-dashes in any example body', () => { + const all = buildDashboardExamplesPrompt( + '000000000000000000000001', + '000000000000000000000002', + '000000000000000000000003', + ); + expect(all).not.toMatch(/\u2014/); // [prose-lint: allow] + }); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/queryTile.test.ts b/packages/api/src/mcp/__tests__/dashboards/queryTile.test.ts new file mode 100644 index 0000000000..586a8d8224 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/queryTile.test.ts @@ -0,0 +1,98 @@ +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_query_tile', () => { + const ctx = setupDashboardTests(); + + it('should return error for non-existent dashboard', async () => { + const result = await callTool(ctx.client!, 'hyperdx_query_tile', { + dashboardId: '000000000000000000000000', + tileId: 'some-tile-id', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); + + it('should return error for non-existent tile', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Tile Query Test', + tiles: [ + { + name: 'My Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const dashboard = JSON.parse(getFirstText(createResult)); + + const result = await callTool(ctx.client!, 'hyperdx_query_tile', { + dashboardId: dashboard.id, + tileId: 'non-existent-tile-id', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Tile not found'); + }); + + it('should return error for invalid time range', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Time Range Test', + tiles: [ + { + name: 'Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const dashboard = JSON.parse(getFirstText(createResult)); + + const result = await callTool(ctx.client!, 'hyperdx_query_tile', { + dashboardId: dashboard.id, + tileId: dashboard.tiles[0].id, + startTime: 'not-a-date', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Invalid'); + }); + + it('should execute query for a valid tile', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Query Tile Test', + tiles: [ + { + name: 'Count Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const dashboard = JSON.parse(getFirstText(createResult)); + + const result = await callTool(ctx.client!, 'hyperdx_query_tile', { + dashboardId: dashboard.id, + tileId: dashboard.tiles[0].id, + startTime: new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(), + endTime: new Date().toISOString(), + }); + + // Should succeed (may have empty results since no data inserted) + expect(result.isError).toBeFalsy(); + expect(result.content).toHaveLength(1); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts new file mode 100644 index 0000000000..3bec1f0799 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts @@ -0,0 +1,1774 @@ +import { MetricsDataType, SourceKind } from '@hyperdx/common-utils/dist/types'; +import mongoose from 'mongoose'; + +import { DEFAULT_DATABASE } from '@/fixtures'; +import Dashboard from '@/models/dashboard'; +import { Source } from '@/models/source'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_save_dashboard', () => { + const ctx = setupDashboardTests(); + + describe('basic CRUD', () => { + it('should create a new dashboard with tiles', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'New MCP Dashboard', + tiles: [ + { + name: 'Line Chart', + x: 0, + y: 0, + w: 12, + h: 4, + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count', where: '' }], + }, + }, + ], + tags: ['mcp-test'], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.id).toBeDefined(); + expect(output.name).toBe('New MCP Dashboard'); + expect(output.tiles).toHaveLength(1); + expect(output.tiles[0].config.displayType).toBe('line'); + expect(output.tags).toEqual(['mcp-test']); + + // Verify in database + const dashboard = await Dashboard.findById(output.id); + expect(dashboard).not.toBeNull(); + expect(dashboard?.name).toBe('New MCP Dashboard'); + }); + + it('should create a dashboard with a markdown tile', async () => { + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Markdown Dashboard', + tiles: [ + { + name: 'Notes', + config: { + displayType: 'markdown', + markdown: '# Hello World', + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles).toHaveLength(1); + expect(output.tiles[0].config.displayType).toBe('markdown'); + }); + + it('should update an existing dashboard', async () => { + const sourceId = ctx.traceSource._id.toString(); + + // Create first + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'Original Name', + tiles: [ + { + name: 'Tile 1', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }, + ); + const created = JSON.parse(getFirstText(createResult)); + + // Update + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'Updated Name', + tiles: [ + { + name: 'Updated Tile', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + tags: ['updated'], + }, + ); + + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.id).toBe(created.id); + expect(updated.name).toBe('Updated Name'); + expect(updated.tiles).toHaveLength(1); + expect(updated.tiles[0].name).toBe('Updated Tile'); + expect(updated.tiles[0].config.displayType).toBe('table'); + }); + + it('should return error for missing source ID', async () => { + const fakeSourceId = '000000000000000000000000'; + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Bad Dashboard', + tiles: [ + { + name: 'Bad Tile', + config: { + displayType: 'line', + sourceId: fakeSourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('source'); + }); + + it('should return error when updating non-existent dashboard', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + id: '000000000000000000000000', + name: 'Ghost Dashboard', + tiles: [ + { + name: 'Tile', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); + + it('should create a dashboard with multiple tile types', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Multi-tile Dashboard', + tiles: [ + { + name: 'Line', + x: 0, + y: 0, + w: 12, + h: 4, + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Table', + x: 0, + y: 4, + w: 12, + h: 4, + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + groupBy: 'SpanName', + groupByColumnsOnLeft: true, + }, + }, + { + name: 'Number', + x: 0, + y: 8, + w: 6, + h: 3, + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Pie', + x: 6, + y: 8, + w: 6, + h: 3, + config: { + displayType: 'pie', + sourceId, + select: [{ aggFn: 'count' }], + groupBy: 'SpanName', + }, + }, + { + name: 'Notes', + x: 0, + y: 11, + w: 12, + h: 2, + config: { displayType: 'markdown', markdown: '# Dashboard Notes' }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles).toHaveLength(5); + + const tableTile = output.tiles.find( + (t: { name: string }) => t.name === 'Table', + ); + expect(tableTile).toBeDefined(); + expect(tableTile.config.groupByColumnsOnLeft).toBe(true); + }); + + it('should create a dashboard with a raw SQL tile', async () => { + const connectionId = ctx.connection._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'SQL Dashboard', + tiles: [ + { + name: 'Raw SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: 'SELECT 1 AS value LIMIT 1', + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles).toHaveLength(1); + }); + + it('should create a dashboard with a heatmap tile on a Trace source', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Heatmap Dashboard', + tiles: [ + { + name: 'Latency Heatmap', + x: 0, + y: 0, + w: 12, + h: 4, + config: { + displayType: 'heatmap', + sourceId, + select: [ + { + valueExpression: 'Duration', + }, + ], + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles).toHaveLength(1); + expect(output.tiles[0].config.displayType).toBe('heatmap'); + expect(output.tiles[0].config.select[0]).toMatchObject({ + valueExpression: 'Duration', + }); + }); + + it('should round-trip every MCP-specific heatmap field through save, get, update, and re-get', async () => { + const sourceId = ctx.traceSource._id.toString(); + + const createConfig = { + displayType: 'heatmap' as const, + sourceId, + select: [ + { + valueExpression: 'Duration', + countExpression: 'count()', + heatmapScaleType: 'log' as const, + }, + ], + where: 'level:error', + whereLanguage: 'lucene' as const, + numberFormat: { output: 'duration' as const, factor: 1e-9 }, + }; + // Mutate select + where on update; numberFormat, whereLanguage, + // and sourceId carry forward via the spread. Re-asserting against + // updatedConfig catches a regression where PUT silently drops any + // carried-forward field. + const updatedConfig = { + ...createConfig, + select: [ + { + valueExpression: "SpanAttributes['http.duration']", + heatmapScaleType: 'linear' as const, + }, + ], + where: 'level:error AND service:checkout', + }; + + const saveResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Heatmap Full Round-Trip', + tiles: [ + { + name: 'Latency Heatmap', + x: 0, + y: 0, + w: 12, + h: 4, + config: createConfig, + }, + ], + }); + expect(saveResult.isError).toBeFalsy(); + const saved = JSON.parse(getFirstText(saveResult)); + expect(saved.tiles).toHaveLength(1); + expect(saved.tiles[0].config).toMatchObject(createConfig); + + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: saved.id, + }); + expect(getResult.isError).toBeFalsy(); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.tiles[0].config).toMatchObject(createConfig); + + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: saved.id, + name: 'Heatmap Full Round-Trip', + tiles: [ + { + ...fetched.tiles[0], + config: { ...fetched.tiles[0].config, ...updatedConfig }, + }, + ], + }, + ); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.tiles[0].config).toMatchObject(updatedConfig); + + const getAfterUpdate = await callTool( + ctx.client!, + 'hyperdx_get_dashboard', + { + id: saved.id, + }, + ); + expect(getAfterUpdate.isError).toBeFalsy(); + const refetched = JSON.parse(getFirstText(getAfterUpdate)); + expect(refetched.tiles[0].config).toMatchObject(updatedConfig); + }); + + it('should reject heatmap tile with empty valueExpression at the schema layer', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Bad Heatmap', + tiles: [ + { + name: 'Heatmap', + config: { + displayType: 'heatmap', + sourceId, + select: [{ valueExpression: '' }], + }, + }, + ], + }); + + expect(result.isError).toBe(true); + }); + + it.each([ + { output: 'duration', factor: 1e-9 }, + { output: 'data_rate' }, + { output: 'throughput' }, + ] as const)( + 'should round-trip numberFormat output "$output" through save and get', + async numberFormat => { + const sourceId = ctx.traceSource._id.toString(); + + const saveResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: `NumberFormat ${numberFormat.output}`, + tiles: [ + { + name: 'Number Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + numberFormat, + }, + }, + { + name: 'Line Tile', + config: { + displayType: 'line', + sourceId, + select: [ + { aggFn: 'count' }, + { + aggFn: 'avg', + valueExpression: 'Duration', + numberFormat, + }, + ], + }, + }, + ], + }, + ); + + expect(saveResult.isError).toBeFalsy(); + const saved = JSON.parse(getFirstText(saveResult)); + + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: saved.id, + }); + expect(getResult.isError).toBeFalsy(); + const fetched = JSON.parse(getFirstText(getResult)); + + const numberTile = fetched.tiles.find( + (t: { name: string }) => t.name === 'Number Tile', + ); + expect(numberTile.config.numberFormat).toEqual(numberFormat); + + const lineTile = fetched.tiles.find( + (t: { name: string }) => t.name === 'Line Tile', + ); + expect(lineTile.config.select[1].numberFormat).toEqual(numberFormat); + }, + ); + + it('should reject numberFormat with an unknown output value', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Bad NumberFormat', + tiles: [ + { + name: 'Number Tile', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + numberFormat: { output: 'not_a_real_output' }, + }, + }, + ], + }); + + expect(result.isError).toBe(true); + }); + + it('should reject heatmap tile on a non-Trace source', async () => { + // Create a Log source so the heatmap source-kind gate has + // something to reject. The schema accepts the tile shape (the + // sourceId is valid), so this exercises the runtime check that + // the REST POST path also runs. + const logSource = await Source.create({ + kind: SourceKind.Log, + team: ctx.team._id, + from: { + databaseName: DEFAULT_DATABASE, + tableName: 'otel_logs', + }, + timestampValueExpression: 'Timestamp', + connection: ctx.connection._id, + name: 'Logs', + }); + + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Heatmap on Log Source', + tiles: [ + { + name: 'Heatmap', + config: { + displayType: 'heatmap', + sourceId: logSource._id.toString(), + select: [{ valueExpression: 'Duration' }], + }, + }, + ], + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('Trace source'); + expect(text).toContain(logSource._id.toString()); + }); + + // Exercises the update-side source-kind gate via filterChangedHeatmapTiles + // (displayType changed to heatmap on an existing tile). + it('should reject update that changes a tile to heatmap on a non-Trace source', async () => { + const logSource = await Source.create({ + kind: SourceKind.Log, + team: ctx.team._id, + from: { databaseName: DEFAULT_DATABASE, tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + connection: ctx.connection._id, + name: 'Logs', + }); + + const created = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Line on Log Source', + tiles: [ + { + name: 'Line', + config: { + displayType: 'line', + sourceId: logSource._id.toString(), + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + expect(created.isError).toBeFalsy(); + const saved = JSON.parse(getFirstText(created)); + + const update = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + id: saved.id, + name: 'Line on Log Source', + tiles: [ + { + ...saved.tiles[0], + config: { + ...saved.tiles[0].config, + displayType: 'heatmap', + select: [{ valueExpression: 'Duration' }], + }, + }, + ], + }); + expect(update.isError).toBe(true); + const text = getFirstText(update); + expect(text).toContain('Trace source'); + expect(text).toContain(logSource._id.toString()); + }); + + // Exercises the update-side source-kind gate via filterChangedHeatmapTiles + // (sourceId changed on an existing heatmap tile). + it('should reject update that changes a heatmap tile source to a non-Trace source', async () => { + const logSource = await Source.create({ + kind: SourceKind.Log, + team: ctx.team._id, + from: { databaseName: DEFAULT_DATABASE, tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + connection: ctx.connection._id, + name: 'Logs', + }); + + const created = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Heatmap re-pointed at Log', + tiles: [ + { + name: 'Heatmap', + config: { + displayType: 'heatmap', + sourceId: ctx.traceSource._id.toString(), + select: [{ valueExpression: 'Duration' }], + }, + }, + ], + }); + expect(created.isError).toBeFalsy(); + const saved = JSON.parse(getFirstText(created)); + + const update = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + id: saved.id, + name: 'Heatmap re-pointed at Log', + tiles: [ + { + ...saved.tiles[0], + config: { + ...saved.tiles[0].config, + sourceId: logSource._id.toString(), + }, + }, + ], + }); + expect(update.isError).toBe(true); + const text = getFirstText(update); + expect(text).toContain('Trace source'); + expect(text).toContain(logSource._id.toString()); + }); + + // Asserts each tile's config survives the serializer/deserializer cycle + // independently when mixed with other displayTypes on the same dashboard. + it('should round-trip a heatmap alongside line and number tiles in one dashboard', async () => { + const sourceId = ctx.traceSource._id.toString(); + + const heatmapConfig = { + displayType: 'heatmap' as const, + sourceId, + select: [ + { valueExpression: 'Duration', heatmapScaleType: 'log' as const }, + ], + }; + const lineConfig = { + displayType: 'line' as const, + sourceId, + select: [{ aggFn: 'count' as const }], + groupBy: "SpanAttributes['service.name']", + }; + const numberConfig = { + displayType: 'number' as const, + sourceId, + select: [{ aggFn: 'count' as const }], + numberFormat: { output: 'number' as const, average: true }, + }; + + const save = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Mixed Tile Round-Trip', + tiles: [ + { name: 'Heatmap Tile', config: heatmapConfig }, + { name: 'Line Tile', config: lineConfig }, + { name: 'Number Tile', config: numberConfig }, + ], + }); + expect(save.isError).toBeFalsy(); + const saved = JSON.parse(getFirstText(save)); + expect(saved.tiles).toHaveLength(3); + + const byName: Record = + Object.fromEntries( + saved.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), + ); + expect(byName['Heatmap Tile'].config).toMatchObject(heatmapConfig); + expect(byName['Line Tile'].config).toMatchObject(lineConfig); + expect(byName['Number Tile'].config).toMatchObject(numberConfig); + + const fetched = JSON.parse( + getFirstText( + await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: saved.id, + }), + ), + ); + const fetchedByName: Record = + Object.fromEntries( + fetched.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), + ); + expect(fetchedByName['Heatmap Tile'].config).toMatchObject(heatmapConfig); + expect(fetchedByName['Line Tile'].config).toMatchObject(lineConfig); + expect(fetchedByName['Number Tile'].config).toMatchObject(numberConfig); + }); + }); + + describe('containers and tabs', () => { + // Mirrors the v2 external-API "Containers and tabs" describe block so + // the MCP path enforces the same 5 cross-field rules: container id + // unique, tab id unique within a container, tile.containerId + // resolves, tile.tabId resolves to a tab on that container, and + // tile.tabId requires tile.containerId. + const buildTile = ( + sourceId: string, + overrides: Record, + ) => ({ + name: 'Tile', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line' as const, + sourceId, + select: [{ aggFn: 'count' as const, where: '' }], + }, + ...overrides, + }); + + it('should round-trip containers, tabs, and tile containerId/tabId on create and update', async () => { + const sourceId = ctx.traceSource._id.toString(); + const containers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + collapsible: true, + bordered: true, + tabs: [ + { id: 'errors', title: 'Errors' }, + { id: 'latency', title: 'Latency' }, + ], + }, + { + id: 'overview', + title: 'Overview', + collapsed: true, + }, + ]; + + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'Containers MCP Round-Trip', + tiles: [ + buildTile(sourceId, { + name: 'In Group, Tab A', + containerId: 'service-health', + tabId: 'errors', + }), + buildTile(sourceId, { + name: 'In Group, Tab B', + containerId: 'service-health', + tabId: 'latency', + }), + // Tile inside a tabbed container without a tabId renders in + // the container shell rather than under a tab. Guards that the + // schema does not accidentally require tabId for every tile in + // a tabbed container. + buildTile(sourceId, { + name: 'In Tabbed Group, No Tab', + containerId: 'service-health', + }), + buildTile(sourceId, { + name: 'In Plain Group', + containerId: 'overview', + }), + buildTile(sourceId, { name: 'Ungrouped' }), + ], + tags: ['containers-mcp'], + containers, + }, + ); + + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.containers).toEqual(containers); + + // Typed as ExternalDashboardTileWithId so a typo on .containerId + // would fail typecheck instead of silently asserting undefined. + const createdTilesByName: Record = + Object.fromEntries( + created.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), + ); + expect(createdTilesByName['In Group, Tab A']).toMatchObject({ + containerId: 'service-health', + tabId: 'errors', + }); + expect(createdTilesByName['In Group, Tab B']).toMatchObject({ + containerId: 'service-health', + tabId: 'latency', + }); + expect(createdTilesByName['In Tabbed Group, No Tab']).toMatchObject({ + containerId: 'service-health', + }); + expect( + createdTilesByName['In Tabbed Group, No Tab'].tabId, + ).toBeUndefined(); + expect(createdTilesByName['In Plain Group']).toMatchObject({ + containerId: 'overview', + }); + expect(createdTilesByName['In Plain Group'].tabId).toBeUndefined(); + expect(createdTilesByName.Ungrouped.containerId).toBeUndefined(); + expect(createdTilesByName.Ungrouped.tabId).toBeUndefined(); + + // Verify a fresh GET via the get tool also returns the structure. + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.containers).toEqual(containers); + + // Update: rename a tab, drop the second container, re-home tiles. + const updatedContainers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: true, + tabs: [ + { id: 'errors', title: 'Error Rate' }, + { id: 'latency', title: 'Latency' }, + ], + }, + ]; + const reHomedUngrouped = { + ...createdTilesByName.Ungrouped, + containerId: 'service-health', + tabId: 'errors', + }; + const droppedContainerTile = { + ...createdTilesByName['In Plain Group'], + containerId: undefined, + tabId: undefined, + }; + + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'Containers MCP Round-Trip', + tiles: [ + createdTilesByName['In Group, Tab A'], + createdTilesByName['In Group, Tab B'], + createdTilesByName['In Tabbed Group, No Tab'], + droppedContainerTile, + reHomedUngrouped, + ], + tags: ['containers-mcp'], + containers: updatedContainers, + }, + ); + + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.containers).toEqual(updatedContainers); + const updatedTilesByName: Record = + Object.fromEntries( + updated.tiles.map((t: ExternalDashboardTileWithId) => [t.name, t]), + ); + expect(updatedTilesByName['In Plain Group'].containerId).toBeUndefined(); + expect(updatedTilesByName.Ungrouped).toMatchObject({ + containerId: 'service-health', + tabId: 'errors', + }); + expect(updatedTilesByName['In Tabbed Group, No Tab'].containerId).toBe( + 'service-health', + ); + expect( + updatedTilesByName['In Tabbed Group, No Tab'].tabId, + ).toBeUndefined(); + }); + + it('should reject duplicate container ids', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Duplicate Container Ids', + tiles: [buildTile(sourceId, {})], + containers: [ + { id: 'dupe', title: 'A', collapsed: false }, + { id: 'dupe', title: 'B', collapsed: false }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Container IDs must be unique'); + }); + + it('should reject duplicate tab ids within a container', async () => { + // Structural rules (duplicate container ids, duplicate tab ids) fire + // through the body schema, which is reported as a stringified issue + // array. Cross-tile-ref rules (covered below) fire through + // collectTileContainerRefIssues and produce plain prose. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Duplicate Tab Ids', + tiles: [buildTile(sourceId, {})], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [ + { id: 'errors', title: 'Errors' }, + { id: 'errors', title: 'Errors Two' }, + ], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Duplicate tab id'); + expect(getFirstText(result)).toContain('errors'); + expect(getFirstText(result)).toContain('service-health'); + }); + + it('should reject a tile that supplies tabId without containerId', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Tab Without Container', + tiles: [buildTile(sourceId, { tabId: 'errors' })], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain( + 'tabId requires containerId to be set', + ); + }); + + it('should reject a tile that references an unknown containerId', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Unknown Container', + tiles: [buildTile(sourceId, { containerId: 'does-not-exist' })], + containers: [{ id: 'real', title: 'Real', collapsed: false }], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain( + 'unknown containerId "does-not-exist"', + ); + }); + + it('should reject a tile that references an unknown tabId within a container', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Unknown Tab', + tiles: [ + buildTile(sourceId, { + containerId: 'service-health', + tabId: 'ghost', + }), + ], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('unknown tabId "ghost"'); + }); + + it('should preserve existing containers on update when body omits the field', async () => { + // Exercises the `effectiveContainers = parsedContainers ?? existingDashboard.containers ?? []` + // fallback in the MCP update handler. Without it, a PUT that + // updates only `tiles` would reject because the tile's + // containerId reference would resolve against an empty array. + const sourceId = ctx.traceSource._id.toString(); + const containers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ]; + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'PUT-without-containers fallback', + tiles: [ + buildTile(sourceId, { + name: 'In Group', + containerId: 'service-health', + tabId: 'errors', + }), + ], + containers, + }, + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'PUT-without-containers fallback', + tiles: created.tiles, + // containers intentionally omitted + }, + ); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.containers).toEqual(containers); + expect(updated.tiles[0]).toMatchObject({ + containerId: 'service-health', + tabId: 'errors', + }); + }); + + it('should wipe persisted containers when update body sets containers: []', async () => { + // Mirror the v2 behavior: an explicit empty array clears the + // persisted containers, and the response normalizes [] back to + // absent on read. + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'Wipe containers', + tiles: [buildTile(sourceId, { name: 'Tile' })], + containers: [{ id: 'overview', title: 'Overview', collapsed: false }], + }, + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.containers).toHaveLength(1); + + const wipedTile = { + ...created.tiles[0], + containerId: undefined, + tabId: undefined, + }; + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'Wipe containers', + tiles: [wipedTile], + containers: [], + }, + ); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.containers).toBeUndefined(); + + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.containers).toBeUndefined(); + }); + + // Bounds mirror DASHBOARD_CONTAINER_ID_MAX (256) on the tile-level + // containerId / tabId. 256 chars must accept; 257 must reject. + // 257 trips the inputSchema's `.max(256)` and surfaces back as the + // MCP SDK's "Input validation error" envelope. + it('should accept a 256-char tile.containerId and reject 257', async () => { + const sourceId = ctx.traceSource._id.toString(); + const idAtMax = 'c'.repeat(256); + const okResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: '256-char containerId', + tiles: [buildTile(sourceId, { containerId: idAtMax })], + containers: [{ id: idAtMax, title: 'Max', collapsed: false }], + }); + expect(okResult.isError).toBeFalsy(); + + const idTooLong = 'c'.repeat(257); + const tooLongResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: '257-char containerId', + tiles: [buildTile(sourceId, { containerId: idTooLong })], + containers: [{ id: idTooLong, title: 'Too long', collapsed: false }], + }, + ); + expect(tooLongResult.isError).toBe(true); + expect(getFirstText(tooLongResult)).toContain( + 'String must contain at most 256 character(s)', + ); + expect(getFirstText(tooLongResult)).toContain('containerId'); + }); + }); + + describe('dashboard filters', () => { + // The MCP input schema delegates to createDashboardBodySchema / + // updateDashboardBodySchema for the filter shape. These tests guard + // that the MCP path lights up the same filter round-trip the v2 REST + // path already covers: filters are saved verbatim, get back with an + // assigned id, survive an update, and reject obvious bad inputs. + + const traceTile = (sourceId: string) => ({ + name: 'Volume', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line' as const, + sourceId, + select: [{ aggFn: 'count' as const, where: '' }], + }, + }); + + it('should round-trip filters on create, get, and update', async () => { + const sourceId = ctx.traceSource._id.toString(); + + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'Service Detail (MCP filter round-trip)', + tiles: [traceTile(sourceId)], + filters: [ + { + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }, + { + type: 'QUERY_EXPRESSION', + name: 'Environment', + expression: 'deployment.environment', + sourceId, + where: "deployment.environment = 'production'", + whereLanguage: 'sql', + }, + ], + }, + ); + + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(Array.isArray(created.filters)).toBe(true); + expect(created.filters).toHaveLength(2); + // The body schema assigns an id to each filter on create. Capture + // them so the update payload can include the same ids and the + // filter array round-trips identically (instead of being treated + // as wholesale replacement with new ids). + const [serviceFilter, envFilter] = created.filters; + expect(serviceFilter).toMatchObject({ + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }); + expect(typeof serviceFilter.id).toBe('string'); + expect(serviceFilter.id.length).toBeGreaterThan(0); + expect(envFilter).toMatchObject({ + type: 'QUERY_EXPRESSION', + name: 'Environment', + expression: 'deployment.environment', + sourceId, + where: "deployment.environment = 'production'", + whereLanguage: 'sql', + }); + + // Fetch and assert the same shape. + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.filters).toEqual(created.filters); + + // Update: rename the first filter and drop the second. The first + // filter keeps its id, the second is dropped (not preserved). + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'Service Detail (MCP filter round-trip)', + tiles: [traceTile(sourceId)], + filters: [ + { + id: serviceFilter.id, + type: 'QUERY_EXPRESSION', + name: 'Service (renamed)', + expression: 'ServiceName', + sourceId, + }, + ], + }, + ); + + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.filters).toHaveLength(1); + expect(updated.filters[0]).toMatchObject({ + id: serviceFilter.id, + type: 'QUERY_EXPRESSION', + name: 'Service (renamed)', + expression: 'ServiceName', + sourceId, + }); + }); + + it('should accept a create payload with stray filter ids (copy-paste round-trip)', async () => { + // An LLM that copies a filter out of hyperdx_get_dashboard and + // back into hyperdx_save_dashboard for a NEW dashboard ships an + // `id` on the filter. The body schema for create is + // `.strict()` and rejects `id`, so saveDashboard normalizes by + // stripping ids before validation. Without that normalization + // this payload would 4xx with a confusing strict-validation + // error. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Create with stray filter id', + tiles: [traceTile(sourceId)], + filters: [ + { + id: '999999999999999999999999', // simulates a copied id + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }, + ], + }); + expect(result.isError).toBeFalsy(); + const dashboard = JSON.parse(getFirstText(result)); + expect(dashboard.filters).toHaveLength(1); + // The new dashboard gets a fresh id; the copied one is discarded. + expect(dashboard.filters[0].id).not.toBe('999999999999999999999999'); + expect(dashboard.filters[0]).toMatchObject({ + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }); + }); + + it('should assign ids to new filters added during update (no-id round-trip)', async () => { + // An LLM updating a dashboard to ADD a brand-new filter ships + // a filter without an `id`. The body schema for update requires + // id, so saveDashboard fills one in. Without that normalization + // this payload would 4xx complaining that id is missing. + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'Update adds new filter', + tiles: [traceTile(sourceId)], + filters: [ + { + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }, + ], + }, + ); + const created = JSON.parse(getFirstText(createResult)); + const existingFilterId = created.filters[0].id; + + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'Update adds new filter', + tiles: [traceTile(sourceId)], + filters: [ + // Existing filter, preserved by id. + { + id: existingFilterId, + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + sourceId, + }, + // Brand-new filter, no id. saveDashboard assigns one. + { + type: 'QUERY_EXPRESSION', + name: 'Environment', + expression: 'deployment.environment', + sourceId, + }, + ], + }, + ); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.filters).toHaveLength(2); + const envFilter = updated.filters.find( + (f: { name: string }) => f.name === 'Environment', + ); + expect(envFilter).toBeDefined(); + expect(typeof envFilter.id).toBe('string'); + expect(envFilter.id.length).toBeGreaterThan(0); + expect(envFilter.id).not.toBe(existingFilterId); + }); + + it('should round-trip a table tile that uses a having clause', async () => { + // mcpTableTileSchema exposes `having` so the service_detail + // example's "Top Error Messages" pattern (groupBy StatusMessage + // with having: "StatusMessage != ''") survives MCP authoring. + // Without `having` on the schema, Zod would strip it and the + // saved tile would include empty-message rows instead of the + // filtered set the prompt promises. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Top Error Messages', + tiles: [ + { + name: 'Top Error Messages', + x: 0, + y: 0, + w: 12, + h: 6, + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count', alias: 'Count' }], + groupBy: 'StatusMessage', + having: "StatusMessage != ''", + orderBy: 'Count DESC', + groupByColumnsOnLeft: true, + }, + }, + ], + }); + expect(result.isError).toBeFalsy(); + const dashboard = JSON.parse(getFirstText(result)); + expect(dashboard.tiles[0].config.having).toBe("StatusMessage != ''"); + }); + }); + + describe('table onClick linking', () => { + // Mirrors the v2 external-API onClick tests so the MCP path + // enforces the same drill-down rules: row-click can target /search + // for a log/trace source or another dashboard, by concrete ID or + // by templated name, with optional whereTemplate/filters. + it('should round-trip a table tile with a search onClick by ID', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'OnClick search by id', + tiles: [ + { + name: 'Errors by Service', + x: 0, + y: 0, + w: 12, + h: 6, + config: { + displayType: 'table', + sourceId, + groupBy: "ResourceAttributes['service.name']", + select: [{ aggFn: 'count' }], + onClick: { + type: 'search', + target: { mode: 'id', id: sourceId }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: "{{ResourceAttributes['service.name']}}", + }, + ], + }, + }, + }, + ], + }, + ); + + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.tiles[0].config.onClick).toEqual({ + type: 'search', + target: { mode: 'id', id: sourceId }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: "{{ResourceAttributes['service.name']}}", + }, + ], + }); + + const getResult = await callTool(ctx.client!, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.tiles[0].config.onClick).toEqual( + created.tiles[0].config.onClick, + ); + }); + + it('should round-trip a table tile with a dashboard onClick by ID', async () => { + const sourceId = ctx.traceSource._id.toString(); + // Create the target dashboard the onClick will link to so the + // server-side `getMissingOnClickDashboards` check resolves. + const targetDashboard = await new Dashboard({ + name: 'Service Detail', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick dashboard by id', + tiles: [ + { + name: 'Top Services', + x: 0, + y: 0, + w: 12, + h: 6, + config: { + displayType: 'table', + sourceId, + groupBy: "ResourceAttributes['service.name']", + select: [{ aggFn: 'count' }], + onClick: { + type: 'dashboard', + target: { + mode: 'id', + id: targetDashboard._id.toString(), + }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: "{{ResourceAttributes['service.name']}}", + }, + ], + }, + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles[0].config.onClick).toEqual({ + type: 'dashboard', + target: { mode: 'id', id: targetDashboard._id.toString() }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: "{{ResourceAttributes['service.name']}}", + }, + ], + }); + }); + + it('should round-trip a templated dashboard onClick (mode=template)', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick dashboard by template', + tiles: [ + { + name: 'Service Picker', + x: 0, + y: 0, + w: 12, + h: 6, + config: { + displayType: 'table', + sourceId, + groupBy: 'SpanName', + select: [{ aggFn: 'count' }], + onClick: { + type: 'dashboard', + target: { + mode: 'template', + template: '{{TargetDashboard}}', + }, + whereLanguage: 'lucene', + whereTemplate: + "service.name:{{ResourceAttributes['service.name']}}", + }, + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles[0].config.onClick).toEqual({ + type: 'dashboard', + target: { mode: 'template', template: '{{TargetDashboard}}' }, + whereLanguage: 'lucene', + whereTemplate: "service.name:{{ResourceAttributes['service.name']}}", + }); + }); + + it('should round-trip onClick on a raw SQL table tile', async () => { + const connectionId = ctx.connection._id.toString(); + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'SQL onClick', + tiles: [ + { + name: 'Raw SQL Table', + x: 0, + y: 0, + w: 12, + h: 6, + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM otel_traces GROUP BY ServiceName LIMIT 50', + onClick: { + type: 'search', + target: { mode: 'id', id: sourceId }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }, + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles[0].config.onClick).toEqual({ + type: 'search', + target: { mode: 'id', id: sourceId }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }); + }); + + it('should reject a table tile onClick referencing a non-existent dashboard', async () => { + const sourceId = ctx.traceSource._id.toString(); + const ghostDashboardId = new mongoose.Types.ObjectId().toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick missing dashboard', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + onClick: { + type: 'dashboard', + target: { mode: 'id', id: ghostDashboardId }, + whereLanguage: 'sql', + }, + }, + }, + ], + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('onClick dashboard'); + expect(text).toContain(ghostDashboardId); + }); + + it('should reject a table tile onClick search target with a non-log/trace source', async () => { + // /search only renders log/trace sources; targeting a metric + // source must be rejected at save time, matching the REST POST + // path. Create the metric source inline so this test is + // self-contained. + const metricSource = await Source.create({ + kind: SourceKind.Metric, + team: ctx.team._id, + from: { + databaseName: DEFAULT_DATABASE, + tableName: '', + }, + metricTables: { + [MetricsDataType.Gauge.toLowerCase()]: 'otel_metrics_gauge', + [MetricsDataType.Sum.toLowerCase()]: 'otel_metrics_sum', + [MetricsDataType.Histogram.toLowerCase()]: 'otel_metrics_histogram', + }, + timestampValueExpression: 'TimeUnix', + connection: ctx.connection._id, + name: 'Metrics', + }); + const sourceId = ctx.traceSource._id.toString(); + + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick metric source', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + onClick: { + type: 'search', + target: { + mode: 'id', + id: metricSource._id.toString(), + }, + whereLanguage: 'sql', + }, + }, + }, + ], + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('log or trace'); + expect(text).toContain(metricSource._id.toString()); + }); + + it('should reject a table tile onClick with an invalid target.id', async () => { + // target.id must be a valid ObjectId. Bad shape is caught by the + // input schema and surfaces as the SDK's "Input validation error" + // envelope rather than a custom 400 message. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick bad object id', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + onClick: { + type: 'dashboard', + target: { mode: 'id', id: 'not-a-valid-object-id' }, + whereLanguage: 'sql', + }, + }, + }, + ], + }); + + expect(result.isError).toBe(true); + }); + + it('should accept onClick without whereLanguage (it is optional at the schema layer)', async () => { + // SearchConditionLanguageSchema is `.optional()` in common-utils, + // so omitting whereLanguage is allowed. Pin that behavior so a + // future tightening of the schema doesn't quietly change it + // without updating the docs that callers rely on. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick no whereLanguage', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + onClick: { + type: 'search', + target: { mode: 'id', id: sourceId }, + // whereLanguage intentionally omitted + }, + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles[0].config.onClick).toMatchObject({ + type: 'search', + target: { mode: 'id', id: sourceId }, + }); + }); + + it('should reject an unknown onClick.type', async () => { + // The schema is a discriminated union on `type`; a bogus type + // must be rejected up front. + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'OnClick bad type', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + onClick: { + type: 'navigate-to-mars', + target: { mode: 'id', id: sourceId }, + whereLanguage: 'sql', + }, + }, + }, + ], + }); + + expect(result.isError).toBe(true); + }); + + it('should validate onClick on update too', async () => { + // Round-trip a tile with no onClick, then update it to add an + // onClick pointing at a non-existent dashboard. The PUT path + // mirrors POST: missing dashboards are rejected with the same + // message. + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + name: 'OnClick update validation', + tiles: [ + { + name: 'Top Services', + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }, + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + + const ghostDashboardId = new mongoose.Types.ObjectId().toString(); + const updateResult = await callTool( + ctx.client!, + 'hyperdx_save_dashboard', + { + id: created.id, + name: 'OnClick update validation', + tiles: [ + { + ...created.tiles[0], + config: { + ...created.tiles[0].config, + onClick: { + type: 'dashboard', + target: { mode: 'id', id: ghostDashboardId }, + whereLanguage: 'sql', + }, + }, + }, + ], + }, + ); + + expect(updateResult.isError).toBe(true); + const text = getFirstText(updateResult); + expect(text).toContain('onClick dashboard'); + expect(text).toContain(ghostDashboardId); + }); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts new file mode 100644 index 0000000000..04bedc87b2 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts @@ -0,0 +1,151 @@ +import mongoose from 'mongoose'; + +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - hyperdx_search_dashboards', () => { + const ctx = setupDashboardTests(); + + it('should find dashboards by name (case-insensitive)', async () => { + await new Dashboard({ + name: 'Service Overview', + tiles: [], + team: ctx.team._id, + tags: [], + }).save(); + await new Dashboard({ + name: 'Error Dashboard', + tiles: [], + team: ctx.team._id, + tags: [], + }).save(); + await new Dashboard({ + name: 'service metrics', + tiles: [], + team: ctx.team._id, + tags: [], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: 'service', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(2); + expect(output.map((d: { name: string }) => d.name).sort()).toEqual([ + 'Service Overview', + 'service metrics', + ]); + }); + + it('should find dashboards by tags', async () => { + await new Dashboard({ + name: 'Dashboard A', + tiles: [], + team: ctx.team._id, + tags: ['production', 'backend'], + }).save(); + await new Dashboard({ + name: 'Dashboard B', + tiles: [], + team: ctx.team._id, + tags: ['production', 'frontend'], + }).save(); + await new Dashboard({ + name: 'Dashboard C', + tiles: [], + team: ctx.team._id, + tags: ['staging'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + tags: ['production'], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(2); + + // Search with multiple tags (AND) + const result2 = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + tags: ['production', 'backend'], + }); + + const output2 = JSON.parse(getFirstText(result2)); + expect(output2).toHaveLength(1); + expect(output2[0].name).toBe('Dashboard A'); + }); + + it('should combine name and tags filters', async () => { + await new Dashboard({ + name: 'API Service', + tiles: [], + team: ctx.team._id, + tags: ['production'], + }).save(); + await new Dashboard({ + name: 'API Errors', + tiles: [], + team: ctx.team._id, + tags: ['staging'], + }).save(); + await new Dashboard({ + name: 'Web Service', + tiles: [], + team: ctx.team._id, + tags: ['production'], + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: 'API', + tags: ['production'], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(1); + expect(output[0].name).toBe('API Service'); + }); + + it('should return empty array when no dashboards match', async () => { + await new Dashboard({ + name: 'Unrelated', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: 'nonexistent-dashboard-name', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(0); + }); + + it('should only return dashboards for the current team', async () => { + await new Dashboard({ + name: 'My Dashboard', + tiles: [], + team: ctx.team._id, + }).save(); + // Create a dashboard for a different team + const otherTeamId = new mongoose.Types.ObjectId(); + await new Dashboard({ + name: 'My Dashboard', + tiles: [], + team: otherTeamId, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: 'My Dashboard', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(1); + }); +}); diff --git a/packages/api/src/mcp/__tests__/dashboards/setup.ts b/packages/api/src/mcp/__tests__/dashboards/setup.ts new file mode 100644 index 0000000000..46f1b3600a --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/setup.ts @@ -0,0 +1,81 @@ +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; + +import * as config from '@/config'; +import { + DEFAULT_DATABASE, + DEFAULT_TRACES_TABLE, + getLoggedInAgent, + getServer, +} from '@/fixtures'; +import Connection from '@/models/connection'; +import { Source } from '@/models/source'; + +import { McpContext } from '../../tools/types'; +import { createTestClient } from '../mcpTestUtils'; + +/** + * Shared setup/teardown for all dashboard MCP tool tests. + * + * Usage: + * const ctx = setupDashboardTests(); + * // Then access ctx.team, ctx.traceSource, ctx.connection, ctx.client + */ +export function setupDashboardTests() { + const server = getServer(); + const ctx = { + server, + team: null as any, + user: null as any, + traceSource: null as any, + connection: null as any, + client: null as Client | null, + }; + + beforeAll(async () => { + await server.start(); + }); + + beforeEach(async () => { + const result = await getLoggedInAgent(server); + ctx.team = result.team; + ctx.user = result.user; + + ctx.connection = await Connection.create({ + team: ctx.team._id, + name: 'Default', + host: config.CLICKHOUSE_HOST, + username: config.CLICKHOUSE_USER, + password: config.CLICKHOUSE_PASSWORD, + }); + + ctx.traceSource = await Source.create({ + kind: SourceKind.Trace, + team: ctx.team._id, + from: { + databaseName: DEFAULT_DATABASE, + tableName: DEFAULT_TRACES_TABLE, + }, + timestampValueExpression: 'Timestamp', + connection: ctx.connection._id, + name: 'Traces', + }); + + const mcpContext: McpContext = { + teamId: ctx.team._id.toString(), + userId: ctx.user._id.toString(), + }; + ctx.client = await createTestClient(mcpContext); + }); + + afterEach(async () => { + await ctx.client?.close(); + await server.clearDBs(); + }); + + afterAll(async () => { + await server.stop(); + }); + + return ctx; +} diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index 28bb45061b..2c2c84be1c 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -821,7 +821,7 @@ Used in the "where" field of select items and search tiles. AND: service.name:api AND http.status_code:>=500 OR: level:error OR level:fatal NOT: NOT level:debug - Wildcard: service.name:front* + Wildcard: ServiceName:front* Phrase: Body:"connection refused" Exists: _exists_:http.route Range (numeric): Duration:>1000000000 @@ -831,16 +831,39 @@ Used in the "where" field of select items and search tiles. NOTE: In Lucene filters, use dot notation for attribute keys (service.name, http.method). This is different from valueExpression and groupBy which require bracket syntax (SpanAttributes['http.method']). -GOTCHA: Lucene queries that reference map-attribute keys via dotted paths (http.status_code, db.system, deployment.environment, anything under SpanAttributes / ResourceAttributes / LogAttributes) are NOT reliably translated to bracket access. Operators (>=, >, <=, <), wildcards (*), AND simple equality all hit translator gaps depending on the column. Symptoms: tile saves fine; hyperdx_query_tile returns an error or returns zero rows when there should be matches; the dashboard renders "Error loading chart" on the tile. Do NOT trust dotted-path Lucene; use SQL with bracket access for ALL map-attribute filtering. +*** SUBSTRING MATCHING (field:value and field:value*) *** + +Lucene field:value does NOT mean exact equality. It translates to ilike(field, '%value%') -- a case-insensitive SUBSTRING match. This has two major consequences: + + 1. Plain value is a substring search: + SpanKind:Server matches "Server", "ServerStreaming", "InternalServer", etc. + SeverityText:error matches "error", "error_timeout", "critical_error", etc. + + 2. Trailing wildcard (field:val*) is prefix-within-substring, NOT a true prefix match: + ServiceName:api* does NOT mean "starts with api". It means "contains a substring starting with api". + In practice this often works for prefix matching, but it is NOT equivalent to SQL LIKE 'api%'. + + For exact matching or true prefix/suffix/contains patterns, use SQL instead: + Exact: whereLanguage: "sql", where: "SpanKind = 'Server'" + Prefix: whereLanguage: "sql", where: "ServiceName LIKE 'api%'" + Contains: whereLanguage: "sql", where: "Body LIKE '%timeout%'" + Regex: whereLanguage: "sql", where: "match(ServiceName, '^api-v[0-9]+')" + + When Lucene substring matching is FINE: free-text columns like Body, StatusMessage where substring is the intent. + When Lucene substring matching is WRONG: enum-like columns (SpanKind, SeverityText, StatusCode, ServiceName) where you want exact or true prefix match. + +*** MAP-ATTRIBUTE GOTCHA *** + +Lucene queries that reference map-attribute keys via dotted paths (http.status_code, db.system, deployment.environment, anything under SpanAttributes / ResourceAttributes / LogAttributes) are NOT reliably translated to bracket access. Operators (>=, >, <=, <), wildcards (*), AND simple equality all hit translator gaps depending on the column. Symptoms: tile saves fine; hyperdx_query_tile returns an error or returns zero rows when there should be matches; the dashboard renders "Error loading chart" on the tile. Do NOT trust dotted-path Lucene; use SQL with bracket access for ALL map-attribute filtering. Unreliable in Lucene (any operation, any operator): http.status_code:>=500 (operator) - http.route:* (wildcard) + http.route:* (wildcard -- also hits the substring matching trap above) db.system:mongodb (simple equality; the translator does NOT map this to SpanAttributes['db.system']) Reliable in Lucene (top-level columns only): Duration:>1000000000 (top-level numeric range) - ServiceName:checkout (top-level string, see fuzzy-match note below for what this actually means) + ServiceName:checkout (top-level string -- but remember this is a substring match, not exact!) Workaround: switch to SQL with bracket access. Map values are stored as strings, so quote the comparison value. @@ -853,11 +876,6 @@ Workaround: switch to SQL with bracket access. Map values are stored as strings, whereLanguage: "sql" where: "SpanAttributes['http.route'] != ''" (filters out empty keys; map values are often empty) -FUZZY-MATCH NOTE: Lucene field:value on a top-level string column translates to ilike(field, '%value%'), not exact equality. That is fine for free-text columns (Body, StatusMessage) but surprising for enum-like columns (SpanKind, SeverityText, StatusCode). SpanKind:Server matches "Server" AND any other string containing "Server" as a substring. For exact-match semantics on top-level columns, use SQL: - - whereLanguage: "sql" - where: "SpanKind = 'Server'" - == SQL FILTER SYNTAX == Alternative to Lucene. Set whereLanguage: "sql" when using SQL syntax. @@ -1235,10 +1253,12 @@ Example: find top patterns for production services over the last 4 hours: Correct: where: "SpanAttributes['db.system'] = 'mongodb'" (whereLanguage: "sql") Lucene parses dotted-path queries happily at save time but the translator does not reliably map them to bracket access. Behavior varies by column and operator. Treat any Lucene filter on a map-attribute key as a save-time bug and use SQL with bracket access for ALL map-attribute filtering. Map values are stored as strings; quote the comparison value. -15. Lucene field:value on enum-like top-level columns (fuzzy substring match, not equality) - Wrong: where: "SpanKind:Server" (whereLanguage: "lucene"; translates to ilike(SpanKind, '%Server%') and matches "Server_Client", "Internal_Server_Error", etc.) - Correct: where: "SpanKind = 'Server'" (whereLanguage: "sql") - Lucene field:value is a substring match (ilike with surrounding wildcards), not exact equality. Fine for Body and StatusMessage; wrong for enum-like columns where exact match matters (SpanKind, SeverityText, StatusCode). When the column has discrete known values, use SQL with =. +15. Lucene field:value and field:value* (substring matching, not exact or true prefix) + Wrong: where: "SpanKind:Server" (whereLanguage: "lucene"; translates to ilike(SpanKind, '%Server%') and matches "ServerStreaming", "InternalServer", etc.) + Wrong: where: "ServiceName:api*" (whereLanguage: "lucene"; NOT a true prefix match -- it is prefix-within-substring) + Correct: where: "SpanKind = 'Server'" (whereLanguage: "sql"; exact match) + Correct: where: "ServiceName LIKE 'api%'" (whereLanguage: "sql"; true prefix match) + Lucene field:value is a substring match (ilike with surrounding wildcards), not exact equality. Lucene field:value* is prefix-within-substring, not SQL LIKE 'value%'. Fine for free-text columns (Body, StatusMessage); wrong for enum-like columns where exact match matters (SpanKind, SeverityText, StatusCode) and wrong when true prefix matching is needed. Use SQL for exact match (=), prefix (LIKE 'x%'), contains (LIKE '%x%'), or regex (match(col, 'pattern')). 16. Forgetting that map-attribute values are often empty strings Wrong: groupBy on SpanAttributes['db.collection.name'] without filtering empty values; the result has an empty-string bucket alongside the real ones, which renders as a blank row in tables and an unlabelled slice in pies. diff --git a/packages/api/src/mcp/tools/dashboards/getDashboardTile.ts b/packages/api/src/mcp/tools/dashboards/getDashboardTile.ts new file mode 100644 index 0000000000..1f4937debf --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/getDashboardTile.ts @@ -0,0 +1,83 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import mongoose from 'mongoose'; +import { z } from 'zod'; + +import Dashboard from '@/models/dashboard'; +import { convertToExternalDashboard } from '@/routers/external-api/v2/utils/dashboards'; + +import { withToolTracing } from '../../utils/tracing'; +import type { McpContext } from '../types'; + +export function registerGetDashboardTile( + server: McpServer, + context: McpContext, +): void { + const { teamId } = context; + + server.registerTool( + 'hyperdx_get_dashboard_tile', + { + title: 'Get a Single Dashboard Tile', + description: + 'Retrieve a single tile from a dashboard by tileId. ' + + 'Useful for inspecting one tile without loading the full dashboard. ' + + 'Use hyperdx_get_dashboard (without an ID) to list dashboards, ' + + 'then hyperdx_get_dashboard (with an ID) to see all tile IDs.', + inputSchema: z.object({ + dashboardId: z.string().describe('Dashboard ID.'), + tileId: z + .string() + .describe( + 'Tile ID within the dashboard. ' + + 'Obtain from hyperdx_get_dashboard.', + ), + }), + }, + withToolTracing( + 'hyperdx_get_dashboard_tile', + context, + async ({ dashboardId, tileId }) => { + if (!mongoose.Types.ObjectId.isValid(dashboardId)) { + return { + isError: true, + content: [{ type: 'text' as const, text: 'Invalid dashboard ID' }], + }; + } + + const dashboard = await Dashboard.findOne({ + _id: dashboardId, + team: teamId, + }); + if (!dashboard) { + return { + isError: true, + content: [{ type: 'text' as const, text: 'Dashboard not found' }], + }; + } + + const externalDashboard = convertToExternalDashboard(dashboard); + const tile = externalDashboard.tiles.find(t => t.id === tileId); + if (!tile) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Tile not found: ${tileId}. Available tile IDs: ${externalDashboard.tiles.map(t => `${t.id} (${t.name})`).join(', ')}`, + }, + ], + }; + } + + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify(tile, null, 2), + }, + ], + }; + }, + ), + ); +} diff --git a/packages/api/src/mcp/tools/dashboards/index.ts b/packages/api/src/mcp/tools/dashboards/index.ts index a1e511e995..f479ae541b 100644 --- a/packages/api/src/mcp/tools/dashboards/index.ts +++ b/packages/api/src/mcp/tools/dashboards/index.ts @@ -3,8 +3,11 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { McpContext, ToolDefinition } from '../types'; import { registerDeleteDashboard } from './deleteDashboard'; import { registerGetDashboard } from './getDashboard'; +import { registerGetDashboardTile } from './getDashboardTile'; +import { registerPatchDashboard } from './patchDashboard'; import { registerQueryTile } from './queryTile'; import { registerSaveDashboard } from './saveDashboard'; +import { registerSearchDashboards } from './searchDashboards'; export * from './schemas'; @@ -13,8 +16,11 @@ const dashboardsTools: ToolDefinition = ( context: McpContext, ) => { registerGetDashboard(server, context); + registerGetDashboardTile(server, context); registerSaveDashboard(server, context); + registerPatchDashboard(server, context); registerDeleteDashboard(server, context); + registerSearchDashboards(server, context); registerQueryTile(server, context); }; diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts new file mode 100644 index 0000000000..39d20cbbc3 --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -0,0 +1,349 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { uniq } from 'lodash'; +import mongoose from 'mongoose'; +import { z } from 'zod'; + +import * as config from '@/config'; +import Dashboard, { IDashboard } from '@/models/dashboard'; +import { + cleanupDashboardAlerts, + collectTileContainerRefIssues, + convertExternalTilesToInternal, + convertToExternalDashboard, + fetchSourcesForValidation, + filterChangedHeatmapTiles, + getHeatmapTilesWithIncompatibleSources, + getInvalidOnClickSearchSources, + getMissingConnections, + getMissingOnClickDashboards, + getMissingSources, +} from '@/routers/external-api/v2/utils/dashboards'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { withToolTracing } from '../../utils/tracing'; +import type { McpContext } from '../types'; +import { mcpPatchTileSchema } from './schemas'; + +export function registerPatchDashboard( + server: McpServer, + context: McpContext, +): void { + const { teamId } = context; + const frontendUrl = config.FRONTEND_URL; + + server.registerTool( + 'hyperdx_patch_dashboard', + { + title: 'Patch Dashboard', + description: + 'Make targeted updates to a dashboard without resubmitting the full object. ' + + 'You can update dashboard-level fields (name, tags) and/or replace a single ' + + 'tile by tileId \u2014 all in one call. Unmentioned tiles and fields are preserved. ' + + 'Use hyperdx_get_dashboard_tile to inspect a tile before patching it. ' + + 'IMPORTANT: After patching a tile, run hyperdx_query_tile to confirm the query still works.', + inputSchema: z.object({ + dashboardId: z.string().describe('Dashboard ID.'), + name: z + .string() + .min(1) + .optional() + .describe('New dashboard name. Omit to keep the current name.'), + tags: z + .array(z.string()) + .optional() + .describe( + 'New tags array (replaces all existing tags). Omit to keep the current tags.', + ), + tileId: z + .string() + .optional() + .describe( + 'ID of the tile to replace. Must be paired with `tile`. ' + + 'Obtain tile IDs from hyperdx_get_dashboard.', + ), + tile: mcpPatchTileSchema + .optional() + .describe( + 'The full replacement tile definition. Replaces the tile matched by tileId. ' + + 'Layout fields (x, y, w, h) and containerId/tabId default to the existing ' + + "tile's values when omitted, so you only need to specify what changed.", + ), + }), + }, + withToolTracing( + 'hyperdx_patch_dashboard', + context, + async ({ dashboardId, name, tags, tileId, tile: inputTile }) => { + // Cross-field validation (kept in handler so the inputSchema + // stays a plain z.object and its properties are visible in the + // JSON Schema that the MCP SDK exposes to LLMs). + if ( + name === undefined && + tags === undefined && + (tileId === undefined || inputTile === undefined) + ) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: 'Provide at least one of: name, tags, or tileId+tile to patch.', + }, + ], + }; + } + if ((tileId === undefined) !== (inputTile === undefined)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: 'tileId and tile must both be provided or both omitted.', + }, + ], + }; + } + + if (!mongoose.Types.ObjectId.isValid(dashboardId)) { + return { + isError: true, + content: [{ type: 'text' as const, text: 'Invalid dashboard ID' }], + }; + } + + const existingDashboard = await Dashboard.findOne({ + _id: dashboardId, + team: teamId, + }); + if (!existingDashboard) { + return { + isError: true, + content: [{ type: 'text' as const, text: 'Dashboard not found' }], + }; + } + + const externalDashboard = convertToExternalDashboard(existingDashboard); + let patchedTiles = externalDashboard.tiles; + let patchedTile: ExternalDashboardTileWithId | undefined; + + // ── Tile-level patch ───────────────────────────────────────── + if (tileId !== undefined && inputTile !== undefined) { + const tileIndex = externalDashboard.tiles.findIndex( + t => t.id === tileId, + ); + if (tileIndex === -1) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Tile not found: ${tileId}. Available tile IDs: ${externalDashboard.tiles.map(t => `${t.id} (${t.name})`).join(', ')}`, + }, + ], + }; + } + + const existingTile = externalDashboard.tiles[tileIndex]; + + // Merge: the incoming tile definition replaces config/name, + // but layout and container refs fall back to the existing tile + // when omitted so the LLM doesn't have to re-specify them. + // Cast once; the Zod schema already validated the shape. + const incoming = inputTile as Partial; + const mergedTile: ExternalDashboardTileWithId = { + ...existingTile, + ...incoming, + id: tileId, // preserve the original tile ID + x: incoming.x ?? existingTile.x, + y: incoming.y ?? existingTile.y, + w: incoming.w ?? existingTile.w, + h: incoming.h ?? existingTile.h, + containerId: + 'containerId' in incoming + ? incoming.containerId + : existingTile.containerId, + tabId: 'tabId' in incoming ? incoming.tabId : existingTile.tabId, + }; + + // Validate the patched tile: sources, connections, heatmap, + // onClick, container refs — same checks as the full update + // path, but scoped to the single changed tile. + const tilesToValidate = [mergedTile]; + + const [ + sources, + missingConnections, + missingOnClickDashboards, + invalidOnClickSearchSources, + ] = await Promise.all([ + fetchSourcesForValidation(teamId), + getMissingConnections(teamId, tilesToValidate), + getMissingOnClickDashboards(teamId, tilesToValidate), + getInvalidOnClickSearchSources(teamId, tilesToValidate), + ]); + + const missingSources = getMissingSources(sources, tilesToValidate); + if (missingSources.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Could not find source IDs: ${missingSources.join(', ')}`, + }, + ], + }; + } + if (missingConnections.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Could not find connection IDs: ${missingConnections.join(', ')}`, + }, + ], + }; + } + + // Heatmap source-kind gate, scoped to tiles whose source or + // displayType changed from the existing version. + const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( + sources, + filterChangedHeatmapTiles( + tilesToValidate, + existingDashboard.tiles ?? [], + ), + ); + if (heatmapNonTraceSources.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`, + }, + ], + }; + } + + if (missingOnClickDashboards.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`, + }, + ], + }; + } + if (invalidOnClickSearchSources.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join(', ')}`, + }, + ], + }; + } + + // Container/tab ref validation against the dashboard's containers. + const effectiveContainers = existingDashboard.containers ?? []; + const tileRefIssues = collectTileContainerRefIssues( + effectiveContainers, + tilesToValidate, + ); + if (tileRefIssues.length > 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Validation error: ${tileRefIssues.join('; ')}`, + }, + ], + }; + } + + // Replace the tile in the full array. + patchedTiles = [...externalDashboard.tiles]; + patchedTiles[tileIndex] = mergedTile; + patchedTile = mergedTile; + } + + // ── Build the $set payload ────────────────────────────────── + const existingTileIds = new Set( + (existingDashboard.tiles ?? []).map((t: { id: string }) => t.id), + ); + const internalTiles = convertExternalTilesToInternal( + patchedTiles, + existingTileIds, + ); + + const setPayload: Partial = {}; + + if (name !== undefined) { + setPayload.name = name; + } + if (tags !== undefined) { + setPayload.tags = uniq(tags); + } + if (tileId !== undefined) { + setPayload.tiles = internalTiles; + } + + const updatedDashboard = await Dashboard.findOneAndUpdate( + { _id: dashboardId, team: teamId }, + { $set: setPayload }, + { new: true }, + ); + + if (!updatedDashboard) { + return { + isError: true, + content: [{ type: 'text' as const, text: 'Dashboard not found' }], + }; + } + + // Clean up alerts for tiles that were removed or converted to + // unsupported config types. + if (tileId !== undefined) { + await cleanupDashboardAlerts({ + dashboardId, + teamId, + internalTiles, + existingTileIds, + }); + } + + // Return a lightweight response: the patched tile (if any) plus + // updated dashboard metadata, without the full tile array. + const output: Record = { + id: updatedDashboard._id.toString(), + name: updatedDashboard.name, + tags: updatedDashboard.tags, + ...(frontendUrl + ? { url: `${frontendUrl}/dashboards/${updatedDashboard._id}` } + : {}), + }; + if (patchedTile) { + output.patchedTile = patchedTile; + output.hint = + 'Use hyperdx_query_tile to test the patched tile query.'; + } + + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify(output, null, 2), + }, + ], + }; + }, + ), + ); +} diff --git a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts index d6bee8eb5c..d5773f1acc 100644 --- a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts @@ -49,7 +49,8 @@ export function registerSaveDashboard( 'Call hyperdx_list_sources first to obtain sourceId and connectionId values. ' + 'IMPORTANT: After saving a dashboard, always run hyperdx_query_tile on each tile ' + 'to confirm the queries work and return expected data. Tiles can silently fail ' + - 'due to incorrect filter syntax, missing attributes, or wrong column names.', + 'due to incorrect filter syntax, missing attributes, or wrong column names. ' + + 'TIP: To update a single tile without resubmitting all tiles, use hyperdx_patch_dashboard instead.', inputSchema: z.object({ id: z .string() diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 77e00a8d73..d6c7035ed0 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -598,7 +598,7 @@ const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ }), }); -const mcpTileSchema = z.union([ +export const mcpTileSchema = z.union([ mcpLineTileSchema, mcpBarTileSchema, mcpTableTileSchema, @@ -610,6 +610,43 @@ const mcpTileSchema = z.union([ mcpSqlTileSchema, ]); +// Layout schema without defaults for the patch tool: layout fields stay +// truly `undefined` when omitted so the merge logic can fall back to the +// existing tile's values instead of Zod filling in 0/12/4. +const mcpPatchTileLayoutSchema = z.object({ + name: z.string().describe('Tile title shown on the dashboard'), + x: z.number().min(0).max(23).optional(), + y: z.number().min(0).optional(), + w: z.number().min(1).max(24).optional(), + h: z.number().min(1).optional(), + id: z.string().max(36).optional(), + containerId: z.string().min(1).max(DASHBOARD_CONTAINER_ID_MAX).optional(), + tabId: z.string().min(1).max(DASHBOARD_CONTAINER_ID_MAX).optional(), +}); + +// Build the patch tile union by extending the default-free layout with +// each tile type's config shape. We only need the `config` field from +// each tile schema; the layout wrapper is replaced. +export const mcpPatchTileSchema = z.union([ + mcpPatchTileLayoutSchema.extend({ config: mcpLineTileSchema.shape.config }), + mcpPatchTileLayoutSchema.extend({ config: mcpBarTileSchema.shape.config }), + mcpPatchTileLayoutSchema.extend({ config: mcpTableTileSchema.shape.config }), + mcpPatchTileLayoutSchema.extend({ + config: mcpNumberTileSchema.shape.config, + }), + mcpPatchTileLayoutSchema.extend({ config: mcpPieTileSchema.shape.config }), + mcpPatchTileLayoutSchema.extend({ + config: mcpHeatmapTileSchema.shape.config, + }), + mcpPatchTileLayoutSchema.extend({ + config: mcpSearchTileSchema.shape.config, + }), + mcpPatchTileLayoutSchema.extend({ + config: mcpMarkdownTileSchema.shape.config, + }), + mcpPatchTileLayoutSchema.extend({ config: mcpSqlTileSchema.shape.config }), +]); + export const mcpTilesParam = z .array(mcpTileSchema) .describe( diff --git a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts new file mode 100644 index 0000000000..c11d9c8b53 --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -0,0 +1,85 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { z } from 'zod'; + +import * as config from '@/config'; +import Dashboard from '@/models/dashboard'; + +import { withToolTracing } from '../../utils/tracing'; +import type { McpContext } from '../types'; + +export function registerSearchDashboards( + server: McpServer, + context: McpContext, +): void { + const { teamId } = context; + const frontendUrl = config.FRONTEND_URL; + + server.registerTool( + 'hyperdx_search_dashboards', + { + title: 'Search Dashboards', + description: + 'Search dashboards by name and/or tags. Returns matching dashboards with ' + + 'their IDs, names, and tags. More targeted than hyperdx_get_dashboard (which ' + + 'lists all dashboards). At least one of query or tags must be provided.', + inputSchema: z.object({ + query: z + .string() + .optional() + .describe( + 'Search term to match against dashboard names (case-insensitive partial match).', + ), + tags: z + .array(z.string()) + .optional() + .describe('Filter to dashboards that have ALL of these tags.'), + }), + }, + withToolTracing( + 'hyperdx_search_dashboards', + context, + async ({ query, tags }) => { + if (query === undefined && tags === undefined) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: 'Provide at least one of: query or tags.', + }, + ], + }; + } + + const filter: Record = { team: teamId }; + + if (query) { + filter.name = { $regex: query, $options: 'i' }; + } + if (tags && tags.length > 0) { + filter.tags = { $all: tags }; + } + + const dashboards = await Dashboard.find(filter) + .select({ name: 1, tags: 1 }) + .lean(); + + const output = dashboards.map(d => ({ + id: d._id.toString(), + name: d.name, + tags: d.tags, + ...(frontendUrl ? { url: `${frontendUrl}/dashboards/${d._id}` } : {}), + })); + + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify(output, null, 2), + }, + ], + }; + }, + ), + ); +} diff --git a/packages/api/src/mcp/tools/query/schemas.ts b/packages/api/src/mcp/tools/query/schemas.ts index 9da11a64b6..c0276b35ff 100644 --- a/packages/api/src/mcp/tools/query/schemas.ts +++ b/packages/api/src/mcp/tools/query/schemas.ts @@ -13,13 +13,18 @@ const WHERE_DESCRIPTION = ' Lucene uses DOT notation: SpanAttributes.http.method:GET\n' + " SQL uses BRACKET notation: SpanAttributes['http.method'] = 'GET'\n\n" + "WRONG: SpanAttributes['key']:value (Lucene cannot parse bracket syntax)\n" + - 'WRONG: level = "error" (SQL syntax with whereLanguage:"lucene")'; + 'WRONG: level = "error" (SQL syntax with whereLanguage:"lucene")\n\n' + + 'SUBSTRING TRAP: Lucene field:value matches ANY row containing "value" as a substring, not exact equality.\n' + + ' SpanKind:Server matches "Server", "ServerStreaming", "InternalServer", etc.\n' + + " For exact match, use SQL: SpanKind = 'Server'"; const WHERE_LANGUAGE_DESCRIPTION = 'Query language for the "where" filter. Default: lucene.\n' + 'Lucene supports comparisons (>= > < <=), wildcards (field:val*), ranges ([1 TO 5]), ' + - 'and map attributes via dot notation. ' + - 'Use "sql" for IN(...) lists, complex expressions, or function calls.'; + 'and map attributes via dot notation. Use "sql" for IN(...) lists, complex expressions, or function calls.\n' + + 'IMPORTANT: Lucene field:value is a SUBSTRING match (ilike), not exact equality. ' + + 'field:val* is prefix-within-substring, not a true prefix match. ' + + "For exact matching or reliable wildcards, use SQL: WHERE field = 'value' or WHERE field LIKE 'val%'."; // ─── Shared Zod schemas ────────────────────────────────────────────────────── From c92b71e0ea18050e5ca0e397eb8215d6d5d5d29b Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 26 May 2026 14:57:05 -0700 Subject: [PATCH 02/11] chore: add changeset for MCP dashboard improvements --- .changeset/mcp-dashboard-improvements.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/mcp-dashboard-improvements.md diff --git a/.changeset/mcp-dashboard-improvements.md b/.changeset/mcp-dashboard-improvements.md new file mode 100644 index 0000000000..b64c102fa7 --- /dev/null +++ b/.changeset/mcp-dashboard-improvements.md @@ -0,0 +1,16 @@ +--- +'@hyperdx/api': patch +--- + +feat(mcp): add patch_dashboard, get_dashboard_tile, search_dashboards tools + +Add three new MCP dashboard tools for granular operations: + +- `hyperdx_get_dashboard_tile` — retrieve a single tile by tileId +- `hyperdx_patch_dashboard` — update name/tags and/or replace one tile + without resubmitting the full dashboard +- `hyperdx_search_dashboards` — search by name and/or tags + +Fix empty parameter schema on patch/search tools caused by Zod +`.refine()` wrapping. Document Lucene substring matching limitations +prominently in tool descriptions and query guide prompt. From d1ebd0aaed189e7b28d0d4fabf05f2d7cb360ea0 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 26 May 2026 14:57:41 -0700 Subject: [PATCH 03/11] chore: unexport mcpTileSchema (only used internally) --- packages/api/src/mcp/tools/dashboards/schemas.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index d6c7035ed0..da41437212 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -598,7 +598,7 @@ const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ }), }); -export const mcpTileSchema = z.union([ +const mcpTileSchema = z.union([ mcpLineTileSchema, mcpBarTileSchema, mcpTableTileSchema, From 96bfb6ac5d2ca31fc37255b75243e12e16b6657d Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 26 May 2026 15:44:35 -0700 Subject: [PATCH 04/11] fix(mcp): harden search and patch dashboard tools searchDashboards: - Escape regex metacharacters in query before passing to MongoDB $regex to prevent injection (e.g. '[' throwing BadValue, '.foo' matching unintended substrings, catastrophic backtracking on patterns like '(a+)+$') - Cap query length at 200 chars via Zod schema - Wrap Dashboard.find in try/catch returning isError on failure patchDashboard: - Work directly with persisted internal tiles array instead of round-tripping all tiles through convertToExternalDashboard (which strips orphaned container refs from unrelated tiles via self-heal logic at utils/dashboards.ts:407-424) - Convert only the single patched tile to internal format via convertToInternalTileConfig - Use positional $set (tiles.) instead of full-array $set so concurrent patches on different tiles don't clobber each other --- .../mcp/tools/dashboards/patchDashboard.ts | 121 ++++++++++-------- .../mcp/tools/dashboards/searchDashboards.ts | 58 ++++++--- 2 files changed, 105 insertions(+), 74 deletions(-) diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index 39d20cbbc3..56b4593701 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -4,12 +4,11 @@ import mongoose from 'mongoose'; import { z } from 'zod'; import * as config from '@/config'; -import Dashboard, { IDashboard } from '@/models/dashboard'; +import Dashboard from '@/models/dashboard'; import { - cleanupDashboardAlerts, collectTileContainerRefIssues, - convertExternalTilesToInternal, convertToExternalDashboard, + convertToInternalTileConfig, fetchSourcesForValidation, filterChangedHeatmapTiles, getHeatmapTilesWithIncompatibleSources, @@ -17,6 +16,7 @@ import { getMissingConnections, getMissingOnClickDashboards, getMissingSources, + isConfigTile, } from '@/routers/external-api/v2/utils/dashboards'; import type { ExternalDashboardTileWithId } from '@/utils/zod'; @@ -122,16 +122,33 @@ export function registerPatchDashboard( }; } - const externalDashboard = convertToExternalDashboard(existingDashboard); - let patchedTiles = externalDashboard.tiles; + // Build the $set payload. Start with metadata-only fields; + // tile patching adds the targeted tile element below. + const setPayload: Record = {}; + + if (name !== undefined) { + setPayload.name = name; + } + if (tags !== undefined) { + setPayload.tags = uniq(tags); + } + let patchedTile: ExternalDashboardTileWithId | undefined; // ── Tile-level patch ───────────────────────────────────────── if (tileId !== undefined && inputTile !== undefined) { - const tileIndex = externalDashboard.tiles.findIndex( - t => t.id === tileId, - ); + // Work directly with the persisted internal tiles array so + // untouched tiles are never round-tripped through the external + // converter (which would strip orphaned container refs from + // unrelated tiles). + const internalTiles = + (existingDashboard.tiles as { id: string }[]) ?? []; + const tileIndex = internalTiles.findIndex(t => t.id === tileId); if (tileIndex === -1) { + // Build a human-readable list of available tile IDs. + // Convert only for the error message (read-only, no write-back). + const externalDashboard = + convertToExternalDashboard(existingDashboard); return { isError: true, content: [ @@ -143,27 +160,40 @@ export function registerPatchDashboard( }; } - const existingTile = externalDashboard.tiles[tileIndex]; + // Read the existing tile's layout and container refs from the + // persisted internal format so fallback values are accurate + // regardless of the external converter's self-healing logic. + const existingInternalTile = internalTiles[tileIndex] as { + id: string; + x?: number; + y?: number; + w?: number; + h?: number; + containerId?: string; + tabId?: string; + config?: { name?: string }; + }; // Merge: the incoming tile definition replaces config/name, // but layout and container refs fall back to the existing tile // when omitted so the LLM doesn't have to re-specify them. - // Cast once; the Zod schema already validated the shape. const incoming = inputTile as Partial; const mergedTile: ExternalDashboardTileWithId = { - ...existingTile, - ...incoming, - id: tileId, // preserve the original tile ID - x: incoming.x ?? existingTile.x, - y: incoming.y ?? existingTile.y, - w: incoming.w ?? existingTile.w, - h: incoming.h ?? existingTile.h, + id: tileId, + name: incoming.name ?? existingInternalTile.config?.name ?? '', + x: incoming.x ?? existingInternalTile.x ?? 0, + y: incoming.y ?? existingInternalTile.y ?? 0, + w: incoming.w ?? existingInternalTile.w ?? 12, + h: incoming.h ?? existingInternalTile.h ?? 4, containerId: 'containerId' in incoming ? incoming.containerId - : existingTile.containerId, - tabId: 'tabId' in incoming ? incoming.tabId : existingTile.tabId, - }; + : existingInternalTile.containerId, + tabId: + 'tabId' in incoming ? incoming.tabId : existingInternalTile.tabId, + // The config comes from the incoming tile (validated by Zod). + config: incoming.config, + } as ExternalDashboardTileWithId; // Validate the patched tile: sources, connections, heatmap, // onClick, container refs — same checks as the full update @@ -268,33 +298,25 @@ export function registerPatchDashboard( }; } - // Replace the tile in the full array. - patchedTiles = [...externalDashboard.tiles]; - patchedTiles[tileIndex] = mergedTile; + // Convert only the patched tile to internal format, then + // target it with a positional $set so concurrent patches on + // different tiles don't clobber each other. + if (!isConfigTile(mergedTile)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: 'Tile must have a config block.', + }, + ], + }; + } + const internalTile = convertToInternalTileConfig(mergedTile); + setPayload[`tiles.${tileIndex}`] = internalTile; patchedTile = mergedTile; } - // ── Build the $set payload ────────────────────────────────── - const existingTileIds = new Set( - (existingDashboard.tiles ?? []).map((t: { id: string }) => t.id), - ); - const internalTiles = convertExternalTilesToInternal( - patchedTiles, - existingTileIds, - ); - - const setPayload: Partial = {}; - - if (name !== undefined) { - setPayload.name = name; - } - if (tags !== undefined) { - setPayload.tags = uniq(tags); - } - if (tileId !== undefined) { - setPayload.tiles = internalTiles; - } - const updatedDashboard = await Dashboard.findOneAndUpdate( { _id: dashboardId, team: teamId }, { $set: setPayload }, @@ -308,17 +330,6 @@ export function registerPatchDashboard( }; } - // Clean up alerts for tiles that were removed or converted to - // unsupported config types. - if (tileId !== undefined) { - await cleanupDashboardAlerts({ - dashboardId, - teamId, - internalTiles, - existingTileIds, - }); - } - // Return a lightweight response: the patched tile (if any) plus // updated dashboard metadata, without the full tile array. const output: Record = { diff --git a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts index c11d9c8b53..bae5efaa63 100644 --- a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -7,6 +7,11 @@ import Dashboard from '@/models/dashboard'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; +/** Escape regex metacharacters so user input is treated as a literal substring. */ +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + export function registerSearchDashboards( server: McpServer, context: McpContext, @@ -25,9 +30,10 @@ export function registerSearchDashboards( inputSchema: z.object({ query: z .string() + .max(200) .optional() .describe( - 'Search term to match against dashboard names (case-insensitive partial match).', + 'Search term to match against dashboard names (case-insensitive substring match).', ), tags: z .array(z.string()) @@ -54,31 +60,45 @@ export function registerSearchDashboards( const filter: Record = { team: teamId }; if (query) { - filter.name = { $regex: query, $options: 'i' }; + filter.name = { $regex: escapeRegExp(query), $options: 'i' }; } if (tags && tags.length > 0) { filter.tags = { $all: tags }; } - const dashboards = await Dashboard.find(filter) - .select({ name: 1, tags: 1 }) - .lean(); + try { + const dashboards = await Dashboard.find(filter) + .select({ name: 1, tags: 1 }) + .lean(); - const output = dashboards.map(d => ({ - id: d._id.toString(), - name: d.name, - tags: d.tags, - ...(frontendUrl ? { url: `${frontendUrl}/dashboards/${d._id}` } : {}), - })); + const output = dashboards.map(d => ({ + id: d._id.toString(), + name: d.name, + tags: d.tags, + ...(frontendUrl + ? { url: `${frontendUrl}/dashboards/${d._id}` } + : {}), + })); - return { - content: [ - { - type: 'text' as const, - text: JSON.stringify(output, null, 2), - }, - ], - }; + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify(output, null, 2), + }, + ], + }; + } catch (err) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Search failed: ${err instanceof Error ? err.message : String(err)}`, + }, + ], + }; + } }, ), ); From 8e822d096590adc31ade3f9e759023634a6fda1c Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 26 May 2026 15:46:46 -0700 Subject: [PATCH 05/11] test(mcp): cover regex escaping and positional tile update searchDashboards: - Regex metacharacters treated as literals (parentheses, brackets, dots) - Invalid regex chars like '[' don't throw patchDashboard: - Positional update verified via raw DB read: untouched tiles are byte-identical before and after patching a sibling tile --- .../dashboards/patchDashboard.test.ts | 83 +++++++++++++++++++ .../dashboards/searchDashboards.test.ts | 66 +++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts index 1e72c98b09..ce6cb0d715 100644 --- a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts @@ -270,6 +270,89 @@ describe('MCP Dashboard Tools - hyperdx_patch_dashboard', () => { expect(getFirstText(result)).toContain('source'); }); + it('should not modify other tiles in the database (positional update)', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Positional Update Test', + tiles: [ + { + name: 'Tile A', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Tile B', + x: 6, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Tile C', + x: 0, + y: 3, + w: 12, + h: 4, + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const dashboardId = created.id; + + // Snapshot the raw tiles from the DB before patching + const beforePatch = await Dashboard.findById(dashboardId).lean(); + const tilesBeforePatch = (beforePatch as any).tiles; + const tileBBefore = tilesBeforePatch[1]; + const tileCBefore = tilesBeforePatch[2]; + + // Patch only tile A (index 0) + const patchResult = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId, + tileId: created.tiles[0].id, + tile: { + name: 'Tile A Patched', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + }); + expect(patchResult.isError).toBeFalsy(); + + // Read raw DB tiles again — tiles B and C should be byte-identical + const afterPatch = await Dashboard.findById(dashboardId).lean(); + const tilesAfterPatch = (afterPatch as any).tiles; + + expect(tilesAfterPatch).toHaveLength(3); + // Tile B (index 1) and Tile C (index 2) should be untouched + expect(JSON.stringify(tilesAfterPatch[1])).toBe( + JSON.stringify(tileBBefore), + ); + expect(JSON.stringify(tilesAfterPatch[2])).toBe( + JSON.stringify(tileCBefore), + ); + // Tile A (index 0) should have changed + expect(tilesAfterPatch[0].config.name).toBe('Tile A Patched'); + }); + it('should round-trip: patch then get_dashboard_tile', async () => { const sourceId = ctx.traceSource._id.toString(); const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { diff --git a/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts index 04bedc87b2..ba90e33671 100644 --- a/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts @@ -126,6 +126,72 @@ describe('MCP Dashboard Tools - hyperdx_search_dashboards', () => { expect(output).toHaveLength(0); }); + it('should treat regex metacharacters as literal substrings', async () => { + await new Dashboard({ + name: 'API (v2) Service', + tiles: [], + team: ctx.team._id, + }).save(); + await new Dashboard({ + name: 'API v2 Service', + tiles: [], + team: ctx.team._id, + }).save(); + + // Parentheses are regex metacharacters — without escaping, "(v2)" + // would be treated as a capture group and match "v2" anywhere. + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: '(v2)', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + // Only the dashboard with literal "(v2)" should match + expect(output).toHaveLength(1); + expect(output[0].name).toBe('API (v2) Service'); + }); + + it('should not throw on regex-invalid characters like [', async () => { + await new Dashboard({ + name: 'Test Dashboard', + tiles: [], + team: ctx.team._id, + }).save(); + + // An unescaped "[" would throw a MongoDB BadValue error + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: '[invalid', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(0); + }); + + it('should match literal dots, not arbitrary characters', async () => { + await new Dashboard({ + name: 'api.v2.service', + tiles: [], + team: ctx.team._id, + }).save(); + await new Dashboard({ + name: 'apiXv2Xservice', + tiles: [], + team: ctx.team._id, + }).save(); + + // Unescaped "." in regex matches any character, so ".v2." would + // match "Xv2X". With escaping, only the literal dot matches. + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: '.v2.', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output).toHaveLength(1); + expect(output[0].name).toBe('api.v2.service'); + }); + it('should only return dashboards for the current team', async () => { await new Dashboard({ name: 'My Dashboard', From 232365ee357c997ffedf62f3ef555630259b1e5f Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 27 May 2026 08:41:45 -0700 Subject: [PATCH 06/11] fix(mcp): address review findings on patch and search tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 — patchDashboard concurrent write race: - Replace `tiles.${tileIndex}` positional set with `tiles.id` in the query filter + `tiles.$` positional operator. A concurrent save_dashboard that replaces the tiles array can no longer cause us to overwrite an unrelated tile at a stale numeric index. - null result from findOneAndUpdate now returns a specific error message indicating the tile may have been removed concurrently. P2 — patchDashboard missing alert cleanup: - Restore cleanupDashboardAlerts call after tile update, scoped to the single patched tile. Catches displayType changes to configs that don't support alerts. P2 — patch tile name required when it should be optional: - Make `name` optional on mcpPatchTileLayoutSchema so the LLM can send a config-only patch without re-specifying the tile title. The merge fallback (incoming.name ?? existing name) is now reachable. - Add .min(1) to both mcpTileLayoutSchema.name and mcpPatchTileLayoutSchema.name to reject blank tile titles at the schema boundary. P2 — searchDashboards empty query/tags bypass: - Replace `query === undefined` guard with length checks so `query: ''` and `tags: []` are rejected instead of silently returning all dashboards. --- .../mcp/tools/dashboards/patchDashboard.ts | 70 +++++++++++++++---- .../api/src/mcp/tools/dashboards/schemas.ts | 8 ++- .../mcp/tools/dashboards/searchDashboards.ts | 13 ++-- 3 files changed, 69 insertions(+), 22 deletions(-) diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index 56b4593701..f37953e6a7 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -6,6 +6,7 @@ import { z } from 'zod'; import * as config from '@/config'; import Dashboard from '@/models/dashboard'; import { + cleanupDashboardAlerts, collectTileContainerRefIssues, convertToExternalDashboard, convertToInternalTileConfig, @@ -65,8 +66,8 @@ export function registerPatchDashboard( .optional() .describe( 'The full replacement tile definition. Replaces the tile matched by tileId. ' + - 'Layout fields (x, y, w, h) and containerId/tabId default to the existing ' + - "tile's values when omitted, so you only need to specify what changed.", + 'Layout fields (x, y, w, h), name, and containerId/tabId default to the ' + + "existing tile's values when omitted, so you only need to specify what changed.", ), }), }, @@ -122,9 +123,14 @@ export function registerPatchDashboard( }; } - // Build the $set payload. Start with metadata-only fields; - // tile patching adds the targeted tile element below. + // Build the $set payload and the query filter. Metadata fields + // are simple top-level $set entries; the tile patch uses the + // positional $ operator matched by 'tiles.id' in the filter. const setPayload: Record = {}; + const queryFilter: Record = { + _id: dashboardId, + team: teamId, + }; if (name !== undefined) { setPayload.name = name; @@ -143,8 +149,8 @@ export function registerPatchDashboard( // unrelated tiles). const internalTiles = (existingDashboard.tiles as { id: string }[]) ?? []; - const tileIndex = internalTiles.findIndex(t => t.id === tileId); - if (tileIndex === -1) { + const existingIdx = internalTiles.findIndex(t => t.id === tileId); + if (existingIdx === -1) { // Build a human-readable list of available tile IDs. // Convert only for the error message (read-only, no write-back). const externalDashboard = @@ -163,7 +169,7 @@ export function registerPatchDashboard( // Read the existing tile's layout and container refs from the // persisted internal format so fallback values are accurate // regardless of the external converter's self-healing logic. - const existingInternalTile = internalTiles[tileIndex] as { + const existingInternalTile = internalTiles[existingIdx] as { id: string; x?: number; y?: number; @@ -236,8 +242,6 @@ export function registerPatchDashboard( }; } - // Heatmap source-kind gate, scoped to tiles whose source or - // displayType changed from the existing version. const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( sources, filterChangedHeatmapTiles( @@ -280,7 +284,6 @@ export function registerPatchDashboard( }; } - // Container/tab ref validation against the dashboard's containers. const effectiveContainers = existingDashboard.containers ?? []; const tileRefIssues = collectTileContainerRefIssues( effectiveContainers, @@ -298,9 +301,7 @@ export function registerPatchDashboard( }; } - // Convert only the patched tile to internal format, then - // target it with a positional $set so concurrent patches on - // different tiles don't clobber each other. + // Convert only the patched tile to internal format. if (!isConfigTile(mergedTile)) { return { isError: true, @@ -313,23 +314,62 @@ export function registerPatchDashboard( }; } const internalTile = convertToInternalTileConfig(mergedTile); - setPayload[`tiles.${tileIndex}`] = internalTile; + + // Use the positional $ operator matched by 'tiles.id' in the + // query filter. This targets the tile by its id field rather + // than a captured numeric index, so a concurrent save_dashboard + // that replaces the whole tiles array can't cause us to + // overwrite an unrelated tile at a stale index. + queryFilter['tiles.id'] = tileId; + setPayload['tiles.$'] = internalTile; patchedTile = mergedTile; } const updatedDashboard = await Dashboard.findOneAndUpdate( - { _id: dashboardId, team: teamId }, + queryFilter, { $set: setPayload }, { new: true }, ); if (!updatedDashboard) { + // When a tile patch is in flight, a null result means the tile + // was removed or the dashboard was deleted between our read + // and this write. + if (tileId !== undefined) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Tile ${tileId} was not found at write time (it may have been removed by a concurrent update).`, + }, + ], + }; + } return { isError: true, content: [{ type: 'text' as const, text: 'Dashboard not found' }], }; } + // Reconcile alerts: if the tile's displayType changed to one + // that doesn't support alerts (e.g. raw SQL line/pie), clean up + // stale alert documents. Scope to just the patched tile — pass + // it as both the "new" tiles and "existing" ids so the helper + // checks whether the updated config still supports alerts. + if (tileId !== undefined) { + const existingTileIds = new Set([tileId]); + const patchedTileInDb = updatedDashboard.tiles.filter( + t => t.id === tileId, + ); + await cleanupDashboardAlerts({ + dashboardId, + teamId, + internalTiles: patchedTileInDb, + existingTileIds, + }); + } + // Return a lightweight response: the patched tile (if any) plus // updated dashboard metadata, without the full tile array. const output: Record = { diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index da41437212..1957841bb6 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -309,7 +309,7 @@ const mcpOnClickSchema = z ); const mcpTileLayoutSchema = z.object({ - name: z.string().describe('Tile title shown on the dashboard'), + name: z.string().min(1).describe('Tile title shown on the dashboard'), x: z .number() .min(0) @@ -614,7 +614,11 @@ const mcpTileSchema = z.union([ // truly `undefined` when omitted so the merge logic can fall back to the // existing tile's values instead of Zod filling in 0/12/4. const mcpPatchTileLayoutSchema = z.object({ - name: z.string().describe('Tile title shown on the dashboard'), + name: z + .string() + .min(1) + .optional() + .describe('Tile title. Omit to keep the existing title.'), x: z.number().min(0).max(23).optional(), y: z.number().min(0).optional(), w: z.number().min(1).max(24).optional(), diff --git a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts index bae5efaa63..e5ae3f368a 100644 --- a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -45,13 +45,16 @@ export function registerSearchDashboards( 'hyperdx_search_dashboards', context, async ({ query, tags }) => { - if (query === undefined && tags === undefined) { + const hasQuery = typeof query === 'string' && query.length > 0; + const hasTags = Array.isArray(tags) && tags.length > 0; + + if (!hasQuery && !hasTags) { return { isError: true, content: [ { type: 'text' as const, - text: 'Provide at least one of: query or tags.', + text: 'Provide at least one of: query (non-empty string) or tags (non-empty array).', }, ], }; @@ -59,10 +62,10 @@ export function registerSearchDashboards( const filter: Record = { team: teamId }; - if (query) { - filter.name = { $regex: escapeRegExp(query), $options: 'i' }; + if (hasQuery) { + filter.name = { $regex: escapeRegExp(query!), $options: 'i' }; } - if (tags && tags.length > 0) { + if (hasTags) { filter.tags = { $all: tags }; } From fbd29468fb45144cb8d715d148c56a7bc452e842 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 27 May 2026 09:32:24 -0700 Subject: [PATCH 07/11] fix(mcp): address P2/P3 review findings searchDashboards: - Use lodash escapeRegExp instead of reimplemented helper - Inline type narrowing to eliminate query! non-null assertion - Tighten tags schema to z.string().min(1) to reject tags: [''] - Add .limit(100) with truncation hint when results exceed cap - Log errors before returning user-facing message patchDashboard: - Coerce legacy empty-string containerId/tabId to undefined in merge (mirrors convertTileToExternalChart self-heal for legacy docs) - Expand concurrent-miss error to state name/tags were also discarded schemas: - Drop unused id field from mcpPatchTileLayoutSchema (tileId is the authoritative selector; inner id was silently overridden) tests: - Switch JSON.stringify byte-equality to toEqual (order-insensitive) - Add tests: config-only patch preserves name, concurrent tile removal detected, empty query/tags/both rejected changeset: - Note .min(1) contract tightening on tile name --- .changeset/mcp-dashboard-improvements.md | 5 + .../dashboards/patchDashboard.test.ts | 107 +++++++++++++++++- .../dashboards/searchDashboards.test.ts | 40 +++++++ .../mcp/tools/dashboards/patchDashboard.ts | 15 ++- .../api/src/mcp/tools/dashboards/schemas.ts | 1 - .../mcp/tools/dashboards/searchDashboards.ts | 35 ++++-- 6 files changed, 184 insertions(+), 19 deletions(-) diff --git a/.changeset/mcp-dashboard-improvements.md b/.changeset/mcp-dashboard-improvements.md index b64c102fa7..375224e649 100644 --- a/.changeset/mcp-dashboard-improvements.md +++ b/.changeset/mcp-dashboard-improvements.md @@ -14,3 +14,8 @@ Add three new MCP dashboard tools for granular operations: Fix empty parameter schema on patch/search tools caused by Zod `.refine()` wrapping. Document Lucene substring matching limitations prominently in tool descriptions and query guide prompt. + +**Breaking (minor):** Tile `name` on `hyperdx_save_dashboard` now requires +at least 1 character (`.min(1)`). Previously empty string `""` was accepted +and silently persisted as a blank title. Callers sending `name: ""` will +now receive a validation error. diff --git a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts index ce6cb0d715..ec2162cb6d 100644 --- a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts @@ -343,16 +343,111 @@ describe('MCP Dashboard Tools - hyperdx_patch_dashboard', () => { expect(tilesAfterPatch).toHaveLength(3); // Tile B (index 1) and Tile C (index 2) should be untouched - expect(JSON.stringify(tilesAfterPatch[1])).toBe( - JSON.stringify(tileBBefore), - ); - expect(JSON.stringify(tilesAfterPatch[2])).toBe( - JSON.stringify(tileCBefore), - ); + expect(tilesAfterPatch[1]).toEqual(tileBBefore); + expect(tilesAfterPatch[2]).toEqual(tileCBefore); // Tile A (index 0) should have changed expect(tilesAfterPatch[0].config.name).toBe('Tile A Patched'); }); + it('should preserve tile name when omitted from patch (config-only patch)', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Name Preservation Test', + tiles: [ + { + name: 'Original Title', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const tileId = created.tiles[0].id; + + // Patch config only — no name field at all + const patchResult = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + tileId, + tile: { + config: { + displayType: 'table', + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + }); + + expect(patchResult.isError).toBeFalsy(); + const patched = JSON.parse(getFirstText(patchResult)); + expect(patched.patchedTile.name).toBe('Original Title'); + expect(patched.patchedTile.config.displayType).toBe('table'); + + // Verify via get + const getResult = await callTool( + ctx.client!, + 'hyperdx_get_dashboard_tile', + { dashboardId: created.id, tileId }, + ); + const tile = JSON.parse(getFirstText(getResult)); + expect(tile.name).toBe('Original Title'); + }); + + it('should return error when tile is removed between read and write', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { + name: 'Concurrent Delete Test', + tiles: [ + { + name: 'Tile A', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + { + name: 'Tile B', + config: { + displayType: 'number', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }); + const created = JSON.parse(getFirstText(createResult)); + const tileAId = created.tiles[0].id; + + // Simulate a concurrent save_dashboard that removes tile A by + // directly updating the DB to drop it from the array. + await Dashboard.findByIdAndUpdate(created.id, { + $pull: { tiles: { id: tileAId } }, + }); + + // Now try to patch tile A — it no longer exists in the array. + const patchResult = await callTool(ctx.client!, 'hyperdx_patch_dashboard', { + dashboardId: created.id, + tileId: tileAId, + tile: { + name: 'Patched A', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + }); + + expect(patchResult.isError).toBe(true); + // Could be "Tile not found" (caught at read) or "was not found at + // write time" (caught by positional $ miss). Either is acceptable. + const text = getFirstText(patchResult); + expect(text).toMatch(/not found|was not found/i); + }); + it('should round-trip: patch then get_dashboard_tile', async () => { const sourceId = ctx.traceSource._id.toString(); const createResult = await callTool(ctx.client!, 'hyperdx_save_dashboard', { diff --git a/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts index ba90e33671..765ab80b4d 100644 --- a/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts @@ -192,6 +192,46 @@ describe('MCP Dashboard Tools - hyperdx_search_dashboards', () => { expect(output[0].name).toBe('api.v2.service'); }); + it('should reject empty query string', async () => { + await new Dashboard({ + name: 'Should Not Appear', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: '', + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('at least one'); + }); + + it('should reject empty tags array', async () => { + await new Dashboard({ + name: 'Should Not Appear', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + tags: [], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('at least one'); + }); + + it('should reject empty query with empty tags', async () => { + const result = await callTool(ctx.client!, 'hyperdx_search_dashboards', { + query: '', + tags: [], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('at least one'); + }); + it('should only return dashboards for the current team', async () => { await new Dashboard({ name: 'My Dashboard', diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index f37953e6a7..636212ea2d 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -183,7 +183,13 @@ export function registerPatchDashboard( // Merge: the incoming tile definition replaces config/name, // but layout and container refs fall back to the existing tile // when omitted so the LLM doesn't have to re-specify them. + // Coerce legacy empty-string containerId/tabId to undefined so + // they don't trip the container-ref validator (mirrors the + // self-heal in convertTileToExternalChart). const incoming = inputTile as Partial; + const existingContainerId = + existingInternalTile.containerId || undefined; + const existingTabId = existingInternalTile.tabId || undefined; const mergedTile: ExternalDashboardTileWithId = { id: tileId, name: incoming.name ?? existingInternalTile.config?.name ?? '', @@ -194,9 +200,8 @@ export function registerPatchDashboard( containerId: 'containerId' in incoming ? incoming.containerId - : existingInternalTile.containerId, - tabId: - 'tabId' in incoming ? incoming.tabId : existingInternalTile.tabId, + : existingContainerId, + tabId: 'tabId' in incoming ? incoming.tabId : existingTabId, // The config comes from the incoming tile (validated by Zod). config: incoming.config, } as ExternalDashboardTileWithId; @@ -341,7 +346,9 @@ export function registerPatchDashboard( content: [ { type: 'text' as const, - text: `Tile ${tileId} was not found at write time (it may have been removed by a concurrent update).`, + text: + `Tile ${tileId} was not found at write time (it may have been removed by a concurrent update). ` + + 'The entire update was rejected — name/tags changes (if any) were not applied. Resubmit.', }, ], }; diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 1957841bb6..ac21685366 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -623,7 +623,6 @@ const mcpPatchTileLayoutSchema = z.object({ y: z.number().min(0).optional(), w: z.number().min(1).max(24).optional(), h: z.number().min(1).optional(), - id: z.string().max(36).optional(), containerId: z.string().min(1).max(DASHBOARD_CONTAINER_ID_MAX).optional(), tabId: z.string().min(1).max(DASHBOARD_CONTAINER_ID_MAX).optional(), }); diff --git a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts index e5ae3f368a..d1934dd131 100644 --- a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -1,16 +1,15 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { escapeRegExp } from 'lodash'; import { z } from 'zod'; import * as config from '@/config'; import Dashboard from '@/models/dashboard'; +import logger from '@/utils/logger'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; -/** Escape regex metacharacters so user input is treated as a literal substring. */ -function escapeRegExp(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} +const SEARCH_RESULTS_LIMIT = 100; export function registerSearchDashboards( server: McpServer, @@ -36,7 +35,7 @@ export function registerSearchDashboards( 'Search term to match against dashboard names (case-insensitive substring match).', ), tags: z - .array(z.string()) + .array(z.string().min(1)) .optional() .describe('Filter to dashboards that have ALL of these tags.'), }), @@ -63,7 +62,7 @@ export function registerSearchDashboards( const filter: Record = { team: teamId }; if (hasQuery) { - filter.name = { $regex: escapeRegExp(query!), $options: 'i' }; + filter.name = { $regex: escapeRegExp(query), $options: 'i' }; } if (hasTags) { filter.tags = { $all: tags }; @@ -72,9 +71,15 @@ export function registerSearchDashboards( try { const dashboards = await Dashboard.find(filter) .select({ name: 1, tags: 1 }) + .limit(SEARCH_RESULTS_LIMIT + 1) .lean(); - const output = dashboards.map(d => ({ + const truncated = dashboards.length > SEARCH_RESULTS_LIMIT; + const results = truncated + ? dashboards.slice(0, SEARCH_RESULTS_LIMIT) + : dashboards; + + const output = results.map(d => ({ id: d._id.toString(), name: d.name, tags: d.tags, @@ -87,11 +92,25 @@ export function registerSearchDashboards( content: [ { type: 'text' as const, - text: JSON.stringify(output, null, 2), + text: JSON.stringify( + truncated + ? { + results: output, + truncated: true, + hint: `Returned first ${SEARCH_RESULTS_LIMIT} results. Narrow your query or tags to see more.`, + } + : output, + null, + 2, + ), }, ], }; } catch (err) { + logger.error( + { err, teamId, query, tags }, + 'hyperdx_search_dashboards: query failed', + ); return { isError: true, content: [ From d4eb7c9154fc04d783f494b7816a85e8d7df66b0 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 27 May 2026 09:48:55 -0700 Subject: [PATCH 08/11] refactor: extract validateDashboardTiles, fix builder-tile alert gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract shared validateDashboardTiles() helper in utils/dashboards.ts that consolidates the ~95-line tile validation block previously duplicated across 5 call sites (REST v2 POST/PUT, MCP save create/ update, MCP patch). Net -355 lines. The helper runs all 7 checks in order: container refs, missing sources, missing connections, source/connection mismatches (was REST-only, now shared with MCP), heatmap source-kind gate, onClick dashboard targets, onClick search source validation. Also move getSourceConnectionMismatches from a private function in the REST router to a shared export in utils — MCP callers now get the same source/connection mismatch validation that was previously REST-only. Fix cleanupDashboardAlerts to also catch builder tiles with alert-incompatible display types (Pie, Table, Heatmap, Search, Markdown). Previously only raw SQL tiles with incompatible types were cleaned up, leaving stale alert documents on builder tiles that changed to an unsupported displayType. --- .../mcp/tools/dashboards/patchDashboard.ts | 113 +------- .../src/mcp/tools/dashboards/saveDashboard.ts | 234 ++-------------- .../src/routers/external-api/v2/dashboards.ts | 252 ++---------------- .../external-api/v2/utils/dashboards.ts | 140 +++++++++- 4 files changed, 192 insertions(+), 547 deletions(-) diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index 636212ea2d..1061208760 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -7,17 +7,10 @@ import * as config from '@/config'; import Dashboard from '@/models/dashboard'; import { cleanupDashboardAlerts, - collectTileContainerRefIssues, convertToExternalDashboard, convertToInternalTileConfig, - fetchSourcesForValidation, - filterChangedHeatmapTiles, - getHeatmapTilesWithIncompatibleSources, - getInvalidOnClickSearchSources, - getMissingConnections, - getMissingOnClickDashboards, - getMissingSources, isConfigTile, + validateDashboardTiles, } from '@/routers/external-api/v2/utils/dashboards'; import type { ExternalDashboardTileWithId } from '@/utils/zod'; @@ -206,103 +199,17 @@ export function registerPatchDashboard( config: incoming.config, } as ExternalDashboardTileWithId; - // Validate the patched tile: sources, connections, heatmap, - // onClick, container refs — same checks as the full update - // path, but scoped to the single changed tile. - const tilesToValidate = [mergedTile]; - - const [ - sources, - missingConnections, - missingOnClickDashboards, - invalidOnClickSearchSources, - ] = await Promise.all([ - fetchSourcesForValidation(teamId), - getMissingConnections(teamId, tilesToValidate), - getMissingOnClickDashboards(teamId, tilesToValidate), - getInvalidOnClickSearchSources(teamId, tilesToValidate), - ]); - - const missingSources = getMissingSources(sources, tilesToValidate); - if (missingSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find source IDs: ${missingSources.join(', ')}`, - }, - ], - }; - } - if (missingConnections.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find connection IDs: ${missingConnections.join(', ')}`, - }, - ], - }; - } - - const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( - sources, - filterChangedHeatmapTiles( - tilesToValidate, - existingDashboard.tiles ?? [], - ), - ); - if (heatmapNonTraceSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`, - }, - ], - }; - } - - if (missingOnClickDashboards.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`, - }, - ], - }; - } - if (invalidOnClickSearchSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join(', ')}`, - }, - ], - }; - } - - const effectiveContainers = existingDashboard.containers ?? []; - const tileRefIssues = collectTileContainerRefIssues( - effectiveContainers, - tilesToValidate, - ); - if (tileRefIssues.length > 0) { + // Validate the patched tile using the shared validation helper. + const validationError = await validateDashboardTiles({ + teamId, + tiles: [mergedTile], + existingTiles: existingDashboard.tiles ?? [], + containers: existingDashboard.containers ?? [], + }); + if (validationError) { return { isError: true, - content: [ - { - type: 'text' as const, - text: `Validation error: ${tileRefIssues.join('; ')}`, - }, - ], + content: [{ type: 'text' as const, text: validationError }], }; } diff --git a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts index d5773f1acc..146eff3f56 100644 --- a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts @@ -8,20 +8,13 @@ import * as config from '@/config'; import Dashboard, { IDashboard } from '@/models/dashboard'; import { cleanupDashboardAlerts, - collectTileContainerRefIssues, convertExternalFiltersToInternal, convertExternalTilesToInternal, convertToExternalDashboard, createDashboardBodySchema, - fetchSourcesForValidation, - filterChangedHeatmapTiles, - getHeatmapTilesWithIncompatibleSources, - getInvalidOnClickSearchSources, - getMissingConnections, - getMissingOnClickDashboards, - getMissingSources, resolveSavedQueryLanguage, updateDashboardBodySchema, + validateDashboardTiles, } from '@/routers/external-api/v2/utils/dashboards'; import type { ExternalDashboardFilter, @@ -181,105 +174,16 @@ async function createDashboard({ const { tiles, filters, containers: parsedContainers } = parsed.data; const tilesWithId = tiles as ExternalDashboardTileWithId[]; - // Mirror the v2 router POST handler: structural container checks ran - // through the body schema; per-tile containerId/tabId resolution runs - // against the request body's containers (no existing dashboard to fall - // back to on create). - const tileRefIssues = collectTileContainerRefIssues( - parsedContainers ?? [], - tilesWithId, - ); - if (tileRefIssues.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Validation error: ${tileRefIssues.join('; ')}`, - }, - ], - }; - } - - // Hoist the source fetch so missing-source and heatmap-source-kind - // checks share a single DB round-trip, mirroring the REST POST path. - const [ - sources, - missingConnections, - missingOnClickDashboards, - invalidOnClickSearchSources, - ] = await Promise.all([ - fetchSourcesForValidation(teamId), - getMissingConnections(teamId, tilesWithId), - getMissingOnClickDashboards(teamId, tilesWithId), - getInvalidOnClickSearchSources(teamId, tilesWithId), - ]); - - const missingSources = getMissingSources(sources, tilesWithId, filters); - if (missingSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find source IDs: ${missingSources.join(', ')}`, - }, - ], - }; - } - if (missingConnections.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find connection IDs: ${missingConnections.join(', ')}`, - }, - ], - }; - } - - // Source-kind gate for heatmap tiles. Mirrors the REST POST path so - // an MCP-issued create cannot persist a heatmap that the REST PUT - // would reject with 400 on the next round-trip. - const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( - sources, - tilesWithId, - ); - if (heatmapNonTraceSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`, - }, - ], - }; - } - - // Validate that a table tile's row-click will not land on a - // missing dashboard or a non-log/trace source. - if (missingOnClickDashboards.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`, - }, - ], - }; - } - if (invalidOnClickSearchSources.length > 0) { + const validationError = await validateDashboardTiles({ + teamId, + tiles: tilesWithId, + filters, + containers: parsedContainers ?? [], + }); + if (validationError) { return { isError: true, - content: [ - { - type: 'text' as const, - text: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join(', ')}`, - }, - ], + content: [{ type: 'text' as const, text: validationError }], }; } @@ -374,53 +278,10 @@ async function updateDashboard({ const { tiles, filters, containers: parsedContainers } = parsed.data; const tilesWithId = tiles as ExternalDashboardTileWithId[]; - // Hoist sources, connections, and existing-dashboard fetches into a - // single Promise.all so the source list is shared across helpers and - // the heatmap source-kind check can scope itself to tiles whose - // sourceId/displayType actually changed in this request, matching - // the REST PUT path. `containers` is in the projection so the - // container/tab ref check can fall back to the persisted containers - // when the payload omits them. - const [ - sources, - missingConnections, - missingOnClickDashboards, - invalidOnClickSearchSources, - existingDashboard, - ] = await Promise.all([ - fetchSourcesForValidation(teamId), - getMissingConnections(teamId, tilesWithId), - getMissingOnClickDashboards(teamId, tilesWithId), - getInvalidOnClickSearchSources(teamId, tilesWithId), - Dashboard.findOne( - { _id: dashboardId, team: teamId }, - { tiles: 1, filters: 1, containers: 1 }, - ).lean(), - ]); - - const missingSources = getMissingSources(sources, tilesWithId, filters); - if (missingSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find source IDs: ${missingSources.join(', ')}`, - }, - ], - }; - } - if (missingConnections.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find connection IDs: ${missingConnections.join(', ')}`, - }, - ], - }; - } + const existingDashboard = await Dashboard.findOne( + { _id: dashboardId, team: teamId }, + { tiles: 1, filters: 1, containers: 1 }, + ).lean(); if (!existingDashboard) { return { @@ -429,70 +290,19 @@ async function updateDashboard({ }; } - // Mirror the v2 router PUT handler: resolve tile container/tab refs - // against an effective container set so a payload that omits the - // `containers` field falls back to the persisted dashboard rather - // than an empty fallback. Otherwise a tile pointing at a preserved - // container would be rejected with "Tile references unknown - // containerId". const effectiveContainers = parsedContainers ?? existingDashboard.containers ?? []; - const tileRefIssues = collectTileContainerRefIssues( - effectiveContainers, - tilesWithId, - ); - if (tileRefIssues.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Validation error: ${tileRefIssues.join('; ')}`, - }, - ], - }; - } - - // Source-kind gate, scoped to heatmap tiles whose source or - // displayType changed in this request. Mirrors the REST PUT path. - const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( - sources, - filterChangedHeatmapTiles(tilesWithId, existingDashboard.tiles ?? []), - ); - if (heatmapNonTraceSources.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`, - }, - ], - }; - } - - // Validate that a table tile's row-click will not land on a - // missing dashboard or a non-log/trace source. - if (missingOnClickDashboards.length > 0) { - return { - isError: true, - content: [ - { - type: 'text' as const, - text: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`, - }, - ], - }; - } - if (invalidOnClickSearchSources.length > 0) { + const validationError = await validateDashboardTiles({ + teamId, + tiles: tilesWithId, + filters, + existingTiles: existingDashboard.tiles ?? [], + containers: effectiveContainers, + }); + if (validationError) { return { isError: true, - content: [ - { - type: 'text' as const, - text: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join(', ')}`, - }, - ], + content: [{ type: 'text' as const, text: validationError }], }; } diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index 3f73418553..4616d9abf0 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -9,23 +9,13 @@ import { ExternalDashboardTileWithId, objectIdSchema } from '@/utils/zod'; import { cleanupDashboardAlerts, - collectTileContainerRefIssues, convertExternalFiltersToInternal, convertExternalTilesToInternal, convertToExternalDashboard, createDashboardBodySchema, - fetchSourcesForValidation, - filterChangedHeatmapTiles, - getHeatmapTilesWithIncompatibleSources, - getInvalidOnClickSearchSources, - getMissingConnections, - getMissingOnClickDashboards, - getMissingSources, - isConfigTile, - isRawSqlExternalTileConfig, resolveSavedQueryLanguage, - SourceForValidation, updateDashboardBodySchema, + validateDashboardTiles, } from './utils/dashboards'; /** @@ -45,29 +35,6 @@ const EXTERNAL_DASHBOARD_PROJECTION = { containers: 1, } as const; -function getSourceConnectionMismatches( - sources: SourceForValidation[], - tiles: ExternalDashboardTileWithId[], -): string[] { - const sourceById = new Map(sources.map(s => [s._id.toString(), s])); - - const sourcesWithInvalidConnections: string[] = []; - for (const tile of tiles) { - if ( - isConfigTile(tile) && - isRawSqlExternalTileConfig(tile.config) && - tile.config.sourceId - ) { - const source = sourceById.get(tile.config.sourceId); - if (source && source.connection.toString() !== tile.config.connectionId) { - sourcesWithInvalidConnections.push(tile.config.sourceId); - } - } - } - - return sourcesWithInvalidConnections; -} - /** * @openapi * components: @@ -1911,90 +1878,14 @@ router.post( containers, } = req.body; - // Tile-level container/tab ref resolution: container structure - // (duplicate ids, tab uniqueness) was checked by the body schema; - // tile-resolution runs here against the request body's containers - // (POST has no existing dashboard to fall back to). - const tileRefIssues = collectTileContainerRefIssues( - containers ?? [], - tiles, - ); - if (tileRefIssues.length > 0) { - return res.status(400).json({ - message: tileRefIssues.join('; '), - }); - } - - // Hoist the source fetch so the source-derived validators - // (missing sources, source/connection mismatch, heatmap source - // kind) reuse one query result instead of each re-running - // `getSources(team)`. onClick helpers stay separate because - // they own their own dashboard lookup as well as a sources - // lookup; firing them inside the same Promise.all keeps the - // request latency unchanged. - const [ - sources, - missingConnections, - missingOnClickDashboards, - invalidOnClickSearchSources, - ] = await Promise.all([ - fetchSourcesForValidation(teamId), - getMissingConnections(teamId, tiles), - getMissingOnClickDashboards(teamId, tiles), - getInvalidOnClickSearchSources(teamId, tiles), - ]); - - const missingSources = getMissingSources(sources, tiles, filters); - const sourceConnectionMismatches = getSourceConnectionMismatches( - sources, + const validationError = await validateDashboardTiles({ + teamId: teamId.toString(), tiles, - ); - const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( - sources, - tiles, - ); - - if (missingSources.length > 0) { - return res.status(400).json({ - message: `Could not find the following source IDs: ${missingSources.join( - ', ', - )}`, - }); - } - if (missingConnections.length > 0) { - return res.status(400).json({ - message: `Could not find the following connection IDs: ${missingConnections.join( - ', ', - )}`, - }); - } - if (sourceConnectionMismatches.length > 0) { - return res.status(400).json({ - message: `The following source IDs do not match the specified connections: ${sourceConnectionMismatches.join( - ', ', - )}`, - }); - } - if (heatmapNonTraceSources.length > 0) { - return res.status(400).json({ - message: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join( - ', ', - )}`, - }); - } - if (missingOnClickDashboards.length > 0) { - return res.status(400).json({ - message: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join( - ', ', - )}`, - }); - } - if (invalidOnClickSearchSources.length > 0) { - return res.status(400).json({ - message: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join( - ', ', - )}`, - }); + filters, + containers: containers ?? [], + }); + if (validationError) { + return res.status(400).json({ message: validationError }); } const internalTiles = convertExternalTilesToInternal(tiles); @@ -2201,122 +2092,35 @@ router.put( containers, } = req.body ?? {}; - // Hoist the source fetch and the existing-dashboard read so the - // source-derived validators reuse one query result and the - // heatmap source-kind check can scope itself to tiles whose - // sourceId/displayType actually changed in this request (a - // source whose `kind` was changed after the dashboard was - // originally accepted shouldn't wedge unrelated edits). onClick - // helpers stay separate because they own their own dashboard - // lookup as well as a sources lookup; firing them inside the - // same Promise.all keeps the request latency unchanged. - const [ - sources, - missingConnections, - missingOnClickDashboards, - invalidOnClickSearchSources, - existingDashboard, - ] = await Promise.all([ - fetchSourcesForValidation(teamId), - getMissingConnections(teamId, tiles), - getMissingOnClickDashboards(teamId, tiles), - getInvalidOnClickSearchSources(teamId, tiles), - Dashboard.findOne( - { _id: dashboardId, team: teamId }, - { tiles: 1, filters: 1, containers: 1 }, - ).lean(), - ]); - - const missingSources = getMissingSources(sources, tiles, filters); - const sourceConnectionMismatches = getSourceConnectionMismatches( - sources, - tiles, - ); - const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( - sources, - filterChangedHeatmapTiles(tiles, existingDashboard?.tiles ?? []), - ); + const existingDashboard = await Dashboard.findOne( + { _id: dashboardId, team: teamId }, + { tiles: 1, filters: 1, containers: 1 }, + ).lean(); - if (missingSources.length > 0) { - return res.status(400).json({ - message: `Could not find the following source IDs: ${missingSources.join( - ', ', - )}`, - }); - } - if (missingConnections.length > 0) { - return res.status(400).json({ - message: `Could not find the following connection IDs: ${missingConnections.join( - ', ', - )}`, - }); - } - if (sourceConnectionMismatches.length > 0) { - return res.status(400).json({ - message: `The following source IDs do not match the specified connections: ${sourceConnectionMismatches.join( - ', ', - )}`, - }); - } - if (heatmapNonTraceSources.length > 0) { - return res.status(400).json({ - message: `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join( - ', ', - )}`, - }); - } - if (missingOnClickDashboards.length > 0) { - return res.status(400).json({ - message: `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join( - ', ', - )}`, - }); + if (existingDashboard == null) { + return res.sendStatus(404); } - if (invalidOnClickSearchSources.length > 0) { - return res.status(400).json({ - message: `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join( - ', ', - )}`, - }); + + const effectiveContainers = + containers ?? existingDashboard.containers ?? []; + const validationError = await validateDashboardTiles({ + teamId: teamId.toString(), + tiles, + filters, + existingTiles: existingDashboard.tiles ?? [], + containers: effectiveContainers, + }); + if (validationError) { + return res.status(400).json({ message: validationError }); } - // PUT performs a read-modify-write on the dashboard doc with no - // optimistic-concurrency check (no `If-Match` / `updatedAt` - // guard). Concurrent PUTs may silently overwrite each other, - // which can leave orphan tile->container refs on layout-shape - // edits. The endpoint contract is at-most-one-writer; the - // OpenAPI description above documents this. Adding ETag-style - // concurrency would be a breaking change to the request shape - // and is tracked separately. `existingDashboard` is fetched in - // the hoisted Promise.all above and reused here for tile-ref - // resolution against the effective container set. const existingTileIds = new Set( - (existingDashboard?.tiles ?? []).map((t: { id: string }) => t.id), + (existingDashboard.tiles ?? []).map((t: { id: string }) => t.id), ); const existingFilterIds = new Set( - (existingDashboard?.filters ?? []).map((f: { id: string }) => f.id), + (existingDashboard.filters ?? []).map((f: { id: string }) => f.id), ); - // Tile-level container/tab ref resolution: container structure - // was checked by the body schema; tile-resolution runs here - // against the effective container set so an omitted `containers` - // body field (which preserves the existing array on write) is - // resolved against the doc's existing containers rather than an - // empty fallback. Without this, a PUT that updates only `tiles` - // and leaves a tile pointing at a real preserved container - // would be rejected with "Tile references unknown containerId". - const effectiveContainers = - containers ?? existingDashboard?.containers ?? []; - const tileRefIssues = collectTileContainerRefIssues( - effectiveContainers, - tiles, - ); - if (tileRefIssues.length > 0) { - return res.status(400).json({ - message: tileRefIssues.join('; '), - }); - } - const internalTiles = convertExternalTilesToInternal( tiles, existingTileIds, diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 2d5a2af6e8..7a1e342e5b 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -1,9 +1,13 @@ -import { displayTypeSupportsRawSqlAlerts } from '@hyperdx/common-utils/dist/core/utils'; +import { + displayTypeSupportsBuilderAlerts, + displayTypeSupportsRawSqlAlerts, +} from '@hyperdx/common-utils/dist/core/utils'; import { validateDashboardContainersStructure, validateDashboardTileContainerRefs, } from '@hyperdx/common-utils/dist/dashboardValidation'; import { + isBuilderSavedChartConfig, isHeatmapCompatibleSource, isRawSqlSavedChartConfig, } from '@hyperdx/common-utils/dist/guards'; @@ -1056,8 +1060,122 @@ export function convertExternalFiltersToInternal( } /** - * Delete alerts for tiles that were removed or converted to raw SQL - * (which doesn't support alerts). + * Returns source IDs on raw SQL tiles whose connection doesn't match + * the source's persisted connection. Catches copy-paste errors where + * the LLM mixes up sourceId and connectionId from different sources. + */ +export function getSourceConnectionMismatches( + sources: SourceForValidation[], + tiles: ExternalDashboardTileWithId[], +): string[] { + const sourceById = new Map(sources.map(s => [s._id.toString(), s])); + + const mismatched: string[] = []; + for (const tile of tiles) { + if ( + isConfigTile(tile) && + isRawSqlExternalTileConfig(tile.config) && + tile.config.sourceId + ) { + const source = sourceById.get(tile.config.sourceId); + if (source && source.connection.toString() !== tile.config.connectionId) { + mismatched.push(tile.config.sourceId); + } + } + } + + return mismatched; +} + +// ── Shared tile validation ─────────────────────────────────────────────── + +export type TileValidationContext = { + teamId: string; + tiles: ExternalDashboardTileWithId[]; + /** Filters to check for missing source IDs (create/full-update paths). */ + filters?: (ExternalDashboardFilter | ExternalDashboardFilterWithId)[]; + /** Existing internal tiles for scoping heatmap change detection (update paths). */ + existingTiles?: DashboardDocument['tiles']; + /** Container set to validate tile containerId/tabId refs against. */ + containers: DashboardContainer[]; +}; + +/** + * Run the full suite of tile validation checks (sources, connections, + * heatmap source-kind, onClick targets, container/tab refs). Returns + * `null` when all checks pass, or an error message string on failure. + * + * Consolidates the ~95-line validation block that was previously + * duplicated across REST v2 POST/PUT, MCP save (create/update), and + * MCP patch handlers. + */ +export async function validateDashboardTiles( + ctx: TileValidationContext, +): Promise { + const { teamId, tiles, filters, existingTiles, containers } = ctx; + + // Container/tab ref resolution. + const tileRefIssues = collectTileContainerRefIssues(containers, tiles); + if (tileRefIssues.length > 0) { + return tileRefIssues.join('; '); + } + + // Fetch sources/connections/onClick targets in parallel. + const [ + sources, + missingConnections, + missingOnClickDashboards, + invalidOnClickSearchSources, + ] = await Promise.all([ + fetchSourcesForValidation(teamId), + getMissingConnections(teamId, tiles), + getMissingOnClickDashboards(teamId, tiles), + getInvalidOnClickSearchSources(teamId, tiles), + ]); + + const missingSources = getMissingSources(sources, tiles, filters); + if (missingSources.length > 0) { + return `Could not find the following source IDs: ${missingSources.join(', ')}`; + } + if (missingConnections.length > 0) { + return `Could not find the following connection IDs: ${missingConnections.join(', ')}`; + } + + const sourceConnectionMismatches = getSourceConnectionMismatches( + sources, + tiles, + ); + if (sourceConnectionMismatches.length > 0) { + return `The following source IDs do not match the specified connections: ${sourceConnectionMismatches.join(', ')}`; + } + + // Heatmap source-kind gate. On create (no existingTiles), validate all + // tiles. On update, scope to tiles whose sourceId/displayType changed. + const heatmapTilesToCheck = existingTiles + ? filterChangedHeatmapTiles(tiles, existingTiles) + : tiles; + const heatmapNonTraceSources = getHeatmapTilesWithIncompatibleSources( + sources, + heatmapTilesToCheck, + ); + if (heatmapNonTraceSources.length > 0) { + return `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`; + } + + if (missingOnClickDashboards.length > 0) { + return `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`; + } + if (invalidOnClickSearchSources.length > 0) { + return `The following onClick search source IDs are not log or trace sources: ${invalidOnClickSearchSources.join(', ')}`; + } + + return null; +} + +/** + * Delete alerts for tiles that were removed or whose config no longer + * supports alerts (raw SQL with incompatible displayType, or builder + * tiles with incompatible displayType like Pie/Table/Heatmap/etc.). */ export async function cleanupDashboardAlerts({ dashboardId, @@ -1072,13 +1190,19 @@ export async function cleanupDashboardAlerts({ }) { const newTileIdSet = new Set(internalTiles.map(t => t.id)); const tileIdsToDeleteAlerts = [ + // Tiles whose config no longer supports alerts (raw SQL or builder). ...internalTiles - .filter( - tile => - isRawSqlSavedChartConfig(tile.config) && - !displayTypeSupportsRawSqlAlerts(tile.config.displayType), - ) + .filter(tile => { + if (isRawSqlSavedChartConfig(tile.config)) { + return !displayTypeSupportsRawSqlAlerts(tile.config.displayType); + } + if (isBuilderSavedChartConfig(tile.config)) { + return !displayTypeSupportsBuilderAlerts(tile.config.displayType); + } + return false; + }) .map(tile => tile.id), + // Tiles that were completely removed. ...[...existingTileIds].filter(id => !newTileIdSet.has(id)), ]; if (tileIdsToDeleteAlerts.length > 0) { From 869b3365fb625035cf3fdc342b6da567d952fa14 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 27 May 2026 10:02:04 -0700 Subject: [PATCH 09/11] chore: unexport validation helpers now internal to validateDashboardTiles --- .../external-api/v2/utils/dashboards.ts | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 7a1e342e5b..8e72aecf8c 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -73,7 +73,7 @@ export type ConfigTile = ExternalDashboardTileWithId & { config: Exclude; }; -export function isRawSqlExternalTileConfig( +function isRawSqlExternalTileConfig( config: ExternalDashboardTileConfig, ): config is ExternalDashboardRawSqlTileConfig { return 'configType' in config && config.configType === 'sql'; @@ -753,15 +753,13 @@ export function convertToInternalTileConfig( * router can pass a single fetched array into multiple helpers without * pulling in `controllers/sources` for the type alone. */ -export type SourceForValidation = Awaited< - ReturnType ->[number]; +type SourceForValidation = Awaited>[number]; /** Fetches sources for a team. Re-exports the controller call so callers * outside `controllers/sources` don't need a second import for the * validation flow. The return type is the awaited shape of `getSources` * (an array of Source documents) so callers can `await` it directly. */ -export async function fetchSourcesForValidation( +async function fetchSourcesForValidation( team: string | mongoose.Types.ObjectId, ): Promise { return getSources(team.toString()); @@ -778,7 +776,7 @@ function getTileOnClick(tile: ExternalDashboardTileWithId) { } /** Returns source IDs referenced in tiles/filters that do not exist for the team */ -export function getMissingSources( +function getMissingSources( sources: SourceForValidation[], tiles: ExternalDashboardTileWithId[], filters?: (ExternalDashboardFilter | ExternalDashboardFilterWithId)[], @@ -826,7 +824,7 @@ export function getMissingSources( * `packages/common-utils/src/guards.ts` and `ChartEditorControls.tsx`), so * UI and API gates move together. */ -export function getHeatmapTilesWithIncompatibleSources( +function getHeatmapTilesWithIncompatibleSources( sources: SourceForValidation[], tiles: ExternalDashboardTileWithId[], ): string[] { @@ -860,7 +858,7 @@ export function getHeatmapTilesWithIncompatibleSources( * tiles, tiles whose displayType just changed to heatmap, and tiles * whose `sourceId` changed all flow through the check. */ -export function filterChangedHeatmapTiles( +function filterChangedHeatmapTiles( requestTiles: ExternalDashboardTileWithId[], existingTiles: DashboardDocument['tiles'], ): ExternalDashboardTileWithId[] { @@ -904,7 +902,7 @@ export function filterChangedHeatmapTiles( * Sources that don't exist are ignored here, getMissingSources handles that * case separately with a clearer error message. */ -export async function getInvalidOnClickSearchSources( +async function getInvalidOnClickSearchSources( team: string | mongoose.Types.ObjectId, tiles: ExternalDashboardTileWithId[], ): Promise { @@ -929,7 +927,7 @@ export async function getInvalidOnClickSearchSources( * Returns dashboard IDs referenced by tile OnClick link-outs (mode=id, * type=dashboard) that do not exist for the team. */ -export async function getMissingOnClickDashboards( +async function getMissingOnClickDashboards( team: string | mongoose.Types.ObjectId, tiles: ExternalDashboardTileWithId[], ): Promise { @@ -955,7 +953,7 @@ export async function getMissingOnClickDashboards( } /** Returns connection IDs referenced in tiles that do not belong to the team */ -export async function getMissingConnections( +async function getMissingConnections( team: string | mongoose.Types.ObjectId, tiles: ExternalDashboardTileWithId[], ): Promise { @@ -1064,7 +1062,7 @@ export function convertExternalFiltersToInternal( * the source's persisted connection. Catches copy-paste errors where * the LLM mixes up sourceId and connectionId from different sources. */ -export function getSourceConnectionMismatches( +function getSourceConnectionMismatches( sources: SourceForValidation[], tiles: ExternalDashboardTileWithId[], ): string[] { From e58895d5bd328996ac2f12ad6cc8eed5218c0c21 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Fri, 29 May 2026 08:25:51 -0700 Subject: [PATCH 10/11] refactor(mcp): extract inputSchema to schemas.ts for patch/search dashboard tools Move inline inputSchema definitions from patchDashboard.ts and searchDashboards.ts into schemas.ts as mcpPatchDashboardSchema and mcpSearchDashboardsSchema. Also replace unicode escape \u2014 with literal em dash in patch description. --- .../mcp/tools/dashboards/patchDashboard.ts | 32 +------------- .../api/src/mcp/tools/dashboards/schemas.ts | 43 +++++++++++++++++++ .../mcp/tools/dashboards/searchDashboards.ts | 16 +------ 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index 20017d889f..0ee13f8cf9 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -1,7 +1,6 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { uniq } from 'lodash'; import mongoose from 'mongoose'; -import { z } from 'zod'; import * as config from '@/config'; import Dashboard from '@/models/dashboard'; @@ -16,7 +15,7 @@ import type { ExternalDashboardTileWithId } from '@/utils/zod'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; -import { mcpPatchTileSchema } from './schemas'; +import { mcpPatchDashboardSchema } from './schemas'; export function registerPatchDashboard( server: McpServer, @@ -35,34 +34,7 @@ export function registerPatchDashboard( 'tile by tileId \u2014 all in one call. Unmentioned tiles and fields are preserved. ' + 'Use clickstack_get_dashboard_tile to inspect a tile before patching it. ' + 'IMPORTANT: After patching a tile, run clickstack_query_tile to confirm the query still works.', - inputSchema: z.object({ - dashboardId: z.string().describe('Dashboard ID.'), - name: z - .string() - .min(1) - .optional() - .describe('New dashboard name. Omit to keep the current name.'), - tags: z - .array(z.string()) - .optional() - .describe( - 'New tags array (replaces all existing tags). Omit to keep the current tags.', - ), - tileId: z - .string() - .optional() - .describe( - 'ID of the tile to replace. Must be paired with `tile`. ' + - 'Obtain tile IDs from clickstack_get_dashboard.', - ), - tile: mcpPatchTileSchema - .optional() - .describe( - 'The full replacement tile definition. Replaces the tile matched by tileId. ' + - 'Layout fields (x, y, w, h), name, and containerId/tabId default to the ' + - "existing tile's values when omitted, so you only need to specify what changed.", - ), - }), + inputSchema: mcpPatchDashboardSchema, }, withToolTracing( 'clickstack_patch_dashboard', diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 22d194c255..a1a63fa593 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -773,6 +773,49 @@ export const mcpFiltersParam = z ']', ); +export const mcpPatchDashboardSchema = z.object({ + dashboardId: z.string().describe('Dashboard ID.'), + name: z + .string() + .min(1) + .optional() + .describe('New dashboard name. Omit to keep the current name.'), + tags: z + .array(z.string()) + .optional() + .describe( + 'New tags array (replaces all existing tags). Omit to keep the current tags.', + ), + tileId: z + .string() + .optional() + .describe( + 'ID of the tile to replace. Must be paired with `tile`. ' + + 'Obtain tile IDs from clickstack_get_dashboard.', + ), + tile: mcpPatchTileSchema + .optional() + .describe( + 'The full replacement tile definition. Replaces the tile matched by tileId. ' + + 'Layout fields (x, y, w, h), name, and containerId/tabId default to the ' + + "existing tile's values when omitted, so you only need to specify what changed.", + ), +}); + +export const mcpSearchDashboardsSchema = z.object({ + query: z + .string() + .max(200) + .optional() + .describe( + 'Search term to match against dashboard names (case-insensitive substring match).', + ), + tags: z + .array(z.string().min(1)) + .optional() + .describe('Filter to dashboards that have ALL of these tags.'), +}); + export const mcpContainersParam = z .array(DashboardContainerSchema) .max(DASHBOARD_MAX_CONTAINERS) diff --git a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts index 66f8f82f3e..fa77915fd3 100644 --- a/packages/api/src/mcp/tools/dashboards/searchDashboards.ts +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -1,6 +1,5 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { escapeRegExp } from 'lodash'; -import { z } from 'zod'; import * as config from '@/config'; import Dashboard from '@/models/dashboard'; @@ -8,6 +7,7 @@ import logger from '@/utils/logger'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; +import { mcpSearchDashboardsSchema } from './schemas'; const SEARCH_RESULTS_LIMIT = 100; @@ -26,19 +26,7 @@ export function registerSearchDashboards( 'Search dashboards by name and/or tags. Returns matching dashboards with ' + 'their IDs, names, and tags. More targeted than clickstack_get_dashboard (which ' + 'lists all dashboards). At least one of query or tags must be provided.', - inputSchema: z.object({ - query: z - .string() - .max(200) - .optional() - .describe( - 'Search term to match against dashboard names (case-insensitive substring match).', - ), - tags: z - .array(z.string().min(1)) - .optional() - .describe('Filter to dashboards that have ALL of these tags.'), - }), + inputSchema: mcpSearchDashboardsSchema, }, withToolTracing( 'clickstack_search_dashboards', From 723e940b167ea57d1c9ef30d5c8d3b29debfe58c Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Fri, 29 May 2026 08:34:14 -0700 Subject: [PATCH 11/11] chore: unexport mcpPatchTileSchema (now internal to schemas.ts) --- packages/api/src/mcp/tools/dashboards/schemas.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index a1a63fa593..5990a9e6b2 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -630,7 +630,7 @@ const mcpPatchTileLayoutSchema = z.object({ // Build the patch tile union by extending the default-free layout with // each tile type's config shape. We only need the `config` field from // each tile schema; the layout wrapper is replaced. -export const mcpPatchTileSchema = z.union([ +const mcpPatchTileSchema = z.union([ mcpPatchTileLayoutSchema.extend({ config: mcpLineTileSchema.shape.config }), mcpPatchTileLayoutSchema.extend({ config: mcpBarTileSchema.shape.config }), mcpPatchTileLayoutSchema.extend({ config: mcpTableTileSchema.shape.config }),