Add async LRO polling and OpenAPI spec sanitization

src/clients/apim-client.ts

@@ -42,6 +42,17 @@ export class ApimClient implements IApimClient {
   private static readonly MAX_DELAY_MS = 30000;
   private static readonly MAX_POLLING_ATTEMPTS = 30;
   private static readonly POLL_INTERVAL_MS = 2000;
+  /** Deadline for async (LRO) operation polling — 7.5 minutes. */
+  private static readonly ASYNC_POLL_TIMEOUT_MS = 7.5 * 60 * 1000;
+  /** Default interval between async operation polls when no Retry-After header. */
+  private static readonly ASYNC_POLL_INTERVAL_MS = 5000;
+  /** Known ARM management plane host suffixes for URL validation. */
+  private static readonly ARM_HOSTS = [
+    'management.azure.com',
+    'management.chinacloudapi.cn',
+    'management.usgovcloudapi.net',
+    'management.microsoftazure.de',
+  ];
   /** Stable ARM API version for Resource Group existence checks */
   private static readonly RESOURCE_GROUP_API_VERSION = '2021-04-01';
 
@@ -291,6 +302,14 @@ export class ApimClient implements IApimClient {
     context: ApimServiceContext,
     descriptor: ResourceDescriptor
   ): Promise<Record<string, unknown> | undefined> {
+    // Some association resources (ProductGroup, ProductApi, GatewayApi) only
+    // support PUT/DELETE. Short-circuit before making a network call.
+    const metadata = RESOURCE_TYPE_METADATA[descriptor.type];
+    if (!metadata.supportsGet) {
+      logger.debug(`Resource type ${descriptor.type} does not support GET, returning undefined`);
+      return undefined;
+    }
+
     const url = buildArmUri(context, descriptor);
     // Azure APIM returns HTTP 500 (not 404) when an API or product has no wiki.
     // Suppress retries for wiki types so the extractor silently skips them.
@@ -353,6 +372,15 @@ export class ApimClient implements IApimClient {
         logger.debug(`Skipping provisioning poll for association resource: ${buildResourceLabel(descriptor)}`);
         return {};
       }
+
+      // Prefer ARM async operation polling when the service provides an
+      // Azure-AsyncOperation or Location header — these long-running operations
+      // (e.g. large API spec imports) may take minutes to complete.
+      const asyncUrl = this.extractAsyncOperationUrl(response);
+      if (asyncUrl) {
+        return await this.pollAsyncOperation(asyncUrl, context, descriptor);
+      }
+
       return await this.pollProvisioningState(context, descriptor);
     }
 
@@ -414,9 +442,14 @@ export class ApimClient implements IApimClient {
 
       // Poll for long-running operations
       if (response.status === 202) {
-        await this.pollProvisioningState(context, descriptor, {
-          treatMissingAsSuccess: true,
-        });
+        const asyncUrl = this.extractAsyncOperationUrl(response);
+        if (asyncUrl) {
+          await this.pollAsyncOperation(asyncUrl, context, descriptor, { treatMissingAsSuccess: true });
+        } else {
+          await this.pollProvisioningState(context, descriptor, {
+            treatMissingAsSuccess: true,
+          });
+        }
       }
 
       return true;
@@ -719,6 +752,132 @@ export class ApimClient implements IApimClient {
     return 'yaml';
   }
 
+  /**
+   * Extract the async operation URL from ARM LRO response headers.
+   * Prefers Azure-AsyncOperation over Location.
+   * Validates the URL points to a known ARM management endpoint to prevent
+   * leaking the bearer token to an unexpected host.
+   */
+  private extractAsyncOperationUrl(response: Response): string | undefined {
+    const asyncOpUrl = response.headers.get('Azure-AsyncOperation')
+      ?? response.headers.get('Operation-Location')
+      ?? response.headers.get('Location');
+
+    if (!asyncOpUrl) return undefined;
+
+    // Validate URL host is a known ARM management endpoint
+    try {
+      const parsed = new URL(asyncOpUrl);
+      const isArmHost = ApimClient.ARM_HOSTS.some(
+        (host) => parsed.hostname === host || parsed.hostname.endsWith(`.${host}`)
+      );
+      if (!isArmHost) {
+        logger.warn(
+          `Ignoring async operation URL with unexpected host: ${parsed.hostname}`
+        );
+        return undefined;
+      }
+    } catch {
+      logger.warn(`Ignoring malformed async operation URL: ${asyncOpUrl}`);
+      return undefined;
+    }
+
+    return asyncOpUrl;
+  }
+
+  /**
+   * Poll an ARM async operation URL until terminal state.
+   * Used for long-running operations like large API spec imports.
+   *
+   * The operation status endpoint returns:
+   *   { "status": "InProgress" | "Succeeded" | "Failed" | "Canceled", ... }
+   *
+   * On success, GETs the original resource to return the final state.
+   */
+  private async pollAsyncOperation(
+    operationUrl: string,
+    context: ApimServiceContext,
+    descriptor: ResourceDescriptor,
+    options: { treatMissingAsSuccess?: boolean } = {}
+  ): Promise<Record<string, unknown>> {
+    const { treatMissingAsSuccess = false } = options;
+    const label = buildResourceLabel(descriptor);
+    const deadline = Date.now() + ApimClient.ASYNC_POLL_TIMEOUT_MS;
+    let pollInterval = ApimClient.ASYNC_POLL_INTERVAL_MS;
+
+    logger.debug(`Polling async operation for ${label}: ${operationUrl}`);
+
+    while (Date.now() < deadline) {
+      await this.delay(pollInterval);
+
+      const response = await this.request(operationUrl);
+
+      // Honour Retry-After if the service provides it
+      const retryAfter = response.headers.get('Retry-After');
+      if (retryAfter) {
+        const parsed = parseInt(retryAfter, 10);
+        if (!isNaN(parsed) && parsed > 0) {
+          pollInterval = parsed * 1000;
+        }
+      }
+
+      // Some Location-style polls respond with 202 (still in progress)
+      // and may not have a JSON body — keep polling.
+      if (response.status === 202) {
+        logger.debug(`Async operation still in progress (HTTP 202) for ${label}`);
+        continue;
+      }
+
+      // Try to parse the status body
+      const text = await response.text();
+      if (!text.trim()) {
+        // Empty body on 200/204 — treat as complete
+        if (response.ok) {
+          return treatMissingAsSuccess
+            ? {}
+            : await this.getResource(context, descriptor) ?? {};
+        }
+        throw new Error(`Async operation returned empty body with status ${response.status} for ${label}`);
+      }
+
+      let body: Record<string, unknown>;
+      try {
+        body = JSON.parse(text) as Record<string, unknown>;
+      } catch {
+        // Non-JSON on an OK status — treat as complete
+        if (response.ok) {
+          return treatMissingAsSuccess
+            ? {}
+            : await this.getResource(context, descriptor) ?? {};
+        }
+        throw new Error(`Async operation returned non-JSON with status ${response.status} for ${label}`);
+      }
+
+      const status = (body.status as string | undefined)?.toLowerCase();
+
+      if (status === 'succeeded') {
+        logger.debug(`Async operation succeeded for ${label}`);
+        if (treatMissingAsSuccess) return {};
+        // GET the final resource state
+        return await this.getResource(context, descriptor) ?? {};
+      }
+
+      if (status === 'failed' || status === 'canceled' || status === 'cancelled') {
+        const error = body.error as Record<string, unknown> | undefined;
+        const code = (error?.code as string) ?? 'UnknownError';
+        const message = (error?.message as string) ?? JSON.stringify(body);
+        throw new Error(`Async operation ${status} for ${label}: [${code}] ${message}`);
+      }
+
+      // InProgress or other intermediate status — keep polling
+      logger.debug(`Async operation status: ${status ?? 'unknown'} for ${label}`);
+    }
+
+    throw new Error(
+      `Async operation polling timed out after ${ApimClient.ASYNC_POLL_TIMEOUT_MS / 1000}s for ${label}`
+    );
+  }
+
   private async pollProvisioningState(
     context: ApimServiceContext,
     descriptor: ResourceDescriptor,

src/services/api-publisher.ts

@@ -200,13 +200,16 @@ async function publishRootApi(
         cleanProps.apiType = apiType;
       }
 
+      // Sanitize the spec before import (e.g. inject missing path parameters)
+      const sanitizedContent = sanitizeOpenApiSpec(specResult.content, specResult.format, descriptor);
+
       // Inject the spec into the PUT body for APIM to import
       json = {
         ...json,
         properties: {
           ...cleanProps,
           format: importFormat,
-          value: specResult.content,
+          value: sanitizedContent,
         },
       };
 
@@ -557,3 +560,84 @@ function getOpenApiOperationIdsWithNullDescription(
     return [];
   }
 }
+
+/**
+ * Sanitize an OpenAPI spec before importing into APIM.
+ * Currently handles:
+ *  - Missing path parameter declarations: APIM rejects specs where a URL
+ *    template contains `{param}` placeholders that are not declared in the
+ *    operation's `parameters` array.  We auto-inject the missing declarations
+ *    as `{ name, in: "path", required: true, schema: { type: "string" } }`
+ *    and log a warning for each one.
+ */
+function sanitizeOpenApiSpec(
+  content: string,
+  format: string | undefined,
+  descriptor: ResourceDescriptor
+): string {
+  // Only process JSON-based OpenAPI specs (openapi+json, json)
+  const isJson =
+    !format ||
+    format.toLowerCase().includes('json') ||
+    content.trimStart().startsWith('{');
+  if (!isJson) return content;
+
+  let spec: Record<string, unknown>;
+  try {
+    spec = JSON.parse(content) as Record<string, unknown>;
+  } catch {
+    return content; // Not valid JSON — return as-is
+  }
+
+  const paths = spec.paths as Record<string, Record<string, unknown>> | undefined;
+  if (!paths) return content;
+
+  const apiLabel = getNamePart(descriptor.nameParts, 0);
+  let modified = false;
+
+  for (const [pathTemplate, pathItem] of Object.entries(paths)) {
+    // Extract placeholders from the path template
+    const placeholders = pathTemplate.match(/\{([^}]+)\}/g);
+    if (!placeholders) continue;
+    const placeholderNames = placeholders.map((p) => p.slice(1, -1));
+
+    for (const [method, operation] of Object.entries(pathItem)) {
+      // Skip non-operation keys (e.g. "summary", "parameters", "servers")
+      if (typeof operation !== 'object' || operation === null) continue;
+      if (!['get', 'put', 'post', 'delete', 'patch', 'head', 'options', 'trace'].includes(method)) continue;
+
+      const op = operation as Record<string, unknown>;
+      const params = (op.parameters ?? []) as Array<Record<string, unknown>>;
+      const declaredPathParams = new Set(
+        params
+          .filter((p) => p.in === 'path')
+          .map((p) => p.name as string)
+      );
+
+      for (const name of placeholderNames) {
+        if (!declaredPathParams.has(name)) {
+          // Inject the missing parameter declaration
+          const newParam: Record<string, unknown> = {
+            name,
+            in: 'path',
+            required: true,
+            schema: { type: 'string' },
+          };
+          const updatedParams = [...params, newParam];
+          op.parameters = updatedParams;
+          // Update params reference for subsequent placeholder checks on this operation
+          params.push(newParam);
+          declaredPathParams.add(name);
+          modified = true;
+
+          const opSummary = (op.summary ?? op.operationId ?? `${method.toUpperCase()} ${pathTemplate}`) as string;
+          logger.warn(
+            `API "${apiLabel}": auto-injected missing path parameter "{${name}}" for operation "${opSummary}"`
+          );
+        }
+      }
+    }
+  }
+
+  return modified ? JSON.stringify(spec) : content;
+}

tests/integration/package-build/package-build.test.ts

@@ -2,16 +2,15 @@
 // Licensed under the MIT license.
 /// <reference types="node" />
 
-import { execFile } from 'node:child_process';
+import { exec as execCb } from 'node:child_process';
 import * as fs from 'node:fs/promises';
 import * as os from 'node:os';
 import * as path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { promisify } from 'node:util';
 import { describe, expect, it } from 'vitest';
 
-const exec = promisify(execFile);
-const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';
+const exec = promisify(execCb);
 const currentFileDir = path.dirname(fileURLToPath(import.meta.url));
 const repoRoot = path.resolve(currentFileDir, '../../..');
 
@@ -33,10 +32,14 @@ function normalizePath(filePath: string): string {
 }
 
 async function runNpm(args: string[]): Promise<string> {
-  const result = await exec(npmCommand, args, {
+  // Use 'npm' with shell so .cmd resolution works on Windows and
+  // plain 'npm' works on Linux/macOS.  Pass args as a single joined string
+  // to avoid the Node.js v24 DEP0190 deprecation warning about passing
+  // array args with shell option.
+  const command = `npm ${args.join(' ')}`;
+  const result = await exec(command, {
     cwd: repoRoot,
     timeout: 90_000,
-    env: process.env,
   });
 
   return result.stdout;