Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@tableau/mcp-server",
"description": "Helping agents see and understand data.",
"version": "2.63.8",
"version": "2.63.9",
"repository": {
"type": "git",
"url": "git+https://github.com/tableau/tableau-mcp.git"
Expand Down
29 changes: 29 additions & 0 deletions src/desktop/metadata/dashboards.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ const WORKBOOK_WITH_USER_NAMESPACE = `<?xml version='1.0' encoding='utf-8' ?>
</dashboards>
</workbook>`;

// A story serializes as a `<dashboard type='storyboard'>` sharing the <dashboards> container with a
// real dashboard; the dashboard resolvers must ignore the story.
const WORKBOOK_WITH_STORYBOARD = `<?xml version='1.0' encoding='utf-8' ?>
<workbook>
<dashboards>
<dashboard name='Overview'><zones /><simple-id uuid='{DB-0001}' /></dashboard>
<dashboard name='QBR Story' type='storyboard'><zones /><simple-id uuid='{ST-0001}' /></dashboard>
</dashboards>
</workbook>`;

describe('extractDashboardXml', () => {
it('finds and extracts an existing dashboard', () => {
const xml = extractDashboardXml(WORKBOOK_WITH_USER_NAMESPACE, 'Overview');
Expand All @@ -36,6 +46,12 @@ describe('extractDashboardXml', () => {
expect(extractDashboardXml(WORKBOOK_WITH_USER_NAMESPACE, 'Does Not Exist')).toBeNull();
});

it('returns null for a storyboard — a story is not a dashboard', () => {
expect(extractDashboardXml(WORKBOOK_WITH_STORYBOARD, 'QBR Story')).toBeNull();
// The real dashboard sharing the container still extracts.
expect(extractDashboardXml(WORKBOOK_WITH_STORYBOARD, 'Overview')).toContain('name="Overview"');
});

// Same live-bug shape as extractSheetXml (sheets.test.ts): an untouched get-dashboard-xml ->
// apply-dashboard round-trip must pass the same well-formed-xml preflight apply-dashboard runs.
it('carries the xmlns:user declaration from the workbook root onto the extracted dashboard', () => {
Expand Down Expand Up @@ -159,6 +175,19 @@ describe('listDashboardRefs / resolveDashboardRef', () => {
it('returns null when neither an id nor a name matches', () => {
expect(resolveDashboardRef(WORKBOOK, 'No Such Dashboard')).toBeNull();
});

it('excludes storyboards: a story is neither listed nor resolvable as a dashboard', () => {
expect(listDashboardRefs(WORKBOOK_WITH_STORYBOARD)).toEqual([
{ id: '{DB-0001}', name: 'Overview' },
]);
expect(resolveDashboardRef(WORKBOOK_WITH_STORYBOARD, 'QBR Story')).toBeNull();
expect(resolveDashboardRef(WORKBOOK_WITH_STORYBOARD, '{ST-0001}')).toBeNull();
// The real dashboard sharing the container still resolves.
expect(resolveDashboardRef(WORKBOOK_WITH_STORYBOARD, 'Overview')).toEqual({
id: '{DB-0001}',
name: 'Overview',
});
});
});

describe('deleteDashboard', () => {
Expand Down
13 changes: 11 additions & 2 deletions src/desktop/metadata/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,19 @@ export function dashboardFragmentSimpleId(dashboardFragmentXml: string): string
return dashboard?.['simple-id']?.['@_uuid']?.trim() || null;
}

// A story serializes as a `<dashboard type='storyboard'>` inside the same `<dashboards>` container as
// ordinary dashboards, so dashboard enumeration/resolution must skip it or a story leaks in as a dashboard.
function isStoryboard(dashboard: ParsedDashboard): boolean {
return dashboard['@_type'] === 'storyboard';
}

// `id` is each dashboard's own `<simple-id uuid>` — the same value the External Client API returns
// as the dashboard id.
export function listDashboardRefs(workbookXml: string): Array<{ id: string; name: string }> {
const workbook = parseXML(workbookXml);
const dashboards = normalizeArray(workbook.workbook?.dashboards?.dashboard);
return dashboards.flatMap((db) => {
if (isStoryboard(db)) return [];
const id = db['simple-id']?.['@_uuid']?.trim();
const name = db['@_name'];
return id && name ? [{ id, name }] : [];
Expand All @@ -201,7 +208,9 @@ export function resolveDashboardRef(
ref: string,
): { id?: string; name: string } | null {
const workbook = parseXML(workbookXml);
const dashboards = normalizeArray(workbook.workbook?.dashboards?.dashboard);
const dashboards = normalizeArray(workbook.workbook?.dashboards?.dashboard).filter(
(db) => !isStoryboard(db),
);
const trimmed = ref.trim();
const matched =
dashboards.find((db) => db['simple-id']?.['@_uuid']?.trim() === trimmed) ??
Expand All @@ -216,7 +225,7 @@ export function resolveDashboardRef(
export function extractDashboardXml(workbookXml: string, dashboardName: string): string | null {
const workbook = parseXML(workbookXml);
const dashboard = findDashboard(workbook, dashboardName);
if (!dashboard) {
if (!dashboard || isStoryboard(dashboard)) {
return null;
}
carryNamespaceDeclarations(workbook.workbook, dashboard);
Expand Down
22 changes: 22 additions & 0 deletions src/desktop/wrappers/loadDashboardXml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,28 @@ describe('loadDashboardXml (External Client API transport)', () => {
expect(calls.find((c) => c.kind === 'apply')).toBeUndefined();
});

it('names storyboardName (not dashboard_name) when the caller name disagrees with the XML', async () => {
const executor = makeExecutorMock({});

const result = await loadDashboardXml({
dashboardName: 'Wrong Name',
xml: "<dashboard name='QBR Story' type='storyboard'><zones /></dashboard>",
executor,
signal: mockSignal,
focus: NO_FOCUS,
kind: 'storyboard',
requireExistingSheet: true,
});

expect(result.isErr()).toBe(true);
if (result.isErr()) {
invariant(result.error.type === 'load-dashboard-xml-error');
invariant(result.error.error.type === 'name-mismatch');
expect(result.error.error.message).toContain('storyboardName');
expect(result.error.error.message).not.toContain('dashboard_name');
}
});

it('goes straight to the whole-workbook apply for an absent dashboard when requireExistingSheet is off', async () => {
const { executor, calls } = absentDashboardExecutor(['Some Other DB']);

Expand Down
12 changes: 8 additions & 4 deletions src/desktop/wrappers/loadDashboardXml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ type LoadDashboardHelperResult = Result<
function resolveCanonicalDashboardName(
dashboardName: string,
xml: string,
kind: LoadDashboardKind,
): Result<string, Extract<LoadDashboardXmlError, { type: 'name-mismatch' }>> {
const callerRef = dashboardName.trim();
// A storyboard is a `<dashboard>` element too, so the fragment tag and read-cached-xml selector
// stay `dashboard`; only the artifact noun and the tool's name param differ by kind.
const paramName = kind === 'storyboard' ? 'storyboardName' : 'dashboard_name';
let xmlName = '';
let xmlId = '';
let isWorkbookDocument = false;
Expand All @@ -86,7 +90,7 @@ function resolveCanonicalDashboardName(
return Err({
type: 'name-mismatch',
message: isWorkbookDocument
? 'Applying a dashboard needs a single <dashboard name="..."> fragment, but the cached file holds ' +
? `Applying a ${kind} needs a single <dashboard name="..."> fragment, but the cached file holds ` +
`a whole <workbook> document. FIX: read-cached-xml with dashboard="${callerRef}" to pull just ` +
'that element, write-cached-xml with the same selector to splice your edit back, then apply ' +
'that file.'
Expand All @@ -100,8 +104,8 @@ function resolveCanonicalDashboardName(
return Err({
type: 'name-mismatch',
message:
`dashboard_name "${dashboardName}" does not match the <dashboard name> in the XML ("${xmlName}")` +
`${xmlId ? ` or its id ("${xmlId}")` : ''}. FIX: Retry with dashboard_name set to the XML's ` +
`${paramName} "${dashboardName}" does not match the <dashboard name> in the XML ("${xmlName}")` +
`${xmlId ? ` or its id ("${xmlId}")` : ''}. FIX: Retry with ${paramName} set to the XML's ` +
`name "${xmlName}"${xmlId ? ` or id "${xmlId}"` : ''} — or update the <dashboard name> attribute ` +
`in the XML to "${dashboardName}" if the caller name is intended.`,
});
Expand Down Expand Up @@ -176,7 +180,7 @@ export async function loadDashboardXml({

// Require the caller's dashboard_name to agree with the XML root name before apply, then
// thread the validated canonical name through the load.
const canonicalNameResult = resolveCanonicalDashboardName(dashboardName, xml);
const canonicalNameResult = resolveCanonicalDashboardName(dashboardName, xml, kind);
if (canonicalNameResult.isErr()) {
log({
level: 'error',
Expand Down
11 changes: 8 additions & 3 deletions src/server.desktop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,15 @@ async function serializeDesktopToolSurface(tool: DesktopTool<any>): Promise<stri
// image exports it joins DYNAMIC_AUTHORING_TOOL_PROFILE, so it moves both surfaces by +1012:
// dynamic authoring 39_069 -> 40_081 (budget 39_087 -> 40_099, keeping the 18-char slack), full
// surface 55_638 -> 56_650 (budget 55_656 -> 56_668).
const DYNAMIC_AUTHORING_SURFACE_EXPECTED = 40_081;
const DYNAMIC_AUTHORING_SURFACE_BUDGET = 40_099;
// Re-pinned 2026-08-19: get-storyboard-xml aligned to its get-xml siblings (get-dashboard-xml/
// get-worksheet-xml) — storyboardName is now required and the deprecated storyboard alias param is
// dropped; net -51 bytes despite the longer "existing storyboard" description. It is in
// DYNAMIC_AUTHORING_TOOL_PROFILE, so both surfaces shrink: dynamic authoring 40_081 -> 40_030
// (budget 40_099 -> 40_048), full surface 56_650 -> 56_599 (budget 56_668 -> 56_617), 18-char slack kept.
const DYNAMIC_AUTHORING_SURFACE_EXPECTED = 40_030;
const DYNAMIC_AUTHORING_SURFACE_BUDGET = 40_048;
const DYNAMIC_AUTHORING_PRODUCT_CEILING = 46_000;
const FULL_TOOL_SURFACE_BUDGET = 56_668;
const FULL_TOOL_SURFACE_BUDGET = 56_617;

describe('desktop tools/list serialized surface', () => {
it('keeps the served dynamic authoring profile under the tool-search auto-deferral threshold budget', async () => {
Expand Down
45 changes: 3 additions & 42 deletions src/tools/desktop/api/getStoryboardXml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,36 +68,6 @@ describe('getStoryboardXmlTool', () => {
expect(resultObj.storyboardXml).toContain('type="storyboard"');
});

it('accepts the deprecated storyboard alias key', async () => {
const result = await getToolResult({ storyboard: 'QBR Story', mode: 'inline' });

expect(result.isError).toBe(false);
invariant(result.content[0].type === 'text');
const resultObj = z
.object({ storyboardXml: z.string() })
.parse(JSON.parse(result.content[0].text));
expect(resultObj.storyboardXml).toContain('name="QBR Story"');
});

it('errors when both storyboardName and its alias are absent', async () => {
const result = await getToolResult({});

expect(result.isError).toBe(true);
invariant(result.content[0].type === 'text');
expect(result.content[0].text).toContain(
'storyboardName is required (storyboard is a deprecated alias).',
);
});

it('errors when storyboardName and its alias disagree', async () => {
const result = await getToolResult({ storyboardName: 'QBR Story', storyboard: 'Other' });

expect(result.isError).toBe(true);
invariant(result.content[0].type === 'text');
expect(result.content[0].text).toContain('storyboardName ("QBR Story")');
expect(result.content[0].text).toContain('Pass one of them.');
});

it('errors when the document route returns no <dashboard> subtree', async () => {
const result = await getToolResult({ storyboardName: 'QBR Story', emptyDocument: true });

Expand Down Expand Up @@ -210,8 +180,7 @@ function makeExecutor({

async function getToolResult(opts: {
session?: string;
storyboardName?: string;
storyboard?: string;
storyboardName: string;
mode?: 'file' | 'inline';
capBytes?: number;
bigDocument?: string;
Expand All @@ -220,14 +189,7 @@ async function getToolResult(opts: {
emptyDocument?: boolean;
customSignal?: AbortSignal;
}): Promise<CallToolResult> {
const {
session = '12345',
storyboardName,
storyboard,
mode = 'file',
capBytes,
customSignal,
} = opts;
const { session = '12345', storyboardName, mode = 'file', capBytes, customSignal } = opts;
const { executor } = makeExecutor(opts);
const tool = getStoryboardXmlTool(new DesktopMcpServer());
const callback = await Provider.from(tool.callback);
Expand All @@ -240,7 +202,7 @@ async function getToolResult(opts: {
...(customSignal && { signal: customSignal }),
...(capBytes !== undefined && { config: { ...base.config, inlineXmlMaxBytes: capBytes } }),
};
return await callback({ session, storyboardName, storyboard, mode }, extra);
return await callback({ session, storyboardName, mode }, extra);
}

async function getToolCalls(opts: { storyboardName: string; customSignal?: AbortSignal }): Promise<{
Expand All @@ -261,7 +223,6 @@ async function getToolCalls(opts: { storyboardName: string; customSignal?: Abort
{
session: '12345',
storyboardName: opts.storyboardName,
storyboard: undefined,
mode: 'inline',
},
extra,
Expand Down
29 changes: 7 additions & 22 deletions src/tools/desktop/api/getStoryboardXml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,13 @@ import { parseXML } from '../../../desktop/metadata/parser.js';
import { runExternalApiReadTool } from '../../../desktop/wrappers/readHarness.js';
import { UnknownError } from '../../../errors/mcpToolError.js';
import { DesktopMcpServer } from '../../../server.desktop.js';
import {
artifactNameParam,
deprecatedArtifactAliasParam,
resolveArtifactNameArg,
sessionParam,
xmlModeParam,
} from '../params.js';
import { artifactNameParam, sessionParam, xmlModeParam } from '../params.js';
import { DesktopTool } from '../tool.js';
import { finishXmlRead, XmlReadFileResult } from './xmlReadResult.js';

const paramsSchema = {
session: sessionParam(),
storyboardName: artifactNameParam('storyboard').optional(),
storyboard: deprecatedArtifactAliasParam('storyboard'),
storyboardName: artifactNameParam('storyboard'),
mode: xmlModeParam(),
};
const title = 'Get Storyboard Document';
Expand All @@ -33,27 +26,19 @@ export const getStoryboardXmlTool = (
server,
name: 'get-storyboard-xml',
title,
description: 'Return one storyboard document subtree.',
description: 'Get structure for an existing storyboard.',
paramsSchema,
annotations: {
readOnlyHint: false, // Writes to a cache file
destructiveHint: false,
idempotentHint: false,
openWorldHint: false,
},
callback: async (
{ session, storyboardName, storyboard, mode },
extra,
): Promise<CallToolResult> => {
callback: async ({ session, storyboardName, mode }, extra): Promise<CallToolResult> => {
return await getStoryboardXml.logAndExecute<GetStoryboardXmlToolResult>({
extra,
args: { session, storyboardName, storyboard, mode },
args: { session, storyboardName, mode },
callback: async () => {
const nameResult = resolveArtifactNameArg('storyboard', storyboardName, storyboard);
if (nameResult.isErr()) {
return nameResult;
}
const resolvedStoryboardName = nameResult.value;
return await runExternalApiReadTool({
session,
extra,
Expand All @@ -71,7 +56,7 @@ export const getStoryboardXmlTool = (

const storyboardResult = resolveItemByNameOrId(
'Storyboard',
resolvedStoryboardName,
storyboardName,
listResult.value.storyboards ?? [],
);
if (storyboardResult.isErr()) {
Expand All @@ -91,7 +76,7 @@ export const getStoryboardXmlTool = (
const storyboardXml = documentResult.value.xml;
if (!parseXML(storyboardXml).dashboard) {
return new UnknownError(
`No storyboard document subtree found for "${resolvedStoryboardName}".`,
`No storyboard document subtree found for "${storyboardName}".`,
).toErr();
}

Expand Down
Loading
Loading