From 8277835d0eee90f62e4c2800f373804311b92b9e Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 12 May 2026 03:27:28 +0000 Subject: [PATCH 1/2] feat: expose containers and tabs in MCP hyperdx_save_dashboard Follow-up to #2201, which added containers / tabs / per-tile containerId / tabId to the v2 external API. The MCP path was held out of that PR to avoid tangling two surfaces in one review. This change wires the same shape through the MCP path so an AI agent can author dashboards with the new layout primitives: - `mcpTileLayoutSchema` (all tile types) gains optional `containerId` and `tabId`, both capped at `DASHBOARD_CONTAINER_ID_MAX`. - `mcpContainersParam` mirrors `DashboardContainerSchema` from `@hyperdx/common-utils` and gets wired into the `hyperdx_save_dashboard` inputSchema as an optional top-level field. - `createDashboard` / `updateDashboard` helpers now thread `containers` through `createDashboardBodySchema` / `updateDashboardBodySchema` (structural checks: unique container id, unique tab id per container) and then run `collectTileContainerRefIssues` for cross-tile resolution (containerId resolves, tabId resolves to a tab on the container, tabId requires containerId). The update path resolves against the persisted containers when the request body omits the field, matching the v2 PUT handler. - `buildQueryGuidePrompt` gains a `CONTAINERS AND TABS` section that documents the container shape, the cross-field rules, an example authoring call, and the verbatim rejection messages. - New integration tests in `mcp/__tests__/dashboards.test.ts` cover one positive round-trip (create + update + GET) and one negative test per rejection rule (5 tests). Closes #2212. Co-Authored-By: Claude Opus 4.7 --- .changeset/hdx-2212-mcp-containers-tabs.md | 12 + .../api/src/mcp/__tests__/dashboards.test.ts | 248 ++++++++++++++++++ .../api/src/mcp/prompts/dashboards/content.ts | 63 +++++ .../src/mcp/tools/dashboards/saveDashboard.ts | 76 +++++- .../api/src/mcp/tools/dashboards/schemas.ts | 47 ++++ 5 files changed, 441 insertions(+), 5 deletions(-) create mode 100644 .changeset/hdx-2212-mcp-containers-tabs.md diff --git a/.changeset/hdx-2212-mcp-containers-tabs.md b/.changeset/hdx-2212-mcp-containers-tabs.md new file mode 100644 index 0000000000..4dd6f6747a --- /dev/null +++ b/.changeset/hdx-2212-mcp-containers-tabs.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/api': minor +--- + +MCP `hyperdx_save_dashboard` now accepts the dashboard organization layer +added in #2201: an optional `containers` array on the dashboard, plus +`containerId` and `tabId` on each tile. The same five cross-field rules +the external API enforces fire on the MCP path: container ids unique, +tab ids unique within a container, tile.containerId resolves, tile.tabId +resolves to a tab on that container, and tile.tabId requires +tile.containerId. The MCP `buildQueryGuidePrompt` documents the new +shape under a CONTAINERS AND TABS section. diff --git a/packages/api/src/mcp/__tests__/dashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards.test.ts index f2a8eaf1be..8c563d4dcf 100644 --- a/packages/api/src/mcp/__tests__/dashboards.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards.test.ts @@ -425,6 +425,254 @@ describe('MCP Dashboard Tools', () => { }); }); + describe('hyperdx_save_dashboard - containers and tabs', () => { + // Mirrors the v2 external-API "Containers and tabs" describe block so + // the MCP path enforces the same 5 cross-field rules: container id + // unique, tab id unique within a container, tile.containerId + // resolves, tile.tabId resolves to a tab on that container, and + // tile.tabId requires tile.containerId. + const buildTile = ( + sourceId: string, + overrides: Record, + ) => ({ + name: 'Tile', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line' as const, + sourceId, + select: [{ aggFn: 'count' as const, where: '' }], + }, + ...overrides, + }); + + it('should round-trip containers, tabs, and tile containerId/tabId on create and update', async () => { + const sourceId = traceSource._id.toString(); + const containers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + collapsible: true, + bordered: true, + tabs: [ + { id: 'errors', title: 'Errors' }, + { id: 'latency', title: 'Latency' }, + ], + }, + { + id: 'overview', + title: 'Overview', + collapsed: true, + }, + ]; + + const createResult = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Containers MCP Round-Trip', + tiles: [ + buildTile(sourceId, { + name: 'In Group, Tab A', + containerId: 'service-health', + tabId: 'errors', + }), + buildTile(sourceId, { + name: 'In Group, Tab B', + containerId: 'service-health', + tabId: 'latency', + }), + 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); + + const createdTilesByName: Record = Object.fromEntries( + created.tiles.map((t: { name: string }) => [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 Plain Group']).toMatchObject({ + containerId: 'overview', + }); + expect(createdTilesByName['In Plain Group'].tabId).toBeUndefined(); + expect(createdTilesByName.Ungrouped.containerId).toBeUndefined(); + expect(createdTilesByName.Ungrouped.tabId).toBeUndefined(); + + // Verify a fresh GET via the get tool also returns the structure. + const getResult = await callTool(client, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.containers).toEqual(containers); + + // Update: rename a tab, drop the second container, re-home tiles. + const updatedContainers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: true, + tabs: [ + { id: 'errors', title: 'Error Rate' }, + { id: 'latency', title: 'Latency' }, + ], + }, + ]; + const reHomedUngrouped = { + ...createdTilesByName.Ungrouped, + containerId: 'service-health', + tabId: 'errors', + }; + const droppedContainerTile = { + ...createdTilesByName['In Plain Group'], + containerId: undefined, + tabId: undefined, + }; + + const updateResult = await callTool(client, 'hyperdx_save_dashboard', { + id: created.id, + name: 'Containers MCP Round-Trip', + tiles: [ + createdTilesByName['In Group, Tab A'], + createdTilesByName['In Group, Tab B'], + 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: { name: string }) => [t.name, t]), + ); + expect(updatedTilesByName['In Plain Group'].containerId).toBeUndefined(); + expect(updatedTilesByName.Ungrouped).toMatchObject({ + containerId: 'service-health', + tabId: 'errors', + }); + }); + + it('should reject duplicate container ids', async () => { + const sourceId = traceSource._id.toString(); + const result = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Duplicate Container Ids', + tiles: [buildTile(sourceId, {})], + containers: [ + { id: 'dupe', title: 'A', collapsed: false }, + { id: 'dupe', title: 'B', collapsed: false }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Container IDs must be unique'); + }); + + it('should reject duplicate tab ids within a container', async () => { + // Structural rules (duplicate container ids, duplicate tab ids) fire + // through the body schema, which is reported as a stringified issue + // array. Cross-tile-ref rules (covered below) fire through + // collectTileContainerRefIssues and produce plain prose. + const sourceId = traceSource._id.toString(); + const result = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Duplicate Tab Ids', + tiles: [buildTile(sourceId, {})], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [ + { id: 'errors', title: 'Errors' }, + { id: 'errors', title: 'Errors Two' }, + ], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('Duplicate tab id'); + expect(getFirstText(result)).toContain('errors'); + expect(getFirstText(result)).toContain('service-health'); + }); + + it('should reject a tile that supplies tabId without containerId', async () => { + const sourceId = traceSource._id.toString(); + const result = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Tab Without Container', + tiles: [buildTile(sourceId, { tabId: 'errors' })], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain( + 'tabId requires containerId to be set', + ); + }); + + it('should reject a tile that references an unknown containerId', async () => { + const sourceId = traceSource._id.toString(); + const result = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Unknown Container', + tiles: [buildTile(sourceId, { containerId: 'does-not-exist' })], + containers: [{ id: 'real', title: 'Real', collapsed: false }], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain( + 'unknown containerId "does-not-exist"', + ); + }); + + it('should reject a tile that references an unknown tabId within a container', async () => { + const sourceId = traceSource._id.toString(); + const result = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Unknown Tab', + tiles: [ + buildTile(sourceId, { + containerId: 'service-health', + tabId: 'ghost', + }), + ], + containers: [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ], + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('unknown tabId "ghost"'); + }); + }); + describe('hyperdx_delete_dashboard', () => { it('should delete an existing dashboard', async () => { const dashboard = await new Dashboard({ diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index 0015b23249..cb73cce89c 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -682,6 +682,69 @@ to plot the first as a ratio of the second. Useful for error rates: ], asRatio: true +== CONTAINERS AND TABS == + +Optional dashboard organization layer. Group related tiles visually with +optional tab bars. Pass on hyperdx_save_dashboard as a top-level "containers" +array; reference containers from tiles via "containerId" (and "tabId" when +the container has tabs). + +Container shape: + { id, title, collapsed, collapsible?, bordered?, tabs? } + + id, title : required, non-empty strings. + collapsed : required boolean. Starts collapsed when true. + collapsible : optional boolean. Defaults to true. When false the group + header has no toggle. + bordered : optional boolean. Defaults to true. + tabs : optional array of { id, title }. Zero or one tab renders + as a plain group header; two or more render as a tab bar. + +Tile fields (on the same tile shape used in tiles[]): + containerId : optional. Must match an id in containers[]. + tabId : optional. Must match a tab id on that container. Requires + containerId. A tile with containerId set but no tabId + renders in the container shell (above any tab bar). + +Rules enforced by the API: + - Container ids must be unique on a dashboard. + - Tab ids must be unique within a container. + - A tile's containerId must reference a real container in the array. + - A tile's tabId must reference a real tab on that container. + - A tile's tabId requires containerId to be set. + +Example: + hyperdx_save_dashboard({ + name: "Service Overview", + containers: [ + { + id: "service-health", + title: "Service Health", + collapsed: false, + tabs: [ + { id: "errors", title: "Errors" }, + { id: "latency", title: "Latency" } + ] + }, + { id: "overview", title: "Overview", collapsed: true } + ], + tiles: [ + { name: "Error Rate", containerId: "service-health", tabId: "errors", + config: { displayType: "line", sourceId: "", select: [{ aggFn: "count" }] } }, + { name: "Throughput", containerId: "overview", + config: { displayType: "line", sourceId: "", select: [{ aggFn: "count" }] } }, + { name: "Latest Logs", + config: { displayType: "search", sourceId: "" } } + ] + }) + +Rejection messages on bad inputs (verbatim): + Tile references unknown containerId "X" : tile.containerId not in array + Tile references unknown tabId "X" in container "Y": tile.tabId not on the container + tabId requires containerId to be set : tile has tabId but no containerId + Container IDs must be unique: "X" : duplicate container.id + Duplicate tab id "X" in container "Y" : duplicate tab.id inside a container + == COMMON MISTAKES == 1. Using valueExpression with aggFn "count" diff --git a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts index c7ce7ef5cf..949ef72048 100644 --- a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts @@ -1,3 +1,4 @@ +import type { DashboardContainer } from '@hyperdx/common-utils/dist/types'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { uniq } from 'lodash'; import mongoose from 'mongoose'; @@ -7,6 +8,7 @@ import * as config from '@/config'; import Dashboard from '@/models/dashboard'; import { cleanupDashboardAlerts, + collectTileContainerRefIssues, convertExternalFiltersToInternal, convertExternalTilesToInternal, convertToExternalDashboard, @@ -20,7 +22,7 @@ import type { ExternalDashboardTileWithId } from '@/utils/zod'; import { withToolTracing } from '../../utils/tracing'; import type { McpContext } from '../types'; -import { mcpTilesParam } from './schemas'; +import { mcpContainersParam, mcpTilesParam } from './schemas'; export function registerSaveDashboard( server: McpServer, @@ -49,12 +51,19 @@ export function registerSaveDashboard( name: z.string().describe('Dashboard name'), tiles: mcpTilesParam, tags: z.array(z.string()).optional().describe('Dashboard tags'), + containers: mcpContainersParam.optional(), }), }, withToolTracing( 'hyperdx_save_dashboard', context, - async ({ id: dashboardId, name, tiles: inputTiles, tags }) => { + async ({ + id: dashboardId, + name, + tiles: inputTiles, + tags, + containers, + }) => { if (!dashboardId) { return createDashboard({ teamId, @@ -62,6 +71,7 @@ export function registerSaveDashboard( name, inputTiles, tags, + containers, }); } return updateDashboard({ @@ -71,6 +81,7 @@ export function registerSaveDashboard( name, inputTiles, tags, + containers, }); }, ), @@ -85,17 +96,20 @@ async function createDashboard({ name, inputTiles, tags, + containers, }: { teamId: string; frontendUrl: string | undefined; name: string; inputTiles: unknown[]; tags: string[] | undefined; + containers: DashboardContainer[] | undefined; }) { const parsed = createDashboardBodySchema.safeParse({ name, tiles: inputTiles, tags, + containers, }); if (!parsed.success) { return { @@ -109,9 +123,29 @@ async function createDashboard({ }; } - const { tiles, filters } = parsed.data; + 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('; ')}`, + }, + ], + }; + } + const [missingSources, missingConnections] = await Promise.all([ getMissingSources(teamId, tilesWithId, filters), getMissingConnections(teamId, tilesWithId), @@ -155,6 +189,7 @@ async function createDashboard({ savedQueryLanguage: normalizedSavedQueryLanguage, savedFilterValues: parsed.data.savedFilterValues, team: teamId, + ...(parsedContainers !== undefined ? { containers: parsedContainers } : {}), }).save(); return { @@ -186,6 +221,7 @@ async function updateDashboard({ name, inputTiles, tags, + containers, }: { teamId: string; frontendUrl: string | undefined; @@ -193,6 +229,7 @@ async function updateDashboard({ name: string; inputTiles: unknown[]; tags: string[] | undefined; + containers: DashboardContainer[] | undefined; }) { if (!mongoose.Types.ObjectId.isValid(dashboardId)) { return { @@ -205,6 +242,7 @@ async function updateDashboard({ name, tiles: inputTiles, tags, + containers, }); if (!parsed.success) { return { @@ -218,7 +256,7 @@ async function updateDashboard({ }; } - const { tiles, filters } = parsed.data; + const { tiles, filters, containers: parsedContainers } = parsed.data; const tilesWithId = tiles as ExternalDashboardTileWithId[]; const [missingSources, missingConnections] = await Promise.all([ @@ -250,7 +288,7 @@ async function updateDashboard({ const existingDashboard = await Dashboard.findOne( { _id: dashboardId, team: teamId }, - { tiles: 1, filters: 1 }, + { tiles: 1, filters: 1, containers: 1 }, ).lean(); if (!existingDashboard) { @@ -260,6 +298,30 @@ 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('; ')}`, + }, + ], + }; + } + const existingTileIds = new Set( (existingDashboard.tiles ?? []).map((t: { id: string }) => t.id), ); @@ -297,6 +359,10 @@ async function updateDashboard({ setPayload.savedFilterValues = parsed.data.savedFilterValues; } + if (parsedContainers !== undefined) { + setPayload.containers = parsedContainers; + } + const updatedDashboard = await Dashboard.findOneAndUpdate( { _id: dashboardId, team: teamId }, { $set: setPayload }, diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index b3aaffe044..fb6ce3c8b2 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -1,5 +1,8 @@ import { AggregateFunctionSchema, + DASHBOARD_CONTAINER_ID_MAX, + DASHBOARD_MAX_CONTAINERS, + DashboardContainerSchema, SearchConditionLanguageSchema, } from '@hyperdx/common-utils/dist/types'; import { z } from 'zod'; @@ -87,6 +90,26 @@ const mcpTileLayoutSchema = z.object({ .max(36) .optional() .describe('Tile ID (auto-generated if omitted)'), + containerId: z + .string() + .min(1) + .max(DASHBOARD_CONTAINER_ID_MAX) + .optional() + .describe( + 'Container this tile belongs to. Must reference the id of a container in the ' + + 'dashboard-level containers array. Omit to render the tile in the ungrouped area.', + ), + tabId: z + .string() + .min(1) + .max(DASHBOARD_CONTAINER_ID_MAX) + .optional() + .describe( + 'Tab within the container this tile belongs to. Requires containerId to be set ' + + "and must match a tab id on that container. Omit to render in the container's shell " + + '(useful when the container has zero or one tabs, or when a tile should appear above ' + + 'the tab bar).', + ), }); const mcpLineTileSchema = mcpTileLayoutSchema.extend({ @@ -325,3 +348,27 @@ export const mcpTilesParam = z '"select": [{ "aggFn": "quantile", "level": 0.95, "valueExpression": "Duration" }], ' + '"numberFormat": { "output": "time", "factor": 0.000000001 } } }', ); + +export const mcpContainersParam = z + .array(DashboardContainerSchema) + .max(DASHBOARD_MAX_CONTAINERS) + .describe( + 'Optional dashboard organization layer. Each container groups one or more tiles ' + + 'visually and may carry a tab bar. Tiles join a container by setting tile.containerId; ' + + 'tiles further select a tab by setting tile.tabId.\n\n' + + 'Rules:\n' + + '- Container ids must be unique on the dashboard.\n' + + '- Tab ids must be unique within a container.\n' + + '- A tile.containerId must reference a container id in this array.\n' + + '- A tile.tabId must reference a tab id on the same container.\n' + + '- tile.tabId requires tile.containerId.\n\n' + + 'Container shape: { id, title, collapsed, collapsible?, bordered?, tabs? }. ' + + '`collapsed` is required; `collapsible` and `bordered` default to true when omitted. ' + + 'Two or more tabs render as a tab bar; zero or one tab renders as a plain group header.\n\n' + + 'Example:\n' + + '[\n' + + ' { "id": "service-health", "title": "Service Health", "collapsed": false,\n' + + ' "tabs": [ { "id": "errors", "title": "Errors" }, { "id": "latency", "title": "Latency" } ] },\n' + + ' { "id": "overview", "title": "Overview", "collapsed": true }\n' + + ']', + ); From 215c30c0ffe4b64cf3bf52a513265a5063467c04 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 12 May 2026 03:51:45 +0000 Subject: [PATCH 2/2] fix(mcp): address deep-review feedback on containers/tabs PR - Type `setPayload` in the update path as `Partial` so field name typos and wrong shapes fail at compile time, mirroring the v2 PUT handler's tightening. - Type the test lookup maps as `Record` so a typo on `containerId` / `tabId` would fail typecheck instead of silently asserting `undefined`. - Add an MCP integration test that exercises the `effectiveContainers = parsedContainers ?? existingDashboard.containers ?? []` fallback in the update path: create with containers, PUT without the `containers` field, assert the persisted containers and the tile container/tab refs round-trip. - Add a test for the `containers: []` wipe path: PUT with an empty array clears the persisted containers, and the response normalizes back to absent on read (matches v2 semantics). - Add a 256-char accept / 257-char reject boundary test for tile `containerId`, mirroring the v2 boundary test on `DASHBOARD_CONTAINER_ID_MAX`. - Extend the round-trip test to cover a tile inside a multi-tab container with no `tabId`: the tile renders in the container shell and round-trips with `containerId` set and `tabId` absent. - Rephrase the prompt's "rejection messages" section to describe the validation categories and error path shape instead of the verbatim wording, so a future reword of the helpers does not silently lie to LLM consumers. - Fix the `mcpContainersParam.describe` text to accurately reflect that `collapsible` / `bordered` are persisted absent when omitted (the renderer, not the schema, treats absence as `true`). Co-Authored-By: Claude Opus 4.7 --- .../api/src/mcp/__tests__/dashboards.test.ts | 144 +++++++++++++++++- .../api/src/mcp/prompts/dashboards/content.ts | 16 +- .../src/mcp/tools/dashboards/saveDashboard.ts | 8 +- .../api/src/mcp/tools/dashboards/schemas.ts | 3 +- 4 files changed, 156 insertions(+), 15 deletions(-) diff --git a/packages/api/src/mcp/__tests__/dashboards.test.ts b/packages/api/src/mcp/__tests__/dashboards.test.ts index 8c563d4dcf..94a23e5bad 100644 --- a/packages/api/src/mcp/__tests__/dashboards.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards.test.ts @@ -11,6 +11,7 @@ import { import Connection from '@/models/connection'; import Dashboard from '@/models/dashboard'; import { Source } from '@/models/source'; +import type { ExternalDashboardTileWithId } from '@/utils/zod'; import { McpContext } from '../tools/types'; import { callTool, createTestClient, getFirstText } from './mcpTestUtils'; @@ -482,6 +483,14 @@ describe('MCP Dashboard Tools', () => { 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', @@ -496,9 +505,12 @@ describe('MCP Dashboard Tools', () => { const created = JSON.parse(getFirstText(createResult)); expect(created.containers).toEqual(containers); - const createdTilesByName: Record = Object.fromEntries( - created.tiles.map((t: { name: string }) => [t.name, t]), - ); + // 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', @@ -507,6 +519,12 @@ describe('MCP Dashboard Tools', () => { 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', }); @@ -550,6 +568,7 @@ describe('MCP Dashboard Tools', () => { tiles: [ createdTilesByName['In Group, Tab A'], createdTilesByName['In Group, Tab B'], + createdTilesByName['In Tabbed Group, No Tab'], droppedContainerTile, reHomedUngrouped, ], @@ -560,14 +579,21 @@ describe('MCP Dashboard Tools', () => { expect(updateResult.isError).toBeFalsy(); const updated = JSON.parse(getFirstText(updateResult)); expect(updated.containers).toEqual(updatedContainers); - const updatedTilesByName: Record = Object.fromEntries( - updated.tiles.map((t: { name: string }) => [t.name, t]), - ); + 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 () => { @@ -671,6 +697,112 @@ describe('MCP Dashboard Tools', () => { expect(result.isError).toBe(true); expect(getFirstText(result)).toContain('unknown tabId "ghost"'); }); + + it('should preserve existing containers on update when body omits the field', async () => { + // Exercises the `effectiveContainers = parsedContainers ?? existingDashboard.containers ?? []` + // fallback in the MCP update handler. Without it, a PUT that + // updates only `tiles` would reject because the tile's + // containerId reference would resolve against an empty array. + const sourceId = traceSource._id.toString(); + const containers = [ + { + id: 'service-health', + title: 'Service Health', + collapsed: false, + tabs: [{ id: 'errors', title: 'Errors' }], + }, + ]; + const createResult = await callTool(client, 'hyperdx_save_dashboard', { + name: 'PUT-without-containers fallback', + tiles: [ + buildTile(sourceId, { + name: 'In Group', + containerId: 'service-health', + tabId: 'errors', + }), + ], + containers, + }); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + + const updateResult = await callTool(client, 'hyperdx_save_dashboard', { + id: created.id, + name: 'PUT-without-containers fallback', + tiles: created.tiles, + // containers intentionally omitted + }); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.containers).toEqual(containers); + expect(updated.tiles[0]).toMatchObject({ + containerId: 'service-health', + tabId: 'errors', + }); + }); + + it('should wipe persisted containers when update body sets containers: []', async () => { + // Mirror the v2 behavior: an explicit empty array clears the + // persisted containers, and the response normalizes [] back to + // absent on read. + const sourceId = traceSource._id.toString(); + const createResult = await callTool(client, 'hyperdx_save_dashboard', { + name: 'Wipe containers', + tiles: [buildTile(sourceId, { name: 'Tile' })], + containers: [{ id: 'overview', title: 'Overview', collapsed: false }], + }); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.containers).toHaveLength(1); + + const wipedTile = { + ...created.tiles[0], + containerId: undefined, + tabId: undefined, + }; + const updateResult = await callTool(client, 'hyperdx_save_dashboard', { + id: created.id, + name: 'Wipe containers', + tiles: [wipedTile], + containers: [], + }); + expect(updateResult.isError).toBeFalsy(); + const updated = JSON.parse(getFirstText(updateResult)); + expect(updated.containers).toBeUndefined(); + + const getResult = await callTool(client, 'hyperdx_get_dashboard', { + id: created.id, + }); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.containers).toBeUndefined(); + }); + + // Bounds mirror DASHBOARD_CONTAINER_ID_MAX (256) on the tile-level + // containerId / tabId. 256 chars must accept; 257 must reject. + // 257 trips the inputSchema's `.max(256)` and surfaces back as the + // MCP SDK's "Input validation error" envelope. + it('should accept a 256-char tile.containerId and reject 257', async () => { + const sourceId = traceSource._id.toString(); + const idAtMax = 'c'.repeat(256); + const okResult = await callTool(client, 'hyperdx_save_dashboard', { + name: '256-char containerId', + tiles: [buildTile(sourceId, { containerId: idAtMax })], + containers: [{ id: idAtMax, title: 'Max', collapsed: false }], + }); + expect(okResult.isError).toBeFalsy(); + + const idTooLong = 'c'.repeat(257); + const tooLongResult = await callTool(client, 'hyperdx_save_dashboard', { + name: '257-char containerId', + tiles: [buildTile(sourceId, { containerId: idTooLong })], + containers: [{ id: idTooLong, title: 'Too long', collapsed: false }], + }); + expect(tooLongResult.isError).toBe(true); + expect(getFirstText(tooLongResult)).toContain( + 'String must contain at most 256 character(s)', + ); + expect(getFirstText(tooLongResult)).toContain('containerId'); + }); }); describe('hyperdx_delete_dashboard', () => { diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index cb73cce89c..3a2053e9cd 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -738,12 +738,16 @@ Example: ] }) -Rejection messages on bad inputs (verbatim): - Tile references unknown containerId "X" : tile.containerId not in array - Tile references unknown tabId "X" in container "Y": tile.tabId not on the container - tabId requires containerId to be set : tile has tabId but no containerId - Container IDs must be unique: "X" : duplicate container.id - Duplicate tab id "X" in container "Y" : duplicate tab.id inside a container +Validation categories on bad inputs: + - tile.containerId references a missing container id + - tile.tabId references a missing tab id on that container + - tile.tabId set without tile.containerId + - duplicate container.id within the dashboard + - duplicate tab.id within a single container + +The server returns a descriptive error string identifying the failing path +(tiles[i].containerId, tiles[i].tabId, containers[i].id, or +containers[i].tabs[j].id). Read the path to know which input to fix. == COMMON MISTAKES == diff --git a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts index 949ef72048..523b0241a2 100644 --- a/packages/api/src/mcp/tools/dashboards/saveDashboard.ts +++ b/packages/api/src/mcp/tools/dashboards/saveDashboard.ts @@ -5,7 +5,7 @@ import mongoose from 'mongoose'; import { z } from 'zod'; import * as config from '@/config'; -import Dashboard from '@/models/dashboard'; +import Dashboard, { IDashboard } from '@/models/dashboard'; import { cleanupDashboardAlerts, collectTileContainerRefIssues, @@ -334,7 +334,11 @@ async function updateDashboard({ existingTileIds, ); - const setPayload: Record = { + // Typed as `Partial` (the canonical Mongo doc shape) so + // misnamed or wrong-shape fields fail at compile time, mirroring the + // v2 PUT handler's tightening at + // `routers/external-api/v2/dashboards.ts:2015`. + const setPayload: Partial = { name, tiles: internalTiles, tags: tags && uniq(tags), diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index fb6ce3c8b2..61d0f70859 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -363,7 +363,8 @@ export const mcpContainersParam = z '- A tile.tabId must reference a tab id on the same container.\n' + '- tile.tabId requires tile.containerId.\n\n' + 'Container shape: { id, title, collapsed, collapsible?, bordered?, tabs? }. ' + - '`collapsed` is required; `collapsible` and `bordered` default to true when omitted. ' + + '`collapsed` is required. `collapsible` and `bordered` are optional and ' + + 'persisted absent when omitted; the renderer treats absence as true. ' + 'Two or more tabs render as a tab bar; zero or one tab renders as a plain group header.\n\n' + 'Example:\n' + '[\n' +