diff --git a/.changeset/mcp-dashboard-improvements.md b/.changeset/mcp-dashboard-improvements.md new file mode 100644 index 0000000000..375224e649 --- /dev/null +++ b/.changeset/mcp-dashboard-improvements.md @@ -0,0 +1,21 @@ +--- +'@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. + +**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/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 5c63a326f8..0000000000 --- a/packages/api/src/mcp/__tests__/dashboards.test.ts +++ /dev/null @@ -1,2517 +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('clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_get_dashboard', { - id: fakeId, - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - }); - - describe('clickstack_save_dashboard', () => { - it('should create a new dashboard with tiles', async () => { - const sourceId = traceSource._id.toString(); - const result = await callTool(client, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, - 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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('clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, - 'clickstack_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('clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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 clickstack_get_dashboard and - // back into clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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('clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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('clickstack_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, 'clickstack_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, 'clickstack_delete_dashboard', { - id: '000000000000000000000000', - }); - - expect(result.isError).toBe(true); - expect(getFirstText(result)).toContain('not found'); - }); - }); - - describe('clickstack_query_tile', () => { - it('should return error for non-existent dashboard', async () => { - const result = await callTool(client, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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, 'clickstack_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 clickstack_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 clickstack_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 clickstack_get_dashboard with no id. - expect(wf).toMatch(/clickstack_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..055887db15 --- /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 - clickstack_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!, 'clickstack_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!, 'clickstack_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..64b9f9cd04 --- /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 - clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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..8cd5a655ec --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/getDashboardTile.test.ts @@ -0,0 +1,103 @@ +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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..666f82edaf --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts @@ -0,0 +1,552 @@ +import Dashboard from '@/models/dashboard'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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 not modify other tiles in the database (positional update)', async () => { + const sourceId = ctx.traceSource._id.toString(); + const createResult = await callTool( + ctx.client!, + 'clickstack_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!, + 'clickstack_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(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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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..5bb16e6113 --- /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 clickstack_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 clickstack_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 clickstack_get_dashboard with no id. + expect(wf).toMatch(/clickstack_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..3eb44a8676 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/queryTile.test.ts @@ -0,0 +1,110 @@ +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - clickstack_query_tile', () => { + const ctx = setupDashboardTests(); + + it('should return error for non-existent dashboard', async () => { + const result = await callTool(ctx.client!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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..d0cfe53521 --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts @@ -0,0 +1,1806 @@ +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 - clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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 clickstack_get_dashboard and + // back into clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, + 'clickstack_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..72a44d4d3f --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/searchDashboards.test.ts @@ -0,0 +1,261 @@ +import mongoose from 'mongoose'; + +import Dashboard from '@/models/dashboard'; + +import { callTool, getFirstText } from '../mcpTestUtils'; +import { setupDashboardTests } from './setup'; + +describe('MCP Dashboard Tools - clickstack_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!, 'clickstack_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!, 'clickstack_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!, + 'clickstack_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!, 'clickstack_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!, 'clickstack_search_dashboards', { + query: 'nonexistent-dashboard-name', + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + 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!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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 reject empty query string', async () => { + await new Dashboard({ + name: 'Should Not Appear', + tiles: [], + team: ctx.team._id, + }).save(); + + const result = await callTool(ctx.client!, 'clickstack_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!, 'clickstack_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!, 'clickstack_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', + 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!, 'clickstack_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 cb60705ffa..9b1cc2f730 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; clickstack_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; clickstack_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. @@ -1247,10 +1265,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..03f834809c --- /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( + 'clickstack_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 clickstack_get_dashboard (without an ID) to list dashboards, ' + + 'then clickstack_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 clickstack_get_dashboard.', + ), + }), + }, + withToolTracing( + 'clickstack_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..0ee13f8cf9 --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -0,0 +1,286 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { uniq } from 'lodash'; +import mongoose from 'mongoose'; + +import * as config from '@/config'; +import Dashboard from '@/models/dashboard'; +import { + cleanupDashboardAlerts, + convertToExternalDashboard, + convertToInternalTileConfig, + isConfigTile, + validateDashboardTiles, +} from '@/routers/external-api/v2/utils/dashboards'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { withToolTracing } from '../../utils/tracing'; +import type { McpContext } from '../types'; +import { mcpPatchDashboardSchema } from './schemas'; + +export function registerPatchDashboard( + server: McpServer, + context: McpContext, +): void { + const { teamId } = context; + const frontendUrl = config.FRONTEND_URL; + + server.registerTool( + 'clickstack_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 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: mcpPatchDashboardSchema, + }, + withToolTracing( + 'clickstack_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' }], + }; + } + + // 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; + } + if (tags !== undefined) { + setPayload.tags = uniq(tags); + } + + let patchedTile: ExternalDashboardTileWithId | undefined; + + // ── Tile-level patch ───────────────────────────────────────── + if (tileId !== undefined && inputTile !== undefined) { + // 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 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 = + convertToExternalDashboard(existingDashboard); + 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(', ')}`, + }, + ], + }; + } + + // 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[existingIdx] 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. + // 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 ?? '', + 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 + : existingContainerId, + tabId: 'tabId' in incoming ? incoming.tabId : existingTabId, + // The config comes from the incoming tile (validated by Zod). + config: incoming.config, + } as ExternalDashboardTileWithId; + + // 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: validationError }], + }; + } + + // Convert only the patched tile to internal format. + if (!isConfigTile(mergedTile)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: 'Tile must have a config block.', + }, + ], + }; + } + const internalTile = convertToInternalTileConfig(mergedTile); + + // 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( + 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). ` + + 'The entire update was rejected — name/tags changes (if any) were not applied. Resubmit.', + }, + ], + }; + } + 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 = { + id: updatedDashboard._id.toString(), + name: updatedDashboard.name, + tags: updatedDashboard.tags, + ...(frontendUrl + ? { url: `${frontendUrl}/dashboards/${updatedDashboard._id}` } + : {}), + }; + if (patchedTile) { + output.patchedTile = patchedTile; + output.hint = + 'Use clickstack_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 4e118c6212..84f19db437 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, @@ -49,7 +42,8 @@ export function registerSaveDashboard( 'Call clickstack_list_sources first to obtain sourceId and connectionId values. ' + 'IMPORTANT: After saving a dashboard, always run clickstack_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 clickstack_patch_dashboard instead.', inputSchema: z.object({ id: z .string() @@ -180,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 }], }; } @@ -373,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 { @@ -428,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/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index fc692acbb7..e8c25b8997 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -311,7 +311,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) @@ -612,6 +612,46 @@ 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() + .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(), + h: z.number().min(1).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. +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( @@ -735,6 +775,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 new file mode 100644 index 0000000000..fa77915fd3 --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/searchDashboards.ts @@ -0,0 +1,115 @@ +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { escapeRegExp } from 'lodash'; + +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'; +import { mcpSearchDashboardsSchema } from './schemas'; + +const SEARCH_RESULTS_LIMIT = 100; + +export function registerSearchDashboards( + server: McpServer, + context: McpContext, +): void { + const { teamId } = context; + const frontendUrl = config.FRONTEND_URL; + + server.registerTool( + 'clickstack_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 clickstack_get_dashboard (which ' + + 'lists all dashboards). At least one of query or tags must be provided.', + inputSchema: mcpSearchDashboardsSchema, + }, + withToolTracing( + 'clickstack_search_dashboards', + context, + async ({ query, tags }) => { + 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 (non-empty string) or tags (non-empty array).', + }, + ], + }; + } + + const filter: Record = { team: teamId }; + + if (hasQuery) { + filter.name = { $regex: escapeRegExp(query), $options: 'i' }; + } + if (hasTags) { + filter.tags = { $all: tags }; + } + + try { + const dashboards = await Dashboard.find(filter) + .select({ name: 1, tags: 1 }) + .limit(SEARCH_RESULTS_LIMIT + 1) + .lean(); + + 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, + ...(frontendUrl + ? { url: `${frontendUrl}/dashboards/${d._id}` } + : {}), + })); + + return { + content: [ + { + type: 'text' as const, + 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 }, + 'clickstack_search_dashboards: query failed', + ); + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `Search failed: ${err instanceof Error ? err.message : String(err)}`, + }, + ], + }; + } + }, + ), + ); +} diff --git a/packages/api/src/mcp/tools/query/schemas.ts b/packages/api/src/mcp/tools/query/schemas.ts index 5fe673da7f..19abd9c71c 100644 --- a/packages/api/src/mcp/tools/query/schemas.ts +++ b/packages/api/src/mcp/tools/query/schemas.ts @@ -13,7 +13,10 @@ 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' + @@ -21,8 +24,10 @@ const WHERE_LANGUAGE_DESCRIPTION = ' Lucene (default): Column:value, Column.mapKey:value, Column:>100\n' + " SQL: Column = 'value', SpanAttributes['key'] = 'value'\n\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.\n\n' + + '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%'.\n\n" + 'Common mistake: writing Column:value (Lucene) but setting whereLanguage to "sql". ' + 'If your filter uses colon syntax, leave whereLanguage as "lucene" (the default).'; diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index cb3735ae82..5b83bcc41d 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: @@ -1922,90 +1889,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); @@ -2212,122 +2103,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 25c59b35f2..844a92e194 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, isPromqlSavedChartConfig, isRawSqlSavedChartConfig, @@ -70,7 +74,7 @@ export type ConfigTile = ExternalDashboardTileWithId & { config: Exclude; }; -export function isRawSqlExternalTileConfig( +function isRawSqlExternalTileConfig( config: ExternalDashboardTileConfig, ): config is ExternalDashboardRawSqlTileConfig { return 'configType' in config && config.configType === 'sql'; @@ -767,15 +771,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()); @@ -792,7 +794,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)[], @@ -840,7 +842,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[] { @@ -874,7 +876,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[] { @@ -918,7 +920,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 { @@ -943,7 +945,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 { @@ -969,7 +971,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 { @@ -1074,8 +1076,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. + */ +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, @@ -1090,13 +1206,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) {