Merge pull request #263 from Alexey-Zheltov/main

This PR fixes several bugs in `extract` and `publish` related to API revision handling, resource deletion, and authentication settings, plus a dependency security bump.

Author: azaslonov

package-lock.json

@@ -207,9 +207,18 @@ export class ApimClient implements IApimClient {
 
         return response;
       } catch (error) {
-        // Do not retry client errors (4xx) — they are deterministic, not transient.
-        // 429 rate-limiting is already handled above and never reaches here.
-        if (error instanceof HttpError && error.status >= 400 && error.status < 500) {
+        // APIM reports operations blocked by an API's in-progress async operation
+        // as a transient 409. Other client errors are deterministic.
+        const isPessimisticConcurrencyConflict =
+          error instanceof HttpError &&
+          error.status === 409 &&
+          error.code === 'PessimisticConcurrencyConflict';
+        if (
+          error instanceof HttpError &&
+          error.status >= 400 &&
+          error.status < 500 &&
+          !isPessimisticConcurrencyConflict
+        ) {
           throw error;
         }
         if (attempt >= ApimClient.MAX_RETRIES) {
@@ -454,7 +463,18 @@ export class ApimClient implements IApimClient {
     context: ApimServiceContext,
     descriptor: ResourceDescriptor
   ): Promise<boolean> {
-    const url = buildArmUri(context, descriptor);
+    let url = buildArmUri(context, descriptor);
+
+    // Deleting an API that has revisions requires deleteRevisions=true; without it
+    // APIM refuses the base (current-revision) delete with "Cannot delete the
+    // current revision of an API." This also removes all revisions in one call,
+    // so callers skip the individual ;rev=N deletes.
+    if (
+      descriptor.type === ResourceType.Api &&
+      !(descriptor.nameParts[0] ?? '').includes(';rev=')
+    ) {
+      url += '&deleteRevisions=true';
+    }
 
     for (let attempt = 1; ; attempt++) {
       try {
@@ -482,6 +502,15 @@ export class ApimClient implements IApimClient {
         if (message.includes('404')) {
           return false;
         }
+        // Resource is still referenced by another entity (e.g. a policy fragment
+        // used by the service policy). It cannot be deleted until the reference is
+        // removed; skip it with a warning instead of failing the whole prune.
+        if (message.includes('is used by the following entities')) {
+          logger.warn(
+            `Skipping delete of ${buildResourceLabel(descriptor)}: still referenced by another entity`
+          );
+          return false;
+        }
         // Transient optimistic-concurrency conflict: cascade deletes of related
         // resources (subscriptions, product/gateway associations) can modify
         // the resource while its async DELETE is in flight. Retry the DELETE.

src/clients/apim-client.ts

@@ -143,6 +143,16 @@ export function getNamePart(nameParts: string[], index: number): string {
   return value;
 }
 
+/** True when an API name carries a revision suffix (e.g. "my-api;rev=2"). */
+export function isApiRevisionName(apiName: string): boolean {
+  return apiName.includes(';rev=');
+}
+
+/** Root API name with any ";rev=N" suffix stripped. */
+export function getApiRootName(apiName: string): string {
+  return apiName.split(';rev=')[0] ?? apiName;
+}
+
 /**
  * Converts a positional template string to a capturing regex.
  * Each `{i}` placeholder becomes a `([^/]+)` capture group; all other

src/lib/resource-path.ts

@@ -200,8 +200,10 @@ async function extractApiRevisions(
     for await (const revision of revisions) {
       try {
         const revNumber = (revision.apiRevision ?? revision.revisionNumber) as string | undefined;
-        if (!revNumber || revNumber === '1') {
-          // Skip revision 1 — it's the main API
+        // Skip the current revision — it is represented by the main API folder.
+        // Using isCurrent (not a hard-coded '1') correctly handles APIs whose
+        // current revision is not revision 1.
+        if (!revNumber || revision.isCurrent === true) {
           continue;
         }
 

src/services/api-extractor.ts

@@ -14,12 +14,15 @@ import type { PublishConfig } from '../models/config.js';
 import * as yaml from 'js-yaml';
 import { ResourceType } from '../models/resource-types.js';
 import {
+  normalizeApiAuthenticationSettings,
   normalizeMcpToolOperationIds,
+  prefersLegacyAuthOverride,
   publishResource,
   type ResourcePublishResult,
 } from './resource-publisher.js';
 import { runParallel } from '../lib/parallel-runner.js';
 import { applyOverrides } from './override-merger.js';
+import { mapDescriptor } from './env-mapper.js';
 import { logger } from '../lib/logger.js';
 import { getNamePart, getPublishTier } from '../lib/resource-path.js';
 import { isAutoGeneratedId } from '../lib/auto-generated.js';
@@ -54,8 +57,26 @@ export async function publishApi(
   config: PublishConfig
 ): Promise<ResourcePublishResult> {
   try {
-    // Step 1: Publish root API (with spec import if available)
-    const rootResult = await publishRootApi(client, store, context, descriptor, config);
+    // Deployed (env-mapped) base descriptor — derived once and used for every
+    // direct APIM call so canonical and affixed names can never diverge.
+    const deployedDescriptor = config.envMapping
+      ? mapDescriptor(descriptor, config.envMapping)
+      : descriptor;
+
+    // Step 1: Publish root API (with spec import if available).
+    // On a fresh target the root is created at its source revision number so
+    // it cannot collide with ;rev=N revision artifacts.
+    const putDescriptor = await resolveRootApiPutDescriptor(
+      client,
+      store,
+      context,
+      descriptor,
+      deployedDescriptor,
+      config
+    );
+    const rootResult = await publishRootApi(client, store, context, descriptor, config, {
+      putDescriptor,
+    });
     if (rootResult.status !== 'success') {
       return rootResult;
     }
@@ -64,7 +85,7 @@ export async function publishApi(
       await alignImportedOperationDescriptions(
         client,
         context,
-        descriptor,
+        deployedDescriptor,
         rootResult.operationIdsWithNullDescription
       );
     }
@@ -75,13 +96,10 @@ export async function publishApi(
     // Step 2b: Align root API only when source marks it as current.
     // Source of truth is properties.isCurrent in root apiInformation.json.
     if (publishedRevisionCount > 0 && rootResult.isCurrent === true) {
-      const alignResult = await alignActiveRevisionWithSource(
-        client,
-        store,
-        context,
-        descriptor,
-        config
-      );
+      const alignResult = await publishRootApi(client, store, context, descriptor, config, {
+        includeSpecification: false,
+        putDescriptor: deployedDescriptor,
+      });
       if (alignResult.status !== 'success') {
         return alignResult;
       }
@@ -148,6 +166,49 @@ interface RootApiResult {
 
 interface PublishRootApiOptions {
   includeSpecification?: boolean;
+  /** Descriptor to PUT to (defaults to the artifact descriptor). Lets the root
+   * API be created at apis/{name};rev=N while still reading apis/{name} artifacts. */
+  putDescriptor?: ResourceDescriptor;
+}
+
+/**
+ * A plain root PUT creates a brand-new API as revision 1, which collides with
+ * a ;rev=1 revision artifact and silently absorbs it whenever the source's
+ * current revision number is > 1. When the API does not yet exist on the
+ * target, PUT the root at its true revision number (apis/{name};rev=N) instead.
+ * Existing APIs keep the plain root PUT — their current revision cannot be
+ * renumbered.
+ *
+ * Reads artifacts via the canonical descriptor; all APIM lookups and the
+ * returned PUT target use the deployed (env-mapped) descriptor, with ;rev=N
+ * appended after affixing so suffix mappings cannot corrupt it.
+ */
+async function resolveRootApiPutDescriptor(
+  client: IApimClient,
+  store: IArtifactStore,
+  context: ApimServiceContext,
+  descriptor: ResourceDescriptor,
+  deployedDescriptor: ResourceDescriptor,
+  config: PublishConfig
+): Promise<ResourceDescriptor> {
+  const json = await store.readResource(config.sourceDir, descriptor);
+  const rev = (json?.properties as Record<string, unknown> | undefined)?.apiRevision;
+  if (typeof rev !== 'string' || rev === '' || rev === '1') {
+    return deployedDescriptor;
+  }
+
+  const existing = await client.getResource(context, deployedDescriptor);
+  if (existing) {
+    return deployedDescriptor;
+  }
+
+  return {
+    ...deployedDescriptor,
+    nameParts: [
+      `${getNamePart(deployedDescriptor.nameParts, 0)};rev=${rev}`,
+      ...deployedDescriptor.nameParts.slice(1),
+    ],
+  };
 }
 
 /**
@@ -176,11 +237,18 @@ 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);
+  // APIs receive in resource-publisher — including env-mapped API names so the
+  // tool operationIds match the affixed API this PUT targets.
+  json = normalizeMcpToolOperationIds(json, context, config.envMapping);
 
   // Apply overrides
   json = applyOverrides(descriptor, json, config.overrides);
+  json = normalizeApiAuthenticationSettings(json, {
+    preferLegacyFields: prefersLegacyAuthOverride(
+      getNamePart(descriptor.nameParts, 0),
+      config.overrides?.apis
+    ),
+  });
   const isCurrent = getApiIsCurrent(json);
 
   // Try to read the specification file for this API
@@ -239,7 +307,7 @@ async function publishRootApi(
   }
 
   // PUT the API resource to APIM
-  await client.putResource(context, descriptor, json);
+  await client.putResource(context, options?.putDescriptor ?? descriptor, json);
 
   return {
     descriptor,
@@ -251,22 +319,6 @@ async function publishRootApi(
   };
 }
 
-async function alignActiveRevisionWithSource(
-  client: IApimClient,
-  store: IArtifactStore,
-  context: ApimServiceContext,
-  descriptor: ResourceDescriptor,
-  config: PublishConfig
-): Promise<RootApiResult & ResourcePublishResult> {
-  logger.debug(
-    `Source marks "${getNamePart(descriptor.nameParts, 0)}" as current; re-applying root metadata to align active revision`
-  );
-
-  return publishRootApi(client, store, context, descriptor, config, {
-    includeSpecification: false,
-  });
-}
-
 /**
  * Find and publish API revisions in numeric order
  */
@@ -294,12 +346,41 @@ async function publishApiRevisions(
     return revA - revB;
   });
 
-  // Publish each revision in order
+  // The current revision is already published as the root API, so skip a
+  // revision artifact that carries the same number — otherwise we re-create /
+  // collide with it (e.g. the root PUT created at ;rev=N for a fresh
+  // multi-revision API, or a stale ;rev=N folder from an older extract).
+  const rootJson = await store.readResource(config.sourceDir, apiDescriptor);
+  const rootRevision = (rootJson?.properties as Record<string, unknown> | undefined)?.apiRevision;
+  const rootRevisionNumber =
+    typeof rootRevision === 'string' && rootRevision !== '' ? Number(rootRevision) : undefined;
+
+  // Publish each revision in order; a failed revision must fail the API —
+  // otherwise errors are silently swallowed and the exit code stays 0.
+  let publishedCount = 0;
   for (const revDescriptor of sortedRevisions) {
-    await publishResource(client, store, context, revDescriptor, config);
+    if (
+      rootRevisionNumber !== undefined &&
+      extractRevisionNumber(getNamePart(revDescriptor.nameParts, 0)) === rootRevisionNumber
+    ) {
+      logger.debug(
+        `Skipping revision ${getNamePart(revDescriptor.nameParts, 0)} — already published as the root API`
+      );
+      continue;
+    }
+    const result = await publishResource(client, store, context, revDescriptor, config);
+    if (result.status === 'failed') {
+      throw new Error(
+        `Failed to publish revision ${getNamePart(revDescriptor.nameParts, 0)}: ` +
+        `${result.error?.message ?? 'unknown error'}`
+      );
+    }
+    if (result.status === 'success') publishedCount++;
   }
 
-  return sortedRevisions.length;
+  // Return only revisions actually published so the caller does not run a
+  // spurious active-revision alignment PUT when nothing changed.
+  return publishedCount;
 }
 
 /**
@@ -467,8 +548,15 @@ async function reconcileOperationsAfterSpecImport(
 
     const patchBody: Record<string, unknown> = { properties: patchProps };
 
+    // Artifact lookup above uses the canonical descriptor; the PATCH must target
+    // the deployed (env-mapped) name so reconciliation hits the API that the
+    // root create actually produced under environment mapping.
+    const patchDescriptor = config.envMapping
+      ? mapDescriptor(descriptor, config.envMapping)
+      : descriptor;
+
     try {
-      await client.patchResource(context, descriptor, patchBody);
+      await client.patchResource(context, patchDescriptor, patchBody);
       logger.debug(`Reconciled operation "${getNamePart(descriptor.nameParts, 1)}" after spec import`);
     } catch (error) {
       logger.warn(

src/services/api-publisher.ts

@@ -29,10 +29,54 @@ import type { ApimServiceContext, ResourceDescriptor } from '../models/types.js'
 import type { PublishConfig } from '../models/config.js';
 import { ResourceType } from '../models/resource-types.js';
 import { getTopologicalOrder } from '../lib/dependency-graph.js';
-import { getNameFromNameParts } from '../lib/resource-path.js';
+import {
+  getNameFromNameParts,
+  getNamePart,
+  getApiRootName,
+  isApiRevisionName,
+} from '../lib/resource-path.js';
 import { logger } from '../lib/logger.js';
 import { toCanonicalDescriptor } from './env-mapper.js';
 
+/**
+ * Drop ;rev=N API deletes whose base API (same workspace) is also queued for
+ * deletion. The base API delete uses deleteRevisions=true and removes all
+ * revisions in one call, so individual revision deletes are redundant and can
+ * hit APIM's "Cannot delete the current revision of an API" error.
+ *
+ * Shared by the real delete path and the dry-run reporter so the preview
+ * matches what publish would actually delete.
+ */
+export function filterRevisionDeletesHandledByBaseApi(
+  descriptors: ResourceDescriptor[]
+): ResourceDescriptor[] {
+  const baseApiKeys = new Set<string>();
+  for (const descriptor of descriptors) {
+    if (descriptor.type !== ResourceType.Api) {
+      continue;
+    }
+    const apiName = getNamePart(descriptor.nameParts, 0);
+    if (!isApiRevisionName(apiName)) {
+      baseApiKeys.add(`${descriptor.workspace ?? ''}::${apiName}`);
+    }
+  }
+
+  if (baseApiKeys.size === 0) {
+    return descriptors;
+  }
+
+  return descriptors.filter((descriptor) => {
+    if (descriptor.type !== ResourceType.Api) {
+      return true;
+    }
+    const apiName = getNamePart(descriptor.nameParts, 0);
+    if (!isApiRevisionName(apiName)) {
+      return true;
+    }
+    return !baseApiKeys.has(`${descriptor.workspace ?? ''}::${getApiRootName(apiName)}`);
+  });
+}
+
 /**
  * Built-in groups that should never be deleted
  */