From 24edf321e6acb8103287a10e2d13938d6a4e6f68 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 5 Jun 2026 10:55:58 -0400 Subject: [PATCH 1/2] fix: Add sourceId to MCP Raw SQL Tile schema --- .changeset/chilled-drinks-pay.md | 6 + .../dashboards/patchDashboard.test.ts | 95 ++++++++++ .../dashboards/saveDashboard.test.ts | 109 +++++++++++ .../__tests__/dashboards/validation.test.ts | 169 ++++++++++++++++++ .../api/src/mcp/prompts/dashboards/content.ts | 36 ++-- .../mcp/tools/dashboards/patchDashboard.ts | 11 ++ .../src/mcp/tools/dashboards/saveDashboard.ts | 11 ++ .../api/src/mcp/tools/dashboards/schemas.ts | 14 +- .../src/mcp/tools/dashboards/validation.ts | 61 +++++++ .../external-api/v2/utils/dashboards.ts | 2 +- packages/common-utils/src/macros.ts | 16 ++ 11 files changed, 518 insertions(+), 12 deletions(-) create mode 100644 .changeset/chilled-drinks-pay.md create mode 100644 packages/api/src/mcp/__tests__/dashboards/validation.test.ts create mode 100644 packages/api/src/mcp/tools/dashboards/validation.ts diff --git a/.changeset/chilled-drinks-pay.md b/.changeset/chilled-drinks-pay.md new file mode 100644 index 0000000000..d5a8b7d1b9 --- /dev/null +++ b/.changeset/chilled-drinks-pay.md @@ -0,0 +1,6 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/api": patch +--- + +fix: Add sourceId to MCP Raw SQL Tile schema diff --git a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts index 666f82edaf..98d4927c3c 100644 --- a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.test.ts @@ -294,6 +294,101 @@ describe('MCP Dashboard Tools - clickstack_patch_dashboard', () => { expect(getFirstText(result)).toContain('source'); }); + it('should reject patching a tile to raw SQL with $__filters but no sourceId', async () => { + const sourceId = ctx.traceSource._id.toString(); + const connectionId = ctx.connection._id.toString(); + const createResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'SQL Macro Patch Test', + tiles: [ + { + name: 'Valid Tile', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count' }], + }, + }, + ], + }, + ); + const created = JSON.parse(getFirstText(createResult)); + const tileId = created.tiles[0].id; + + const result = await callTool(ctx.client!, 'clickstack_patch_dashboard', { + dashboardId: created.id, + tileId, + tile: { + name: 'Filtered SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ServiceName LIMIT 10', + }, + }, + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('sourceId'); + expect(text).toContain('$__filters'); + expect(text).toContain('Filtered SQL'); + + // The original tile must be untouched in the database. + const dashboard = await Dashboard.findById(created.id); + const persistedTile = dashboard?.tiles?.find( + (t: { id: string }) => t.id === tileId, + ); + expect(persistedTile?.config?.name).toBe('Valid Tile'); + }); + + it('should allow patching a tile to raw SQL with $__filters when a sourceId is set', async () => { + const sourceId = ctx.traceSource._id.toString(); + const connectionId = ctx.connection._id.toString(); + const createResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'SQL Macro Patch Test (valid)', + 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: 'Filtered SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sourceId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ServiceName LIMIT 10', + }, + }, + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.patchedTile.config.sourceId).toBe(sourceId); + }); + it('should not modify other tiles in the database (positional update)', async () => { const sourceId = ctx.traceSource._id.toString(); const createResult = await callTool( diff --git a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts index d0cfe53521..5b58b03e9f 100644 --- a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.test.ts @@ -262,6 +262,115 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => { expect(output.tiles).toHaveLength(1); }); + it('should persist sourceId on a raw SQL tile that uses $__filters', async () => { + const connectionId = ctx.connection._id.toString(); + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool(ctx.client!, 'clickstack_save_dashboard', { + name: 'SQL Dashboard with filters', + tiles: [ + { + name: 'Filtered SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sourceId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ServiceName LIMIT 10', + }, + }, + ], + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.tiles).toHaveLength(1); + // sourceId must round-trip so the $__filters macro can resolve. + expect(output.tiles[0].config.sourceId).toBe(sourceId); + + const dashboard = await Dashboard.findById(output.id); + expect(dashboard?.tiles?.[0]?.config?.source?.toString()).toBe(sourceId); + }); + + it('should reject a raw SQL tile that uses $__filters without a sourceId', async () => { + const connectionId = ctx.connection._id.toString(); + const result = await callTool(ctx.client!, 'clickstack_save_dashboard', { + name: 'SQL Dashboard missing source', + tiles: [ + { + name: 'Filtered SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ServiceName LIMIT 10', + }, + }, + ], + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('sourceId'); + expect(text).toContain('$__filters'); + expect(text).toContain('Filtered SQL'); + + // Nothing should have been persisted. + expect(await Dashboard.countDocuments({})).toBe(0); + }); + + it('should reject a raw SQL tile that uses $__sourceTable without a sourceId', async () => { + const connectionId = ctx.connection._id.toString(); + const result = await callTool(ctx.client!, 'clickstack_save_dashboard', { + name: 'SQL Dashboard missing source', + tiles: [ + { + name: 'Source Table SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: + 'SELECT ServiceName, count() AS c FROM $__sourceTable WHERE $__timeFilter(Timestamp) GROUP BY ServiceName LIMIT 10', + }, + }, + ], + }); + + expect(result.isError).toBe(true); + const text = getFirstText(result); + expect(text).toContain('sourceId'); + expect(text).toContain('$__sourceTable'); + expect(text).toContain('Source Table SQL'); + + // Nothing should have been persisted. + expect(await Dashboard.countDocuments({})).toBe(0); + }); + + it('should allow a raw SQL tile without a sourceId when it does not use $__filters', async () => { + const connectionId = ctx.connection._id.toString(); + const result = await callTool(ctx.client!, 'clickstack_save_dashboard', { + name: 'Multi-table SQL Dashboard', + tiles: [ + { + name: 'Joined SQL', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: + 'SELECT t.ServiceName, count() AS c FROM otel_traces t JOIN otel_logs l ON t.TraceId = l.TraceId GROUP BY t.ServiceName LIMIT 10', + }, + }, + ], + }); + + 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', { diff --git a/packages/api/src/mcp/__tests__/dashboards/validation.test.ts b/packages/api/src/mcp/__tests__/dashboards/validation.test.ts new file mode 100644 index 0000000000..88d0a1910f --- /dev/null +++ b/packages/api/src/mcp/__tests__/dashboards/validation.test.ts @@ -0,0 +1,169 @@ +import mongoose from 'mongoose'; + +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +import { + getRawSqlMissingSourceError, + getRawSqlTilesMissingRequiredSource, +} from '../../tools/dashboards/validation'; + +const connectionId = new mongoose.Types.ObjectId().toString(); +const sourceId = new mongoose.Types.ObjectId().toString(); + +function makeSqlTile(overrides: { + name?: string; + sqlTemplate: string; + sourceId?: string; +}): ExternalDashboardTileWithId { + return { + id: 'sql-tile', + x: 0, + y: 0, + w: 12, + h: 4, + name: overrides.name ?? 'SQL Tile', + config: { + configType: 'sql', + displayType: 'table', + connectionId, + sqlTemplate: overrides.sqlTemplate, + ...(overrides.sourceId ? { sourceId: overrides.sourceId } : {}), + }, + } as ExternalDashboardTileWithId; +} + +describe('getRawSqlTilesMissingRequiredSource', () => { + it('flags a raw SQL tile that uses $__filters without a sourceId', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + name: 'Errors by Service', + sqlTemplate: + 'SELECT count() FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters', + }), + ]), + ).toEqual([{ tile: 'Errors by Service', macros: ['$__filters'] }]); + }); + + it('flags a raw SQL tile that uses $__sourceTable without a sourceId', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + name: 'Span Count', + sqlTemplate: + 'SELECT count() FROM $__sourceTable WHERE $__timeFilter(Timestamp)', + }), + ]), + ).toEqual([{ tile: 'Span Count', macros: ['$__sourceTable'] }]); + }); + + it('reports every source-dependent macro used by a tile', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + name: 'Both Macros', + sqlTemplate: + 'SELECT count() FROM $__sourceTable WHERE $__timeFilter(Timestamp) AND $__filters', + }), + ]), + ).toEqual([ + { tile: 'Both Macros', macros: ['$__filters', '$__sourceTable'] }, + ]); + }); + + it('does not flag a raw SQL tile that sets a sourceId', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + sourceId, + sqlTemplate: + 'SELECT count() FROM $__sourceTable WHERE $__timeFilter(Timestamp) AND $__filters', + }), + ]), + ).toEqual([]); + }); + + it('does not flag a raw SQL tile that uses no source-dependent macro', () => { + // A multi-table query that legitimately omits sourceId and references + // neither $__filters nor $__sourceTable must not be rejected. + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + sqlTemplate: + 'SELECT count() FROM otel_traces t JOIN otel_logs l ON t.TraceId = l.TraceId WHERE $__timeFilter(t.Timestamp) LIMIT 10', + }), + ]), + ).toEqual([]); + }); + + it('falls back to a positional label when the tile has no name', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ + name: '', + sqlTemplate: 'SELECT count() FROM otel_traces WHERE $__filters', + }), + ]), + ).toEqual([{ tile: 'tile #1', macros: ['$__filters'] }]); + }); + + it('ignores builder (non-SQL) tiles entirely', () => { + const builderTile = { + id: 'builder', + x: 0, + y: 0, + w: 12, + h: 4, + name: 'Line', + config: { + displayType: 'line', + sourceId, + select: [{ aggFn: 'count', valueExpression: '', where: '' }], + }, + } as unknown as ExternalDashboardTileWithId; + expect(getRawSqlTilesMissingRequiredSource([builderTile])).toEqual([]); + }); + + it('collects every offending tile across a mixed set', () => { + expect( + getRawSqlTilesMissingRequiredSource([ + makeSqlTile({ name: 'A', sqlTemplate: 'SELECT 1 WHERE $__filters' }), + makeSqlTile({ + name: 'B', + sourceId, + sqlTemplate: 'SELECT 1 WHERE $__filters', + }), + makeSqlTile({ + name: 'C', + sqlTemplate: 'SELECT 1 FROM $__sourceTable', + }), + ]), + ).toEqual([ + { tile: 'A', macros: ['$__filters'] }, + { tile: 'C', macros: ['$__sourceTable'] }, + ]); + }); +}); + +describe('getRawSqlMissingSourceError', () => { + it('returns null when all tiles are valid', () => { + expect( + getRawSqlMissingSourceError([ + makeSqlTile({ sqlTemplate: 'SELECT 1 LIMIT 1' }), + ]), + ).toBeNull(); + }); + + it('builds a message naming the offending tiles and macros', () => { + const message = getRawSqlMissingSourceError([ + makeSqlTile({ + name: 'Errors by Service', + sqlTemplate: 'SELECT count() FROM otel_traces WHERE $__filters', + }), + ]); + expect(message).toContain('sourceId'); + expect(message).toContain('$__filters'); + expect(message).toContain('$__sourceTable'); + expect(message).toContain('Errors by Service (uses $__filters)'); + }); +}); diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index 9b1cc2f730..a79fde71ab 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -67,6 +67,9 @@ Use BUILDER tiles (with sourceId) for most cases: Use RAW SQL tiles (with connectionId) only for queries the builder cannot express: Requires configType: "sql" plus a displayType (line, stacked_bar, table, number, pie). Use when you need JOINs, sub-queries, CTEs, or expressions the builder does not generate. + ALWAYS set sourceId on a raw SQL tile (in addition to connectionId) UNLESS the query reads + from multiple tables (e.g. JOINs across sources). sourceId enables the $__filters and + $__sourceTable macros. When you can set sourceId, include "AND $__filters" in the WHERE clause. == COLUMN NAMING == @@ -677,7 +680,7 @@ copy the right ID and discover which filters are available. examples['infrastructure_sql'] = ` == INFRASTRUCTURE (Raw SQL) == -When to use: the user wants metric-like infrastructure visibility (request rate, ingestion volume, table sizes) but the builder tile types do not express the exact aggregation. Raw SQL tiles take connectionId (not sourceId) and a sqlTemplate with HyperDX macros. +When to use: the user wants metric-like infrastructure visibility (request rate, ingestion volume, table sizes) but the builder tile types do not express the exact aggregation. Raw SQL tiles take connectionId, a sqlTemplate with HyperDX macros, and (when the query reads a single table) a sourceId. Set sourceId and add "AND $__filters" so dashboard filters apply. { name: "Infrastructure (SQL)", @@ -689,7 +692,8 @@ When to use: the user wants metric-like infrastructure visibility (request rate, configType: "sql", displayType: "line", connectionId: "${connectionId}", - sqlTemplate: "SELECT $__timeInterval(Timestamp) AS ts, count() AS logs_per_interval FROM otel_logs WHERE $__timeFilter(Timestamp) GROUP BY ts ORDER BY ts" + sourceId: "${logSourceId}", + sqlTemplate: "SELECT $__timeInterval(Timestamp) AS ts, count() AS logs_per_interval FROM otel_logs WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ts ORDER BY ts" } }, { @@ -699,7 +703,8 @@ When to use: the user wants metric-like infrastructure visibility (request rate, configType: "sql", displayType: "table", connectionId: "${connectionId}", - sqlTemplate: "SELECT ServiceName, count() AS span_count, avg(Duration) AS avg_duration FROM otel_traces WHERE Timestamp >= fromUnixTimestamp64Milli({startDateMilliseconds:Int64}) AND Timestamp < fromUnixTimestamp64Milli({endDateMilliseconds:Int64}) GROUP BY ServiceName ORDER BY span_count DESC LIMIT 20" + sourceId: "${traceSourceId}", + sqlTemplate: "SELECT ServiceName, count() AS span_count, avg(Duration) AS avg_duration FROM otel_traces WHERE Timestamp >= fromUnixTimestamp64Milli({startDateMilliseconds:Int64}) AND Timestamp < fromUnixTimestamp64Milli({endDateMilliseconds:Int64}) AND $__filters GROUP BY ServiceName ORDER BY span_count DESC LIMIT 20" } }, { @@ -709,7 +714,8 @@ When to use: the user wants metric-like infrastructure visibility (request rate, configType: "sql", displayType: "line", connectionId: "${connectionId}", - sqlTemplate: "SELECT $__timeInterval(Timestamp) AS ts, ServiceName, countIf(StatusCode = 'STATUS_CODE_ERROR') / count() AS error_rate FROM otel_traces WHERE $__timeFilter(Timestamp) GROUP BY ServiceName, ts ORDER BY ts" + sourceId: "${traceSourceId}", + sqlTemplate: "SELECT $__timeInterval(Timestamp) AS ts, ServiceName, countIf(StatusCode = 'STATUS_CODE_ERROR') / count() AS error_rate FROM otel_traces WHERE $__timeFilter(Timestamp) AND $__filters GROUP BY ServiceName, ts ORDER BY ts" } } ] @@ -725,7 +731,10 @@ SQL TEMPLATE REFERENCE: $__fromTime / $__toTime start/end as DateTime values $__fromTime_ms / $__toTime_ms start/end as DateTime64 values $__interval_s raw interval in seconds - $__filters dashboard filter conditions (resolves to 1=1 when none) + $__filters dashboard filter conditions (resolves to 1=1 when none). + REQUIRES sourceId on the tile. Without it, filters never apply. + $__sourceTable the source's \`database\`.\`table\`. + REQUIRES sourceId on the tile. Without it, the query fails to run. Query parameters: {startDateMilliseconds:Int64} start of date range in milliseconds @@ -901,7 +910,11 @@ For configType: "sql" tiles, write ClickHouse SQL with template macros: $__fromTime / $__toTime start/end as DateTime values $__fromTime_ms / $__toTime_ms start/end as DateTime64 values $__interval_s raw interval in seconds (for arithmetic) - $__filters dashboard filter conditions (1=1 when none) + $__filters dashboard filter conditions (1=1 when none). + REQUIRES sourceId on the tile. Set sourceId (unless the + query reads multiple tables) so dashboard filters apply. + $__sourceTable the source's \`database\`.\`table\`. + REQUIRES sourceId on the tile. Without it, the query fails to run. QUERY PARAMETERS (ClickHouse parameterized syntax): {startDateMilliseconds:Int64} @@ -1097,6 +1110,7 @@ Workaround: when you need a row-click drill-down from a map-attribute groupBy, a Raw SQL, alias the column: configType: "sql", displayType: "table", + sourceId: "", sqlTemplate: "SELECT SpanAttributes['http.route'] AS Route, count() AS Requests FROM otel_traces WHERE $__timeFilter(TimestampTime) AND $__filters GROUP BY Route ORDER BY Requests DESC LIMIT 100" onClick: { type: "search", target: { mode: "id", id: "" }, whereLanguage: "sql", @@ -1232,9 +1246,11 @@ Example: find top patterns for production services over the last 4 hours: 8. Forgetting to validate tiles after saving Always call clickstack_query_tile on EVERY tile after clickstack_save_dashboard, not just one. Save validates input shape but not query semantics. Several known gaps (Lucene comparison/wildcard on map attributes, builder tiles on metric sources, malformed having) pass save and fail at render time. A dashboard with one bad tile renders the whole page in a degraded state; the user sees "Error loading chart" with no way to know which tile broke unless you validated. If query_tile returns an error, fix the where / SQL and re-save before declaring the dashboard ready. -9. Using sourceId with SQL tiles or connectionId with builder tiles - Builder tiles (line, table, etc.) use sourceId. - SQL tiles (configType: "sql") use connectionId. +9. Using connectionId with builder tiles, or omitting connectionId or sourceId on a single-table SQL tile + Builder tiles (line, table, etc.) use sourceId (no connectionId). + SQL tiles (configType: "sql") REQUIRE connectionId, and should ALSO set sourceId + whenever the query reads a single table. SourceId is what makes $__filters work. + Only omit sourceId on a SQL tile when the query spans multiple tables (e.g. JOINs). 10. Assuming StatusCode or SeverityText values Values like STATUS_CODE_ERROR, Ok, error, fatal vary by deployment. @@ -1313,6 +1329,6 @@ External docs for the syntax and macros referenced in this guide: ClickHouse SQL functions (the "where" field with whereLanguage: "sql", and any sqlTemplate body): https://clickhouse.com/docs/sql-reference - Dashboard time / filter macros used in raw-SQL tile sqlTemplate ($__timeFilter, $__timeFilter_ms, $__timeInterval, $__timeInterval_ms, $__filters): + Dashboard time / filter macros used in raw-SQL tile sqlTemplate ($__timeFilter, $__timeFilter_ms, $__timeInterval, $__timeInterval_ms, $__filters, $__sourceTable): https://clickhouse.com/docs/use-cases/observability/clickstack/dashboards/sql-visualizations`; } diff --git a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts index f21782c85d..9cc0a8483a 100644 --- a/packages/api/src/mcp/tools/dashboards/patchDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/patchDashboard.ts @@ -12,9 +12,11 @@ import { } from '@/routers/external-api/v2/utils/dashboards'; import type { ExternalDashboardTileWithId } from '@/utils/zod'; +import { mcpError } from '../../utils/errors'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; import { mcpPatchDashboardSchema } from './schemas'; +import { getRawSqlMissingSourceError } from './validation'; export function registerPatchDashboard( server: McpServer, @@ -163,6 +165,15 @@ export function registerPatchDashboard( config: incoming.config, } as ExternalDashboardTileWithId; + // Error on raw SQL tiles that have no source defined but which use + // macros which require a source to be set + const sqlFilterSourceError = getRawSqlMissingSourceError([ + mergedTile, + ]); + if (sqlFilterSourceError) { + return mcpError(sqlFilterSourceError); + } + // Validate the patched tile using the shared validation helper. const validationError = await validateDashboardTiles({ teamId, diff --git a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts index 4fcee6d604..725db4a8d6 100644 --- a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts @@ -27,6 +27,7 @@ import { mcpError } from '../../utils/errors'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; import { mcpContainersParam, mcpFiltersParam, mcpTilesParam } from './schemas'; +import { getRawSqlMissingSourceError } from './validation'; export function registerSaveDashboard( server: McpServer, @@ -175,6 +176,11 @@ async function createDashboard({ const { tiles, filters, containers: parsedContainers } = parsed.data; const tilesWithId = tiles as ExternalDashboardTileWithId[]; + const sqlFilterSourceError = getRawSqlMissingSourceError(tilesWithId); + if (sqlFilterSourceError) { + return mcpError(sqlFilterSourceError); + } + const validationError = await validateDashboardTiles({ teamId, tiles: tilesWithId, @@ -272,6 +278,11 @@ async function updateDashboard({ const { tiles, filters, containers: parsedContainers } = parsed.data; const tilesWithId = tiles as ExternalDashboardTileWithId[]; + const sqlFilterSourceError = getRawSqlMissingSourceError(tilesWithId); + if (sqlFilterSourceError) { + return mcpError(sqlFilterSourceError); + } + const existingDashboard = await Dashboard.findOne( { _id: dashboardId, team: teamId }, { tiles: 1, filters: 1, containers: 1 }, diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index f88033a5a7..36b4d79766 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -589,6 +589,16 @@ const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ .describe( 'Connection ID (not sourceId) – call clickstack_list_sources to find available connections', ), + sourceId: z + .string() + .optional() + .describe( + 'Source ID for the table this query reads from (call clickstack_list_sources). ' + + 'ALWAYS set this for raw SQL tiles UNLESS the query reads from multiple tables ' + + '(e.g. JOINs or sub-queries spanning several sources), in which case omit it. ' + + 'sourceId is REQUIRED by two macros: $__filters and $__sourceTable.' + + 'The sourceId must belong to the same connection as connectionId.', + ), sqlTemplate: z .string() .describe( @@ -597,7 +607,9 @@ const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ '{intervalSeconds:Int64}, {intervalMilliseconds:Int64}.\n' + 'Or use macros: $__timeFilter(col), $__timeFilter_ms(col), $__dateFilter(col), ' + '$__fromTime, $__toTime, $__fromTime_ms, $__toTime_ms, ' + - '$__timeInterval(col), $__timeInterval_ms(col), $__interval_s, $__filters.\n' + + '$__timeInterval(col), $__timeInterval_ms(col), $__interval_s, $__filters, $__sourceTable.\n' + + 'IMPORTANT: $__filters and $__sourceTable both require sourceId to be set on this tile. ' + + 'Prefer including "AND $__filters" in the WHERE clause so dashboard filters apply.\n' + 'Example: "SELECT $__timeInterval(TimestampTime) AS ts, ServiceName, count() ' + 'FROM otel_logs WHERE $__timeFilter(TimestampTime) AND $__filters ' + 'GROUP BY ServiceName, ts ORDER BY ts"', diff --git a/packages/api/src/mcp/tools/dashboards/validation.ts b/packages/api/src/mcp/tools/dashboards/validation.ts new file mode 100644 index 0000000000..2cdbbab170 --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/validation.ts @@ -0,0 +1,61 @@ +import { SOURCE_DEPENDENT_MACROS } from '@hyperdx/common-utils/dist/macros'; + +import { + isConfigTile, + isRawSqlExternalTileConfig, +} from '@/routers/external-api/v2/utils/dashboards'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; + +/** + * Returns one entry per raw SQL tile that uses a source-dependent macro but + * does not set a `sourceId`, recording which macro(s) triggered it so callers + * can build a precise error message. + */ +export function getRawSqlTilesMissingRequiredSource( + tiles: ExternalDashboardTileWithId[], +): { tile: string; macros: string[] }[] { + const offending: { tile: string; macros: string[] }[] = []; + tiles.forEach((tile, index) => { + if ( + !isConfigTile(tile) || + !isRawSqlExternalTileConfig(tile.config) || + tile.config.sourceId + ) { + return; + } + const { sqlTemplate } = tile.config; + const macros = SOURCE_DEPENDENT_MACROS.filter(macro => + sqlTemplate.includes(macro), + ); + if (macros.length > 0) { + offending.push({ + tile: tile.name?.trim() || `tile #${index + 1}`, + macros: [...macros], + }); + } + }); + return offending; +} + +/** + * MCP-only guard: a raw SQL tile that uses a source-dependent macro + * ($__filters or $__sourceTable) must set a `sourceId`. + * + * Returns a human-readable error message, or `null` when all tiles are valid. + */ +export function getRawSqlMissingSourceError( + tiles: ExternalDashboardTileWithId[], +): string | null { + const offending = getRawSqlTilesMissingRequiredSource(tiles); + if (offending.length === 0) return null; + const list = offending + .map(({ tile, macros }) => `${tile} (uses ${macros.join(', ')})`) + .join('; '); + return ( + 'Raw SQL tiles that use the $__filters or $__sourceTable macro must set a sourceId. ' + + "Without a source, $__filters cannot resolve dashboard filters against the source's " + + 'columns and $__sourceTable fails at query time. Add a sourceId to the following tiles ' + + '(call clickstack_list_sources to find it), or remove the macro if the query reads ' + + `from multiple tables: ${list}` + ); +} 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 214899f85c..a8506a3e6b 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -74,7 +74,7 @@ export type ConfigTile = ExternalDashboardTileWithId & { config: Exclude; }; -function isRawSqlExternalTileConfig( +export function isRawSqlExternalTileConfig( config: ExternalDashboardTileConfig, ): config is ExternalDashboardRawSqlTileConfig { return 'configType' in config && config.configType === 'sql'; diff --git a/packages/common-utils/src/macros.ts b/packages/common-utils/src/macros.ts index 297ebd1c7e..142cb84724 100644 --- a/packages/common-utils/src/macros.ts +++ b/packages/common-utils/src/macros.ts @@ -161,6 +161,22 @@ export const MACRO_SUGGESTIONS = [ })), ]; +/** + * Macro tokens (in `$__name` form) whose expansion depends on the chart's + * source being configured. With no source: + * - `$__filters` cannot resolve dashboard filters against the source's + * columns and falls back to `(1=1)`, so the filters are silently dropped. + * - `$__sourceTable` throws at render time ("requires a source to be + * selected"), so the query fails outright. + * + * Every other macro only needs the dashboard time range / interval and takes + * its column as an argument, so it does not require a source. + */ +export const SOURCE_DEPENDENT_MACROS = [ + '$__filters', + '$__sourceTable', +] as const; + type MacroMatch = { full: string; args: string[]; From 770e9af2fa12043d382b5a0db11ddffdbeb4b988 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 5 Jun 2026 11:05:56 -0400 Subject: [PATCH 2/2] Update packages/api/src/mcp/tools/dashboards/schemas.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- packages/api/src/mcp/tools/dashboards/schemas.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 36b4d79766..d3a306f426 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -596,7 +596,7 @@ const mcpSqlTileSchema = mcpTileLayoutSchema.extend({ 'Source ID for the table this query reads from (call clickstack_list_sources). ' + 'ALWAYS set this for raw SQL tiles UNLESS the query reads from multiple tables ' + '(e.g. JOINs or sub-queries spanning several sources), in which case omit it. ' + - 'sourceId is REQUIRED by two macros: $__filters and $__sourceTable.' + + 'sourceId is REQUIRED by two macros: $__filters and $__sourceTable. ' + 'The sourceId must belong to the same connection as connectionId.', ), sqlTemplate: z