fix(muster): support per-user auth and address workflow plugin review feedback

.changeset/muster-review-followup.md

@@ -0,0 +1,22 @@
+---
+'@giantswarm/backstage-plugin-muster': minor
+'@giantswarm/backstage-plugin-muster-backend': minor
+'@giantswarm/backstage-plugin-ai-chat-backend': patch
+'@giantswarm/backstage-plugin-gs-node': minor
+'app': patch
+'backend': patch
+---
+
+Support muster MCP servers behind per-user auth (`authProvider` entries in
+`aiChat.mcp`): the muster frontend now forwards the user's OAuth token to the
+muster-backend proxy, which opens per-user MCP sessions. Previously such
+servers were reported as unconfigured and the Workflows page failed with a 503.
+
+Also addresses review feedback on the initial muster plugins: the shared MCP
+client cache moved from ai-chat-backend to `@giantswarm/backstage-plugin-gs-node`
+and is reused by muster-backend; config parsing no longer throws on unnamed
+`aiChat.mcp` entries; muster-backend uses `@backstage/errors` classes instead
+of a hand-rolled error middleware; query parameter validation rejects empty
+and repeated values; execution fetch errors are surfaced in the UI instead of
+being silently swallowed; duplicate workflow step ids no longer drop nodes
+from the graph; and `formatDuration` is shared instead of copy-pasted.

packages/app/src/App.tsx

@@ -11,6 +11,7 @@ import { fluxPluginOverrides } from './modules/flux';
 import aiChatPlugin from '@giantswarm/backstage-plugin-ai-chat';
 import { aiChatPluginOverrides } from './modules/ai-chat';
 import musterPlugin from '@giantswarm/backstage-plugin-muster';
+import { musterPluginOverrides } from './modules/muster';
 
 // Upstream NFS plugins:
 import catalogPlugin from '@backstage/plugin-catalog/alpha';
@@ -53,6 +54,7 @@ const app = createApp({
     aiChatPlugin,
     aiChatPluginOverrides,
     musterPlugin,
+    musterPluginOverrides,
 
     // Upstream NFS plugins:
     catalogPlugin,

packages/app/src/modules/muster/MusterApiOverride.ts

@@ -0,0 +1,19 @@
+import { ApiBlueprint } from '@backstage/frontend-plugin-api';
+import {
+  musterAuthProvidersApiRef,
+  MusterAuthProviders,
+} from '@giantswarm/backstage-plugin-muster';
+import { gsAuthProvidersApiRef } from '@giantswarm/backstage-plugin-gs';
+
+export const MusterApiOverride = ApiBlueprint.make({
+  name: 'auth-providers',
+  params: defineParams =>
+    defineParams({
+      api: musterAuthProvidersApiRef,
+      deps: { gsAuthProvidersApi: gsAuthProvidersApiRef },
+      factory: ({ gsAuthProvidersApi }) => {
+        const authProviders = gsAuthProvidersApi.getMCPAuthApis();
+        return new MusterAuthProviders(authProviders);
+      },
+    }),
+});

packages/app/src/modules/muster/index.ts

@@ -0,0 +1,7 @@
+import { createFrontendModule } from '@backstage/frontend-plugin-api';
+import { MusterApiOverride } from './MusterApiOverride';
+
+export const musterPluginOverrides = createFrontendModule({
+  pluginId: 'muster',
+  extensions: [MusterApiOverride],
+});

plugins/ai-chat-backend/package.json

@@ -34,6 +34,7 @@
     "@backstage/backend-plugin-api": "backstage:^",
     "@backstage/config": "backstage:^",
     "@backstage/errors": "backstage:^",
+    "@giantswarm/backstage-plugin-gs-node": "workspace:^",
     "ai": "^6.0.176",
     "express": "^5.0.0",
     "express-promise-router": "^4.1.0",

plugins/ai-chat-backend/src/getMcpTools.ts

@@ -6,7 +6,10 @@ import {
   MCPClient,
 } from '@ai-sdk/mcp';
 import { AuthTokens } from './utils';
-import { isClosedClientError, McpClientCache } from './McpClientCache';
+import {
+  isClosedClientError,
+  McpClientCache,
+} from '@giantswarm/backstage-plugin-gs-node';
 
 interface McpServerConfig {
   url: string;

plugins/ai-chat-backend/src/router.ts

@@ -23,7 +23,7 @@ import express from 'express';
 import Router from 'express-promise-router';
 import { readFileSync } from 'fs';
 import { getMcpTools } from './getMcpTools';
-import { McpClientCache } from './McpClientCache';
+import { McpClientCache } from '@giantswarm/backstage-plugin-gs-node';
 import { frontendTools } from './frontendTools';
 import {
   createSkillTools,

plugins/gs-node/package.json

@@ -24,6 +24,7 @@
     "postpack": "backstage-cli package postpack"
   },
   "dependencies": {
+    "@ai-sdk/mcp": "^1.0.41",
     "@backstage/backend-plugin-api": "backstage:^",
     "@backstage/errors": "backstage:^",
     "@backstage/types": "backstage:^",

plugins/gs-node/src/index.ts

@@ -1 +1,2 @@
+export * from './mcp';
 export * from './registry';

plugins/gs-node/src/mcp/index.ts

@@ -0,0 +1 @@
+export { McpClientCache, isClosedClientError } from './McpClientCache';

plugins/muster-backend/package.json

@@ -29,10 +29,12 @@
     "@backstage/backend-plugin-api": "backstage:^",
     "@backstage/config": "backstage:^",
     "@backstage/errors": "backstage:^",
+    "@giantswarm/backstage-plugin-gs-node": "workspace:^",
     "express": "^5.0.0",
     "express-promise-router": "^4.1.0"
   },
   "devDependencies": {
+    "@backstage/backend-defaults": "backstage:^",
     "@backstage/backend-test-utils": "backstage:^",
     "@backstage/cli": "backstage:^",
     "@types/express": "^5.0.0",

plugins/muster-backend/src/MusterMcpClient.test.ts

@@ -29,6 +29,7 @@ describe('readMusterServerFromConfig', () => {
     expect(readMusterServerFromConfig(config, logger)).toEqual({
       url: 'http://muster:8090/mcp',
       headers: { 'X-Custom': 'value' },
+      authProvider: undefined,
     });
   });
 
@@ -45,18 +46,60 @@ describe('readMusterServerFromConfig', () => {
     expect(readMusterServerFromConfig(config, logger)).toEqual({
       url: 'http://muster-prod/mcp',
       headers: undefined,
+      authProvider: undefined,
     });
   });
 
-  it('rejects entries that require per-user auth', () => {
+  it('tolerates entries without a name', () => {
+    const config = mockServices.rootConfig({
+      data: {
+        aiChat: {
+          mcp: [
+            { url: 'http://anonymous/mcp' },
+            { name: 'muster', url: 'http://muster/mcp' },
+          ],
+        },
+      },
+    });
+
+    expect(readMusterServerFromConfig(config, logger)).toEqual({
+      url: 'http://muster/mcp',
+      headers: undefined,
+      authProvider: undefined,
+    });
+  });
+
+  it('passes through the authProvider for per-user auth', () => {
+    const config = mockServices.rootConfig({
+      data: {
+        aiChat: {
+          mcp: [
+            {
+              name: 'muster',
+              url: 'http://muster/mcp',
+              authProvider: 'mcp-muster',
+            },
+          ],
+        },
+      },
+    });
+
+    expect(readMusterServerFromConfig(config, logger)).toEqual({
+      url: 'http://muster/mcp',
+      headers: undefined,
+      authProvider: 'mcp-muster',
+    });
+  });
+
+  it('rejects entries with useBackstageUserToken', () => {
     const config = mockServices.rootConfig({
       data: {
         aiChat: {
           mcp: [
             {
               name: 'muster',
               url: 'http://muster/mcp',
-              authProvider: 'gs',
+              useBackstageUserToken: true,
             },
           ],
         },
@@ -70,26 +113,34 @@ describe('readMusterServerFromConfig', () => {
 describe('MusterMcpClient', () => {
   const logger = mockServices.logger.mock();
 
-  function buildClient(execute: jest.Mock) {
-    const mcpClient = {
+  function buildMcpClient(execute: jest.Mock | undefined) {
+    return {
       toolsFromDefinitions: jest.fn(
         ({ tools }: { tools: { name: string }[] }) =>
           Object.fromEntries(tools.map(tool => [tool.name, { execute }])),
       ),
       close: jest.fn(),
     } as unknown as MCPClient;
+  }
 
-    return new MusterMcpClient({ url: 'http://muster/mcp' }, logger, () =>
-      Promise.resolve(mcpClient),
+  function buildClient(execute: jest.Mock | undefined) {
+    const factory = jest.fn((_headers: Record<string, string> | undefined) =>
+      Promise.resolve(buildMcpClient(execute)),
+    );
+    const client = new MusterMcpClient(
+      { url: 'http://muster/mcp' },
+      logger,
+      factory,
     );
+    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"}]}' }],
       isError: false,
     });
-    const client = buildClient(execute);
+    const { client } = buildClient(execute);
 
     const result = await client.callTool('core_workflow_list', {});
 
@@ -105,7 +156,7 @@ describe('MusterMcpClient', () => {
       content: [{ type: 'text', text: 'workflow not found' }],
       isError: true,
     });
-    const client = buildClient(execute);
+    const { client } = buildClient(execute);
 
     await expect(
       client.callTool('core_workflow_get', { name: 'missing' }),
@@ -116,10 +167,42 @@ describe('MusterMcpClient', () => {
     const execute = jest.fn().mockResolvedValue({
       content: [{ type: 'text', text: 'plain text' }],
     });
-    const client = buildClient(execute);
+    const { client } = buildClient(execute);
 
     await expect(client.callTool('core_workflow_list', {})).resolves.toBe(
       'plain text',
     );
   });
+
+  it('reports a clean error when a tool has no executor', async () => {
+    const { client } = buildClient(undefined);
+
+    await expect(client.callTool('core_workflow_list', {})).rejects.toThrow(
+      'has no executor',
+    );
+  });
+
+  it('passes an Authorization header for per-user tokens', async () => {
+    const execute = jest.fn().mockResolvedValue({
+      content: [{ type: 'text', text: '{}' }],
+    });
+    const { client, factory } = buildClient(execute);
+
+    await client.callTool('core_workflow_list', {}, { authToken: 'token-a' });
+
+    expect(factory).toHaveBeenCalledWith({ Authorization: 'Bearer token-a' });
+  });
+
+  it('caches clients per user token', async () => {
+    const execute = jest.fn().mockResolvedValue({
+      content: [{ type: 'text', text: '{}' }],
+    });
+    const { client, factory } = buildClient(execute);
+
+    await client.callTool('core_workflow_list', {}, { authToken: 'token-a' });
+    await client.callTool('core_workflow_list', {}, { authToken: 'token-a' });
+    await client.callTool('core_workflow_list', {}, { authToken: 'token-b' });
+
+    expect(factory).toHaveBeenCalledTimes(2);
+  });
 });