Handle MCP publish from embedded API metadata (#173)

Copilot · EMaher · web-flow · commit 8632901b190a · 2026-06-19T15:05:28.000-07:00
* fix: update MCP publish handling progress
* fix: finalize MCP publish handling
* test: clarify MCP publish coverage
* fixing round-trip test
* fixing expected structure

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: Elizabeth Maher &lt;enewman@microsoft.com&gt;
diff --git a/src/models/resource-types.ts b/src/models/resource-types.ts
@@ -313,10 +313,12 @@ export const RESOURCE_TYPE_METADATA: Record<ResourceType, ResourceTypeMetadata>
   },
   [ResourceType.McpServer]: {
     // Singleton MCP (Model Context Protocol) server configuration per API.
-    // ARM path: apis/{apiId}/mcpServers/default
+    // Artifact data is embedded in apiInformation.json; retain the descriptor
+    // only for ARM URI/dependency modeling. With infoFile unset, extract/publish
+    // no longer read or write the legacy mcpServerInformation.json sidecar.
     armPathSuffix: 'apis/{0}/mcpServers/default',
     artifactDirectory: 'apis/{0}',
-    infoFile: 'mcpServerInformation.json',
+    infoFile: null, // Embedded in apiInformation.json
     supportsGet: true,
   },
   [ResourceType.Workspace]: {
diff --git a/src/services/api-extractor.ts b/src/services/api-extractor.ts
@@ -40,113 +40,6 @@ export interface ApiExtractionResult {
   policies: string[];
 }
 
-function getApiProperties(apiJson: Record<string, unknown>): Record<string, unknown> | undefined {
-  return apiJson.properties as Record<string, unknown> | undefined;
-}
-
-function hasEmbeddedMcpConfiguration(apiJson: Record<string, unknown>): boolean {
-  const properties = getApiProperties(apiJson);
-  if (!properties) {
-    return false;
-  }
-
-  // Check if mcpTools has actual content (non-empty array), excluding null
-  const mcpTools = properties.mcpTools as unknown[] | undefined | null;
-  if (Array.isArray(mcpTools) && mcpTools.length > 0) {
-    return true;
-  }
-
-  // Check if mcpProperties exists and is not null or undefined
-  if (properties.mcpProperties != null) {
-    return true;
-  }
-
-  // MCP APIs created from an existing MCP server are wired purely via
-  // backendId + (optionally absent) mcpProperties. A non-null backendId on a
-  // type='mcp' API is itself an MCP server configuration we must capture.
-  if (typeof properties.backendId === 'string' && properties.backendId.length > 0) {
-    return true;
-  }
-
-  return false;
-}
-
-function buildEmbeddedMcpServerResource(
-  apiJson: Record<string, unknown>,
-  backendUrl?: string
-): Record<string, unknown> {
-  const properties = getApiProperties(apiJson) ?? {};
-  const resourceProperties: Record<string, unknown> = {};
-
-  // Clone mcpProperties so we can augment it with serverUrl without mutating
-  // the caller's apiJson.
-  let mcpProperties: Record<string, unknown> | undefined;
-  if (properties.mcpProperties && typeof properties.mcpProperties === 'object') {
-    mcpProperties = { ...(properties.mcpProperties as Record<string, unknown>) };
-  } else if (backendUrl) {
-    mcpProperties = {};
-  }
-
-  if (mcpProperties && backendUrl && mcpProperties.serverUrl === undefined) {
-    mcpProperties.serverUrl = backendUrl;
-  }
-
-  if (mcpProperties !== undefined) {
-    resourceProperties.mcpProperties = mcpProperties;
-  }
-  if (properties.mcpTools !== undefined) {
-    resourceProperties.mcpTools = properties.mcpTools;
-  }
-  // Preserve the link to the upstream backend so the MCP server sidecar is
-  // self-describing for MCP-from-existing-MCP-server APIs.
-  if (typeof properties.backendId === 'string' && properties.backendId.length > 0) {
-    resourceProperties.backendId = properties.backendId;
-  }
-
-  return {
-    name: 'default',
-    properties: resourceProperties,
-  };
-
-}
-
-/**
- * For an MCP API wired to a backend via `backendId`, fetch that backend and
- * return its `properties.url` so the MCP sidecar can carry the actual upstream
- * server URL. Returns undefined when the API has no backendId, the backend
- * cannot be fetched, or the backend has no url.
- */
-async function resolveLinkedBackendUrl(
-  client: IApimClient,
-  context: ApimServiceContext,
-  apiJson: Record<string, unknown>,
-  workspace?: string
-): Promise<string | undefined> {
-  const properties = getApiProperties(apiJson);
-  const backendId = properties?.backendId;
-  if (typeof backendId !== 'string' || backendId.length === 0) {
-    return undefined;
-  }
-
-  // backendId may be either a bare resource name or a full ARM resource id.
-  // We only consume the trailing name segment for descriptor lookup.
-  const backendName = backendId.includes('/') ? backendId.split('/').pop()! : backendId;
-
-  try {
-    const backendJson = await client.getResource(context, {
-      type: ResourceType.Backend,
-      nameParts: [backendName],
-      workspace,
-    });
-    const url = (backendJson?.properties as Record<string, unknown> | undefined)?.url;
-    return typeof url === 'string' && url.length > 0 ? url : undefined;
-  } catch (error) {
-    const errorMessage = error instanceof Error ? error.message : String(error);
-    logger.debug(`Could not resolve backend "${backendName}" for MCP server URL: ${errorMessage}`);
-    return undefined;
-  }
-}
-
 /**
  * Extract all API-specific resources for a single API.
  * This includes revisions, specifications, operations, policies, etc.
@@ -259,11 +152,6 @@ export async function extractApiResources(
     client, store, context, apiDescriptor, outputDir
   );
 
-  // Extract MCP server configuration (singleton per API; silently skipped when not present)
-  result.mcpServer = await extractApiMcpServer(
-    client, store, context, apiDescriptor, apiJson, outputDir
-  );
-
   // Extract GraphQL resolvers and their policies
   const resolverResult = await extractGraphQLResolvers(
     client, store, context, apiDescriptor, apiJson, outputDir, filter, workspace
@@ -585,7 +473,7 @@ async function extractGraphQLResolvers(
   const policies: string[] = [];
 
   // Only extract resolvers for GraphQL APIs — use the already-fetched apiJson
-  const properties = getApiProperties(apiJson);
+  const properties = apiJson.properties as Record<string, unknown> | undefined;
   const apiType = properties?.type as string | undefined;
   if (apiType?.toLowerCase() !== 'graphql') {
     return { resolvers, resolverPolicies, policies };
@@ -629,58 +517,6 @@ async function extractGraphQLResolvers(
   return { resolvers, resolverPolicies, policies };
 }
 
-/**
- * Extract MCP (Model Context Protocol) server configuration for an API.
- *
- * MCP configuration is embedded directly on the API resource
- * (`properties.mcpTools`, `properties.mcpProperties`, `properties.backendId`).
- * There is no separate child resource served by ARM — the
- * `apis/{id}/mcpServers/default` endpoint returns 404 even on working MCP APIs,
- * and `apis/{id}/mcpServers` returns 500 (no such collection). All MCP data
- * therefore comes from the API JSON itself.
- *
- * For MCP APIs created from an existing MCP server (the `backendId` pattern),
- * the upstream URL lives on the linked backend, not on the API. To make the
- * extracted `mcpServerInformation.json` self-describing, the extractor
- * resolves that backend and surfaces its URL as `mcpProperties.serverUrl`.
- */
-async function extractApiMcpServer(
-  client: IApimClient,
-  store: IArtifactStore,
-  context: ApimServiceContext,
-  apiDescriptor: ResourceDescriptor,
-  apiJson: Record<string, unknown>,
-  outputDir: string
-): Promise<boolean> {
-  const apiType = (getApiProperties(apiJson)?.type as string | undefined)?.toLowerCase();
-
-  // Avoid creating MCP artifacts for non-MCP APIs unless they carry meaningful MCP metadata.
-  if (apiType !== 'mcp' && !hasEmbeddedMcpConfiguration(apiJson)) {
-    return false;
-  }
-
-  if (!hasEmbeddedMcpConfiguration(apiJson)) {
-    return false;
-  }
-
-  const mcpDescriptor: ResourceDescriptor = {
-    type: ResourceType.McpServer,
-    nameParts: [...apiDescriptor.nameParts],
-    workspace: apiDescriptor.workspace,
-  };
-
-  // Resolve the upstream backend URL so the MCP sidecar carries the actual
-  // server URL (not just a backendId reference + uri template).
-  const backendUrl = await resolveLinkedBackendUrl(client, context, apiJson, apiDescriptor.workspace);
-  await store.writeResource(
-    outputDir,
-    mcpDescriptor,
-    buildEmbeddedMcpServerResource(apiJson, backendUrl)
-  );
-  logger.info(`Extracted ${buildResourceLabel(mcpDescriptor)} from API metadata`);
-  return true;
-}
-
 /**
  * Extract API tag associations in workspace scope using the tag-centric
  * `tags/{tag}/apiLinks` endpoint.
diff --git a/src/services/api-publisher.ts b/src/services/api-publisher.ts
@@ -13,7 +13,11 @@ import type { ApimServiceContext, ResourceDescriptor } from '../models/types.js'
 import type { PublishConfig } from '../models/config.js';
 import * as yaml from 'js-yaml';
 import { ResourceType } from '../models/resource-types.js';
-import { publishResource, type ResourcePublishResult } from './resource-publisher.js';
+import {
+  normalizeMcpToolOperationIds,
+  publishResource,
+  type ResourcePublishResult,
+} from './resource-publisher.js';
 import { runParallel } from '../lib/parallel-runner.js';
 import { applyOverrides } from './override-merger.js';
 import { logger } from '../lib/logger.js';
@@ -33,7 +37,6 @@ const API_CHILD_TYPES: ResourceType[] = [
   ResourceType.ApiRelease,
   ResourceType.ApiTagDescription,
   ResourceType.ApiWiki,
-  ResourceType.McpServer,
   ResourceType.GraphQLResolver,
   ResourceType.GraphQLResolverPolicy,
 ];
@@ -167,6 +170,11 @@ async function publishRootApi(
     };
   }
 
+  // Root APIs publish through api-publisher rather than publishResource, so
+  // they need the same pre-override MCP tool normalization here that revision
+  // APIs receive in resource-publisher.
+  json = normalizeMcpToolOperationIds(json, context);
+
   // Apply overrides
   json = applyOverrides(descriptor, json, config.overrides);
   const isCurrent = getApiIsCurrent(json);
diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts
@@ -120,6 +120,12 @@ export async function publishResource(
       };
     }
 
+    // Rewrite MCP tool operationIds against the target service before overrides
+    // so explicit override values win unchanged.
+    if (descriptor.type === ResourceType.Api) {
+      json = normalizeMcpToolOperationIds(json, context);
+    }
+
     // Apply overrides (deep merge, preserves opaque structure)
     json = applyOverrides(descriptor, json, config.overrides);
 
@@ -233,7 +239,6 @@ export async function publishResource(
     // validation errors in APIM's revision creation.
     if (descriptor.type === ResourceType.Api) {
       const apiName = getNamePart(descriptor.nameParts, 0);
-      json = normalizeMcpToolOperationIds(json, context);
       if (apiName.includes(';rev=')) {
         const baseApiName = apiName.split(';rev=')[0];
         const props = json.properties as Record<string, unknown> | undefined;
@@ -516,7 +521,7 @@ function normalizeApiReleaseApiId(
  * operationId to target service ARM prefix so APIM validates MCP tools against
  * existing operations in the target instance.
  */
-function normalizeMcpToolOperationIds(
+export function normalizeMcpToolOperationIds(
   json: Record<string, unknown>,
   context: ApimServiceContext
 ): Record<string, unknown> {
diff --git a/tests/integration/all-resource-types/expected-structure.json b/tests/integration/all-resource-types/expected-structure.json
@@ -553,34 +553,29 @@
         },
         {
           "name": "src-mcp-from-api",
-          "files": ["apiInformation.json", "mcpServerInformation.json"],
+          "files": ["apiInformation.json"],
           "spotChecks": {
             "apiInformation.json": {
               "properties.displayName": "KS MCP from Existing API",
               "properties.path": "ks/mcp-from-api",
               "properties.type": "mcp",
-              "properties.subscriptionRequired": false
-            },
-            "mcpServerInformation.json": {
+              "properties.subscriptionRequired": false,
               "properties.mcpTools": "exists"
             }
           },
           "notes": "MCP API exposing operations of an existing REST API as MCP tools via mcpTools (each tool's operationId references the backing REST API; this MCP API has no operations of its own)"
         },
         {
           "name": "src-mcp-existing-server",
-          "files": ["apiInformation.json", "mcpServerInformation.json", "policy.xml"],
+          "files": ["apiInformation.json", "policy.xml"],
           "spotChecks": {
             "apiInformation.json": {
               "properties.displayName": "KS MCP Existing Server Demo",
               "properties.path": "ks/mcp-existing",
               "properties.type": "mcp",
               "properties.subscriptionRequired": false,
-              "properties.backendId": "src-backend-mcp-learn"
-            },
-            "mcpServerInformation.json": {
-              "properties.mcpProperties.endpoints.mcp.uriTemplate": "/mcp",
-              "properties.backendId": "src-backend-mcp-learn"
+              "properties.backendId": "src-backend-mcp-learn",
+              "properties.mcpProperties.endpoints.mcp.uriTemplate": "/mcp"
             }
           },
           "notes": "Working existing-server MCP demo. APIM exposes a public Microsoft Learn MCP server through a policy-based MCP proxy so the API is extractable and demoable end to end."
diff --git a/tests/unit/services/api-product-extractor.test.ts b/tests/unit/services/api-product-extractor.test.ts
@@ -850,7 +850,7 @@ describe('api-extractor', () => {
       );
     });
 
-    it('should extract MCP server configuration from embedded API metadata when present', async () => {
+    it('should keep embedded MCP configuration in apiInformation.json without writing a sidecar file', async () => {
       const client = createMockClient({
         getApiSpecification: vi.fn().mockResolvedValue(undefined),
         getResource: vi.fn().mockResolvedValue(undefined),
@@ -873,21 +873,15 @@ describe('api-extractor', () => {
         client, store, testContext, apiDescriptor, apiJson, '/output'
       );
 
-      expect(result.mcpServer).toBe(true);
+      expect(result.mcpServer).toBe(false);
       expect(client.getResource).not.toHaveBeenCalledWith(
         testContext,
         expect.objectContaining({ type: ResourceType.McpServer })
       );
-      expect(store.writeResource).toHaveBeenCalledWith(
+      expect(store.writeResource).not.toHaveBeenCalledWith(
         '/output',
         expect.objectContaining({ type: ResourceType.McpServer, nameParts: ['my-api'] }),
-        {
-          name: 'default',
-          properties: {
-            mcpProperties: { serverUrl: 'https://example.com/mcp' },
-            mcpTools: [{ name: 'invokeTool' }],
-          },
-        }
+        expect.anything()
       );
     });
 
@@ -910,31 +904,6 @@ describe('api-extractor', () => {
 
       expect(result.mcpServer).toBe(false);
     });
-
-    it('should return mcpServer=false and not throw when getResource throws for McpServer', async () => {
-      const client = createMockClient({
-        getApiSpecification: vi.fn().mockResolvedValue(undefined),
-        getResource: vi.fn().mockImplementation(async (_ctx: unknown, desc: ResourceDescriptor) => {
-          if (desc.type === ResourceType.McpServer) {
-            throw new Error('404 Not Found');
-          }
-          return undefined;
-        }),
-      });
-      const store = createMockStore();
-      const apiDescriptor: ResourceDescriptor = {
-        type: ResourceType.Api,
-        nameParts: ['my-api'],
-      };
-
-      const result = await extractApiResources(
-        client, store, testContext, apiDescriptor,
-        { name: 'my-api' },
-        '/output'
-      );
-
-      expect(result.mcpServer).toBe(false);
-    });
   });
 });
 
diff --git a/tests/unit/services/api-publisher.test.ts b/tests/unit/services/api-publisher.test.ts
diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts
