fix(gateway): address review feedback

Author: Alexander Zaslonov

src/clients/apim-client.ts

@@ -24,7 +24,8 @@ export class HttpError extends Error {
   constructor(
     public readonly status: number,
     message: string,
-    public readonly code?: string  // APIM error code, e.g. "MethodNotAllowedInPricingTier"
+    public readonly code?: string,  // APIM error code, e.g. "MethodNotAllowedInPricingTier"
+    public readonly body?: unknown
   ) {
     super(message);
     this.name = 'HttpError';
@@ -41,6 +42,26 @@ export function isLinkAlreadyExistsError(error: unknown): boolean {
   return error instanceof HttpError && error.status === 409;
 }
 
+/** Returns true when APIM reports that an association's referenced API/group is absent. */
+export function isAssociationReferenceNotFoundError(error: unknown): boolean {
+  if (
+    !(error instanceof HttpError) ||
+    ![400, 404].includes(error.status) ||
+    !['ValidationError', 'ResourceNotFound'].includes(error.code ?? '')
+  ) {
+    return false;
+  }
+
+  const body = error.body as Record<string, unknown> | undefined;
+  const armError = body?.error as Record<string, unknown> | undefined;
+  const details = Array.isArray(armError?.details) ? armError.details : [];
+  return details.some((detail) => {
+    if (typeof detail !== 'object' || detail === null) return false;
+    const target = (detail as Record<string, unknown>).target;
+    return typeof target === 'string' && ['aid', 'gid'].includes(target.toLowerCase());
+  });
+}
+
 export class ApimClient implements IApimClient {
   private credential: DefaultAzureCredential;
   private readonly authScope: string;
@@ -187,8 +208,9 @@ export class ApimClient implements IApimClient {
         if (!response.ok) {
           const errorText = await response.text();
           let errorCode: string | undefined;
+          let errorBody: unknown;
           try {
-            const errorBody: unknown = JSON.parse(errorText);
+            errorBody = JSON.parse(errorText);
             if (
               typeof errorBody === 'object' && errorBody !== null &&
               'error' in errorBody &&
@@ -202,7 +224,7 @@ export class ApimClient implements IApimClient {
           } catch {
             // Response body is not JSON — no error code available
           }
-          throw new HttpError(response.status, `HTTP ${response.status}: ${errorText}`, errorCode);
+          throw new HttpError(response.status, `HTTP ${response.status}: ${errorText}`, errorCode, errorBody);
         }
 
         return response;

src/services/delete-unmatched-service.ts

@@ -36,7 +36,7 @@ import {
   isApiRevisionName,
 } from '../lib/resource-path.js';
 import { logger } from '../lib/logger.js';
-import { toCanonicalDescriptor } from './env-mapper.js';
+import { mapDescriptor, toCanonicalDescriptor, toCanonicalName } from './env-mapper.js';
 
 /**
  * Drop ;rev=N API deletes whose base API (same workspace) is also queued for
@@ -243,6 +243,9 @@ async function computeGatewayApiDeleteActions(
       type: ResourceType.Gateway,
       nameParts: [gatewayName],
     };
+    const deployedGatewayDescriptor = envMapping !== undefined
+      ? mapDescriptor(gatewayDescriptor, envMapping)
+      : gatewayDescriptor;
 
     // Desired API set (canonical names) from the gateway's apis.json artifact.
     let desiredApis: Set<string>;
@@ -260,7 +263,7 @@ async function computeGatewayApiDeleteActions(
       for await (const apiJson of client.listResources(
         context,
         ResourceType.GatewayApi,
-        gatewayDescriptor
+        deployedGatewayDescriptor
       )) {
         const apiName = extractResourceName(apiJson);
         if (!apiName) {
@@ -269,7 +272,7 @@ async function computeGatewayApiDeleteActions(
 
         const deployedDescriptor: ResourceDescriptor = {
           type: ResourceType.GatewayApi,
-          nameParts: [gatewayName, apiName],
+          nameParts: [getNamePart(deployedGatewayDescriptor.nameParts, 0), apiName],
         };
 
         // Compare the deployed API against the desired set using canonical names
@@ -281,7 +284,13 @@ async function computeGatewayApiDeleteActions(
             // Belongs to another environment — do not touch.
             continue;
           }
-          canonicalApiName = getNamePart(canonicalDescriptor.nameParts, 1);
+          const canonicalChildName = toCanonicalName(apiName, ResourceType.Api, envMapping);
+          if (canonicalChildName === undefined) {
+            // The gateway can be shared (for example, "managed"), so the child
+            // API must independently belong to this environment's namespace.
+            continue;
+          }
+          canonicalApiName = canonicalChildName;
         }
 
         if (!desiredApis.has(canonicalApiName)) {

src/services/env-mapper.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { ResourceType } from '../models/resource-types.js';
+import { MANAGED_GATEWAY_NAME, ResourceType } from '../models/resource-types.js';
 import type { ResourceDescriptor } from '../models/types.js';
 import type { EnvironmentOverride, OverrideConfig } from '../models/config.js';
 
@@ -180,7 +180,12 @@ function splitRevisionSuffix(name: string, type: ResourceType): { base: string;
     : { base: name.slice(0, idx), revSuffix: name.slice(idx) };
 }
 
+function isManagedGatewayName(name: string, type: ResourceType): boolean {
+  return type === ResourceType.Gateway && name === MANAGED_GATEWAY_NAME;
+}
+
 export function toDeployedName(name: string, type: ResourceType, m: EnvMapping): string {
+  if (isManagedGatewayName(name, type)) return name;
   if (!m.appliesTo.has(type)) return name;
   const { base, revSuffix } = splitRevisionSuffix(name, type);
   return `${m.prefix}${base}${m.suffix}${revSuffix}`;
@@ -192,6 +197,7 @@ export function toDeployedName(name: string, type: ResourceType, m: EnvMapping):
  * Returns input unchanged when type ∉ appliesTo.
  */
 export function toCanonicalName(deployedName: string, type: ResourceType, m: EnvMapping): string | undefined {
+  if (isManagedGatewayName(deployedName, type)) return deployedName;
   if (!m.appliesTo.has(type)) return deployedName;
   if (!isInEnvNamespace(deployedName, type, m)) return undefined;
 
@@ -207,6 +213,7 @@ export function toCanonicalName(deployedName: string, type: ResourceType, m: Env
  * When type ∉ appliesTo → returns true (namespace scoping doesn't apply to this type).
  */
 export function isInEnvNamespace(deployedName: string, type: ResourceType, m: EnvMapping): boolean {
+  if (isManagedGatewayName(deployedName, type)) return true;
   if (!m.appliesTo.has(type)) return true;
   const { base } = splitRevisionSuffix(deployedName, type);
   if (base.length < m.prefix.length + m.suffix.length) return false;

src/services/extract-service.ts

@@ -439,7 +439,7 @@ async function extractGatewayAssociations(
   store: IArtifactStore,
   context: ApimServiceContext,
   outputDir: string,
-  _filter: FilterConfig | undefined,
+  filter: FilterConfig | undefined,
   result: ExtractionResult
 ): Promise<void> {
   const gatewayResults = result.typeResults.filter((r) => r.type === ResourceType.Gateway);
@@ -478,6 +478,10 @@ async function extractGatewayAssociations(
     type: ResourceType.Gateway,
     nameParts: [MANAGED_GATEWAY_NAME],
   };
+  if (!shouldIncludeResource(managedDescriptor, filter)) {
+    return;
+  }
+
   try {
     const apiNames: string[] = [];
     for await (const apiJson of client.listResources(context, ResourceType.GatewayApi, managedDescriptor)) {
@@ -486,11 +490,8 @@ async function extractGatewayAssociations(
         apiNames.push(name);
       }
     }
-    // Only record the managed gateway when it actually has assignments. An empty
-    // apis.json would neither help publish nor delete-unmatched reconciliation
-    // (which scopes to gateways that own at least one local assignment).
+    await store.writeAssociation(outputDir, managedDescriptor, 'apis', apiNames);
     if (apiNames.length > 0) {
-      await store.writeAssociation(outputDir, managedDescriptor, 'apis', apiNames);
       result.totalExtracted++;
       logger.info(`Extracted ${apiNames.length} API associations for gateway "${MANAGED_GATEWAY_NAME}"`);
     }

src/services/resource-publisher.ts

@@ -19,7 +19,7 @@ import { isAutoGeneratedId } from '../lib/auto-generated.js';
 import { isWorkspaceScope, buildLinkPayload } from '../lib/workspace-link.js';
 import { logger } from '../lib/logger.js';
 import { REDACTION_MARKER } from './secret-redactor.js';
-import { isLinkAlreadyExistsError } from '../clients/apim-client.js';
+import { isAssociationReferenceNotFoundError, isLinkAlreadyExistsError } from '../clients/apim-client.js';
 import type { OverrideConfig, OverrideSection } from '../models/config.js';
 import { buildResourceLabel } from '../lib/resource-uri.js';
 import { mapDescriptor, toDeployedName } from './env-mapper.js';
@@ -540,8 +540,7 @@ async function publishAssociation(
         // The referenced API/group is absent on the target (filtered out or
         // failed to publish). Skip this single link with a warning instead of
         // aborting the whole association, so other present entries still link.
-        const message = error instanceof Error ? error.message : String(error);
-        if (message.includes('API not found') || message.includes('Group not found')) {
+        if (isAssociationReferenceNotFoundError(error)) {
           logger.warn(
             `Skipping ${associationType} association '${entry.name}' on ` +
             `'${getNamePart(descriptor.nameParts, 0)}': referenced resource not found on target`

tests/unit/services/delete-unmatched-service.test.ts

@@ -13,6 +13,7 @@ import { ResourceType } from '../../../src/models/resource-types.js';
 import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/types.js';
 import { PublishConfig } from '../../../src/models/config.js';
 import { LogLevel } from '../../../src/lib/logger.js';
+import { buildEnvMapping } from '../../../src/services/env-mapper.js';
 
 function createMockClient(apimResources: Map<ResourceType, Record<string, unknown>[]> = new Map()) {
   return {
@@ -277,6 +278,63 @@ describe('delete-unmatched-service', () => {
       expect(result.filter((d) => d.type === ResourceType.GatewayApi)).toHaveLength(0);
     });
 
+    it('should list custom gateway APIs using the deployed parent name', async () => {
+      const localDescriptors: ResourceDescriptor[] = [
+        { type: ResourceType.GatewayApi, nameParts: ['custom-gateway'] },
+      ];
+      const listedParents: Array<ResourceDescriptor | undefined> = [];
+      const client = createMockClient();
+      client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType, parent?: ResourceDescriptor) {
+        if (type === ResourceType.GatewayApi) {
+          listedParents.push(parent);
+          if (parent?.nameParts[0] === 'dev-custom-gateway') {
+            yield { name: 'dev-api-stale' };
+          }
+        }
+      };
+      const store = createMockStore(localDescriptors);
+      store.readAssociation = vi.fn().mockResolvedValue([]);
+      const envMapping = buildEnvMapping({
+        namePrefix: 'dev-',
+        appliesTo: [ResourceType.Gateway, ResourceType.Api],
+      });
+
+      const result = await computeDeleteActions(client, store, testContext, { ...testConfig, envMapping });
+
+      expect(listedParents).toContainEqual({
+        type: ResourceType.Gateway,
+        nameParts: ['dev-custom-gateway'],
+      });
+      expect(result).toContainEqual({
+        type: ResourceType.GatewayApi,
+        nameParts: ['dev-custom-gateway', 'dev-api-stale'],
+      });
+    });
+
+    it('should delete all in-scope managed gateway APIs for an empty desired set without touching other environments', async () => {
+      const apimResources = new Map<ResourceType, Record<string, unknown>[]>([
+        [ResourceType.GatewayApi, [
+          { name: 'dev-api-one' },
+          { name: 'dev-api-two' },
+          { name: 'prod-api' },
+        ]],
+      ]);
+      const localDescriptors: ResourceDescriptor[] = [
+        { type: ResourceType.GatewayApi, nameParts: ['managed'] },
+      ];
+      const client = createMockClient(apimResources);
+      const store = createMockStore(localDescriptors);
+      store.readAssociation = vi.fn().mockResolvedValue([]);
+      const envMapping = buildEnvMapping({ namePrefix: 'dev-' });
+
+      const result = await computeDeleteActions(client, store, testContext, { ...testConfig, envMapping });
+
+      expect(result.filter((descriptor) => descriptor.type === ResourceType.GatewayApi)).toEqual([
+        { type: ResourceType.GatewayApi, nameParts: ['managed', 'dev-api-one'] },
+        { type: ResourceType.GatewayApi, nameParts: ['managed', 'dev-api-two'] },
+      ]);
+    });
+
     it('should not touch any gateway assignments when artifacts track no gateways', async () => {
       const apimResources = new Map<ResourceType, Record<string, unknown>[]>([
         [ResourceType.GatewayApi, [{ name: 'api-a' }, { name: 'api-b' }]],

tests/unit/services/env-mapper.test.ts

@@ -12,6 +12,7 @@ import {
   buildEnvMappingFromOverrides,
   toDeployedName,
   toCanonicalName,
+  toCanonicalDescriptor,
   isInEnvNamespace,
   mapDescriptor,
   type EnvironmentOverride,
@@ -393,6 +394,21 @@ describe('env-mapper', () => {
       expect(result).toEqual({ type: ResourceType.GatewayApi, nameParts: ['my-gw', 'dev-petstore'] });
     });
 
+    it('GatewayApi: preserves the managed gateway when Gateway is explicitly affixed', () => {
+      const mapping = prefixMapping('dev-', [ResourceType.Gateway, ResourceType.Api]);
+      const descriptor: ResourceDescriptor = { type: ResourceType.GatewayApi, nameParts: ['managed', 'petstore'] };
+
+      expect(mapDescriptor(descriptor, mapping)).toEqual({
+        type: ResourceType.GatewayApi,
+        nameParts: ['managed', 'dev-petstore'],
+      });
+      expect(toCanonicalDescriptor({
+        type: ResourceType.GatewayApi,
+        nameParts: ['managed', 'dev-petstore'],
+      }, mapping)).toEqual(descriptor);
+      expect(isInEnvNamespace('managed', ResourceType.Gateway, mapping)).toBe(true);
+    });
+
     it('ServicePolicy: no nameParts, returned unchanged', () => {
       const d: ResourceDescriptor = { type: ResourceType.ServicePolicy, nameParts: [] };
       expect(mapDescriptor(d, m)).toEqual(d);

tests/unit/services/extract-service.test.ts

@@ -633,8 +633,7 @@ describe('extract-service', () => {
       );
     });
 
-    it('should not write a managed gateway association when it has no apis', async () => {
-      // Empty managed gateway must not produce an artifact or inflate the count.
+    it('should write an empty managed gateway association when it has no apis', async () => {
       const client = createMockClient();
       const store = createMockStore();
 
@@ -645,13 +644,41 @@ describe('extract-service', () => {
         logLevel: LogLevel.INFO,
       });
 
+      expect(store.writeAssociation).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({ nameParts: ['managed'] }),
+        'apis',
+        []
+      );
+      expect(result.totalExtracted).toBe(0);
+    });
+
+    it('should not extract the managed gateway when excluded by the gateway filter', async () => {
+      let managedGatewayListed = false;
+      const client = createMockClient();
+      client.listResources = async function* (_ctx: ApimServiceContext, type: ResourceType, parent?: ResourceDescriptor) {
+        if (type === ResourceType.GatewayApi && parent?.nameParts[0] === 'managed') {
+          managedGatewayListed = true;
+          yield { name: 'managed-api' };
+        }
+      };
+      const store = createMockStore();
+
+      await runExtraction(client, store, {
+        service: testContext,
+        outputDir: '/output',
+        filter: { gateways: [] },
+        includeTransitive: false,
+        logLevel: LogLevel.INFO,
+      });
+
+      expect(managedGatewayListed).toBe(false);
       expect(store.writeAssociation).not.toHaveBeenCalledWith(
         expect.anything(),
         expect.objectContaining({ nameParts: ['managed'] }),
         'apis',
         expect.anything()
       );
-      expect(result.totalExtracted).toBe(0);
     });
 
     it('should handle gateway association extraction error gracefully', async () => {