diff --git a/.changeset/muster-backend-call-tool-meta.md b/.changeset/muster-backend-call-tool-meta.md new file mode 100644 index 000000000..d22088d4e --- /dev/null +++ b/.changeset/muster-backend-call-tool-meta.md @@ -0,0 +1,5 @@ +--- +'@giantswarm/backstage-plugin-muster-backend': patch +--- + +Fixed the workflow endpoints failing with "tool not found" against real muster servers. The muster aggregator only exposes its meta-tools (`list_tools`, `call_tool`, ...) over MCP, so the proxy now invokes the core workflow tools through the `call_tool` meta-tool and unwraps its result envelope. diff --git a/plugins/muster-backend/src/MusterMcpClient.test.ts b/plugins/muster-backend/src/MusterMcpClient.test.ts index 68c81c2b3..f4c4871d6 100644 --- a/plugins/muster-backend/src/MusterMcpClient.test.ts +++ b/plugins/muster-backend/src/MusterMcpClient.test.ts @@ -135,38 +135,92 @@ describe('MusterMcpClient', () => { return { client, factory }; } - it('parses JSON text content from tool results', async () => { - const execute = jest.fn().mockResolvedValue({ - content: [{ type: 'text', text: '{"workflows":[{"name":"wf-a"}]}' }], + /** + * Wrap a target tool's MCP result the way muster's call_tool meta-tool + * returns it: as a JSON string inside the meta-tool's text content block. + */ + function callToolEnvelope(inner: { + content: { type: string; text: string }[]; + isError?: boolean; + }) { + return { + content: [{ type: 'text', text: JSON.stringify(inner) }], isError: false, - }); + }; + } + + it('invokes workflow tools through the call_tool meta-tool', async () => { + const execute = jest.fn().mockResolvedValue( + callToolEnvelope({ + content: [{ type: 'text', text: '{"workflows":[{"name":"wf-a"}]}' }], + isError: false, + }), + ); const { client } = buildClient(execute); const result = await client.callTool('core_workflow_list', {}); expect(result).toEqual({ workflows: [{ name: 'wf-a' }] }); expect(execute).toHaveBeenCalledWith( - {}, + { name: 'core_workflow_list', arguments: {} }, expect.objectContaining({ toolCallId: expect.any(String) }), ); }); - it('throws on tool-level errors', async () => { + it('forwards tool arguments inside the call_tool payload', async () => { + const execute = jest + .fn() + .mockResolvedValue( + callToolEnvelope({ content: [{ type: 'text', text: '{}' }] }), + ); + const { client } = buildClient(execute); + + await client.callTool('core_workflow_execution_list', { + workflow_name: 'wf-a', + limit: 5, + }); + + expect(execute).toHaveBeenCalledWith( + { + name: 'core_workflow_execution_list', + arguments: { workflow_name: 'wf-a', limit: 5 }, + }, + expect.anything(), + ); + }); + + it('throws on meta-tool errors', async () => { const execute = jest.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'workflow not found' }], + content: [{ type: 'text', text: "tool 'core_workflow_get' not found" }], isError: true, }); const { client } = buildClient(execute); + await expect( + client.callTool('core_workflow_get', { name: 'missing' }), + ).rejects.toThrow("tool 'core_workflow_get' not found"); + }); + + it('throws on target tool errors inside the envelope', async () => { + const execute = jest.fn().mockResolvedValue( + callToolEnvelope({ + content: [{ type: 'text', text: 'workflow not found' }], + isError: true, + }), + ); + const { client } = buildClient(execute); + await expect( client.callTool('core_workflow_get', { name: 'missing' }), ).rejects.toThrow('workflow not found'); }); - it('returns raw text when the payload is not JSON', async () => { - const execute = jest.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'plain text' }], - }); + it('returns raw text when the inner payload is not JSON', async () => { + const execute = jest.fn().mockResolvedValue( + callToolEnvelope({ + content: [{ type: 'text', text: 'plain text' }], + }), + ); const { client } = buildClient(execute); await expect(client.callTool('core_workflow_list', {})).resolves.toBe( @@ -174,7 +228,19 @@ describe('MusterMcpClient', () => { ); }); - it('reports a clean error when a tool has no executor', async () => { + it('supports servers that return the payload without an envelope', async () => { + const execute = jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: '{"workflows":[{"name":"wf-a"}]}' }], + isError: false, + }); + const { client } = buildClient(execute); + + await expect(client.callTool('core_workflow_list', {})).resolves.toEqual({ + workflows: [{ name: 'wf-a' }], + }); + }); + + it('reports a clean error when call_tool has no executor', async () => { const { client } = buildClient(undefined); await expect(client.callTool('core_workflow_list', {})).rejects.toThrow( @@ -183,9 +249,11 @@ describe('MusterMcpClient', () => { }); it('passes an Authorization header for per-user tokens', async () => { - const execute = jest.fn().mockResolvedValue({ - content: [{ type: 'text', text: '{}' }], - }); + const execute = jest + .fn() + .mockResolvedValue( + callToolEnvelope({ content: [{ type: 'text', text: '{}' }] }), + ); const { client, factory } = buildClient(execute); await client.callTool('core_workflow_list', {}, { authToken: 'token-a' }); @@ -194,9 +262,11 @@ describe('MusterMcpClient', () => { }); it('caches clients per user token', async () => { - const execute = jest.fn().mockResolvedValue({ - content: [{ type: 'text', text: '{}' }], - }); + const execute = jest + .fn() + .mockResolvedValue( + callToolEnvelope({ content: [{ type: 'text', text: '{}' }] }), + ); const { client, factory } = buildClient(execute); await client.callTool('core_workflow_list', {}, { authToken: 'token-a' }); diff --git a/plugins/muster-backend/src/MusterMcpClient.ts b/plugins/muster-backend/src/MusterMcpClient.ts index 95f15d856..f5543c79d 100644 --- a/plugins/muster-backend/src/MusterMcpClient.ts +++ b/plugins/muster-backend/src/MusterMcpClient.ts @@ -10,9 +10,10 @@ import { McpClientCache, } from '@giantswarm/backstage-plugin-gs-node'; -// Muster workflow tools proxied by this plugin. Definitions are passed to -// `toolsFromDefinitions` so we never have to list the (potentially huge) -// aggregated muster tool catalog just to call four core tools. +// Muster workflow tools proxied by this plugin. The muster aggregator only +// exposes its meta-tools (list_tools, call_tool, ...) over MCP; concrete +// tools like core_workflow_list must be invoked through the `call_tool` +// meta-tool with the target tool name and arguments. const WORKFLOW_TOOL_NAMES = [ 'core_workflow_list', 'core_workflow_get', @@ -22,6 +23,9 @@ const WORKFLOW_TOOL_NAMES = [ export type WorkflowToolName = (typeof WORKFLOW_TOOL_NAMES)[number]; +/** Muster meta-tool used to execute any aggregated tool by name. */ +const CALL_TOOL = 'call_tool'; + const CACHE_KEY_SERVER_NAME = 'muster'; export interface MusterServerConfig { @@ -126,32 +130,33 @@ export class MusterMcpClient { ? { ...this.server.headers, Authorization: `Bearer ${authToken}` } : this.server.headers; + if (!WORKFLOW_TOOL_NAMES.includes(toolName)) { + throw new NotFoundError(`Unknown muster tool: ${toolName}`); + } + const client = await this.cache.getOrCreate(cacheKey, () => this.clientFactory(headers), ); const tools = client.toolsFromDefinitions({ - tools: WORKFLOW_TOOL_NAMES.map(name => ({ - name, - inputSchema: { type: 'object' as const }, - })), + tools: [{ name: CALL_TOOL, inputSchema: { type: 'object' as const } }], }); - const tool = tools[toolName]; - if (!tool) { - throw new NotFoundError(`Unknown muster tool: ${toolName}`); - } - if (typeof tool.execute !== 'function') { + const tool = tools[CALL_TOOL]; + if (!tool || typeof tool.execute !== 'function') { throw new ServiceUnavailableError( - `Muster tool ${toolName} has no executor`, + `Muster meta-tool ${CALL_TOOL} has no executor`, ); } let result; try { - result = await tool.execute(args, { - toolCallId: `muster-backend-${toolName}`, - messages: [], - }); + result = await tool.execute( + { name: toolName, arguments: args }, + { + toolCallId: `muster-backend-${toolName}`, + messages: [], + }, + ); } catch (error) { if (isClosedClientError(error)) { this.logger.warn( @@ -170,11 +175,13 @@ export class MusterMcpClient { } /** - * Muster returns JSON payloads serialized into MCP text content blocks. - * Unwrap the first text block and parse it; tool-level errors (isError) - * surface as exceptions with the error text. + * Unwrap an MCP tool result's first text content block. Tool-level errors + * (isError) surface as exceptions with the error text. */ - private parseResult(result: unknown, toolName: string): unknown { + private unwrapTextContent( + result: unknown, + toolName: string, + ): string | undefined { const { content, isError } = (result ?? {}) as { content?: ContentItem[]; isError?: boolean; @@ -188,6 +195,40 @@ export class MusterMcpClient { ); } + return text; + } + + /** + * The `call_tool` meta-tool wraps the target tool's MCP result as a JSON + * string inside its own text content block, so unwrap twice: first the + * meta-tool envelope, then the target tool's result, whose text payload is + * the JSON document the workflow endpoints return. + */ + private parseResult(result: unknown, toolName: string): unknown { + const envelope = this.unwrapTextContent(result, toolName); + if (envelope === undefined) { + return undefined; + } + + let inner: unknown; + try { + inner = JSON.parse(envelope); + } catch { + // Not a call_tool envelope; treat it as the tool's direct payload. + return envelope; + } + + if ( + inner === null || + typeof inner !== 'object' || + !Array.isArray((inner as { content?: unknown }).content) + ) { + // Direct JSON payload from a server that exposes tools without the + // call_tool envelope. + return inner; + } + + const text = this.unwrapTextContent(inner, toolName); if (text === undefined) { return undefined; }