fix: env-mapped op reconcile, revision-only env suffix, auth override key, revision-collision guard, tests

Author: Alexey-Zheltov

src/services/api-publisher.ts

@@ -345,9 +345,27 @@ async function publishApiRevisions(
     return revA - revB;
   });
 
+  // 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.
   for (const revDescriptor of sortedRevisions) {
+    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(
@@ -525,8 +543,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/env-mapper.ts

@@ -169,9 +169,11 @@ export function toCanonicalDescriptor(d: ResourceDescriptor, m: EnvMapping): Res
 
 /**
  * Split an API name into base and ";rev=N" suffix so env affixes apply to the
- * base name only (deployed form is "{prefix}{base}{suffix};rev=N").
+ * base name only (deployed form is "{prefix}{base}{suffix};rev=N"). The ;rev=N
+ * suffix is only meaningful for API revisions, so non-API types are never split.
  */
-function splitRevisionSuffix(name: string): { base: string; revSuffix: string } {
+function splitRevisionSuffix(name: string, type: ResourceType): { base: string; revSuffix: string } {
+  if (type !== ResourceType.Api) return { base: name, revSuffix: '' };
   const idx = name.indexOf(';rev=');
   return idx === -1
     ? { base: name, revSuffix: '' }
@@ -180,7 +182,7 @@ function splitRevisionSuffix(name: string): { base: string; revSuffix: string }
 
 export function toDeployedName(name: string, type: ResourceType, m: EnvMapping): string {
   if (!m.appliesTo.has(type)) return name;
-  const { base, revSuffix } = splitRevisionSuffix(name);
+  const { base, revSuffix } = splitRevisionSuffix(name, type);
   return `${m.prefix}${base}${m.suffix}${revSuffix}`;
 }
 
@@ -193,7 +195,7 @@ export function toCanonicalName(deployedName: string, type: ResourceType, m: Env
   if (!m.appliesTo.has(type)) return deployedName;
   if (!isInEnvNamespace(deployedName, type, m)) return undefined;
 
-  const { base, revSuffix } = splitRevisionSuffix(deployedName);
+  const { base, revSuffix } = splitRevisionSuffix(deployedName, type);
   let name = base;
   if (m.prefix) name = name.slice(m.prefix.length);
   if (m.suffix) name = name.slice(0, name.length - m.suffix.length);
@@ -206,7 +208,7 @@ export function toCanonicalName(deployedName: string, type: ResourceType, m: Env
  */
 export function isInEnvNamespace(deployedName: string, type: ResourceType, m: EnvMapping): boolean {
   if (!m.appliesTo.has(type)) return true;
-  const { base } = splitRevisionSuffix(deployedName);
+  const { base } = splitRevisionSuffix(deployedName, type);
   if (base.length < m.prefix.length + m.suffix.length) return false;
   return base.startsWith(m.prefix) && base.endsWith(m.suffix);
 }

src/services/resource-publisher.ts

@@ -394,10 +394,7 @@ export async function publishResource(
     if (descriptor.type === ResourceType.Api) {
       const apiName = getNamePart(descriptor.nameParts, 0);
       json = normalizeApiAuthenticationSettings(json, {
-        preferLegacyFields: prefersLegacyAuthOverride(
-          apiName.split(';rev=')[0] ?? apiName,
-          config.overrides?.apis
-        ),
+        preferLegacyFields: prefersLegacyAuthOverride(apiName, config.overrides?.apis),
       });
       if (apiName.includes(';rev=')) {
         const baseApiName = apiName.split(';rev=')[0];

tests/unit/services/api-publisher.test.ts

@@ -11,6 +11,7 @@ import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/type
 import { PublishConfig } from '../../../src/models/config.js';
 import { LogLevel } from '../../../src/lib/logger.js';
 import { applyOverrides } from '../../../src/services/override-merger.js';
+import { EnvMapping } from '../../../src/services/env-mapper.js';
 
 // Mock resource-publisher so we can verify call sequence
 const mockPublishResource = vi.fn();
@@ -479,6 +480,31 @@ describe('api-publisher', () => {
       expect(client.putResource).toHaveBeenCalledTimes(1);
     });
 
+    it('skips the revision artifact whose number equals the current root revision', async () => {
+      const client = createMockClient();
+      const store = createMockStore([
+        { type: ResourceType.Api, nameParts: ['orders-api;rev=2'] },
+        { type: ResourceType.Api, nameParts: ['orders-api;rev=3'] },
+      ]);
+      // Root is the current revision 3; the ;rev=3 artifact duplicates it.
+      store.readResource.mockImplementation(async (_dir: string, d: ResourceDescriptor) => {
+        if (d.type === ResourceType.Api && !(d.nameParts[0] ?? '').includes(';rev=')) {
+          return { name: 'orders-api', properties: { apiRevision: '3', isCurrent: true } };
+        }
+        return null;
+      });
+
+      const apiDescriptor: ResourceDescriptor = { type: ResourceType.Api, nameParts: ['orders-api'] };
+
+      await publishApi(client, store, testContext, apiDescriptor, testConfig);
+
+      const publishedRevs = mockPublishResource.mock.calls.map(
+        (c: unknown[]) => (c[3] as ResourceDescriptor).nameParts[0]
+      );
+      expect(publishedRevs).toContain('orders-api;rev=2');
+      expect(publishedRevs).not.toContain('orders-api;rev=3');
+    });
+
     it('should replay root API without re-importing specification after revisions', async () => {
       const client = createMockClient();
       const revisions = [{ type: ResourceType.Api, nameParts: ['orders-api;rev=2'] }];
@@ -596,6 +622,72 @@ describe('api-publisher', () => {
       expect(mockPublishResource.mock.calls[0][3].nameParts[0]).toBe('orders-api;rev=2');
     });
 
+    it('fails the API publish when a revision publish fails (no silent success)', async () => {
+      const client = createMockClient();
+      const store = createMockStore([
+        { type: ResourceType.Api, nameParts: ['orders-api;rev=2'] },
+      ]);
+      // Root API publishes fine; the revision publish returns failed.
+      mockPublishResource.mockResolvedValue({
+        descriptor: { type: ResourceType.Api, nameParts: ['orders-api;rev=2'] },
+        status: 'failed',
+        action: 'noop',
+        error: new Error('revision boom'),
+      });
+
+      const apiDescriptor: ResourceDescriptor = {
+        type: ResourceType.Api,
+        nameParts: ['orders-api'],
+      };
+
+      const result = await publishApi(client, store, testContext, apiDescriptor, testConfig);
+
+      expect(result.status).toBe('failed');
+      expect(result.error?.message).toContain('orders-api;rev=2');
+    });
+
+    it('reconciles operations against the env-mapped API name after spec import', async () => {
+      const client = createMockClient();
+      const store = createMockStore([
+        { type: ResourceType.ApiOperation, nameParts: ['orders-api', 'get-orders'] },
+      ]);
+      store.readResource.mockImplementation(async (_dir: string, d: ResourceDescriptor) => {
+        if (d.type === ResourceType.Api && !(d.nameParts[0] ?? '').includes(';rev=')) {
+          return { name: 'orders-api', properties: {} };
+        }
+        if (d.type === ResourceType.ApiOperation) {
+          return { name: 'get-orders', properties: { displayName: 'Get Orders', method: 'GET', urlTemplate: '/orders' } };
+        }
+        return null;
+      });
+      store.readContent.mockImplementation(async (_dir: string, _d: ResourceDescriptor, kind: string) => {
+        return kind === 'specification'
+          ? { content: 'openapi: 3.0.0\ninfo:\n  title: t\n  version: "1"\npaths: {}\n', format: 'yaml' }
+          : undefined;
+      });
+      // Execute the reconcile tasks so the PATCH target can be asserted.
+      mockRunParallel.mockImplementation(async (tasks: Array<() => Promise<unknown>>) => {
+        for (const t of tasks) await t();
+      });
+
+      const envMapping: EnvMapping = { prefix: 'dev-', suffix: '-eu', appliesTo: new Set([ResourceType.Api]) };
+      const config: PublishConfig = { ...testConfig, envMapping };
+
+      await publishApi(
+        client, store, testContext,
+        { type: ResourceType.Api, nameParts: ['orders-api'] },
+        config
+      );
+
+      const opPatch = client.patchResource.mock.calls.find(
+        (c: unknown[]) => (c[1] as ResourceDescriptor).type === ResourceType.ApiOperation
+      );
+      expect(opPatch).toBeDefined();
+      // PATCH must target the affixed API name, not the canonical one.
+      expect((opPatch![1] as ResourceDescriptor).nameParts[0]).toBe('dev-orders-api-eu');
+      expect((opPatch![1] as ResourceDescriptor).nameParts[1]).toBe('get-orders');
+    });
+
     it('should publish API child resources in parallel', async () => {
       const client = createMockClient();
       const children = [

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

@@ -297,6 +297,13 @@ describe('env-mapper', () => {
       expect(isInEnvNamespace('dev-orders-api-eu;rev=2', ResourceType.Api, mb)).toBe(true);
       expect(isInEnvNamespace('orders-api;rev=2', ResourceType.Api, mb)).toBe(false);
     });
+
+    it('does NOT split ;rev= for non-API types (affix applies to the whole name)', () => {
+      const mb = bothMapping('dev-', '-eu');
+      // ;rev= is only meaningful for API revisions; other types treat it literally.
+      expect(toDeployedName('token;rev=2', ResourceType.NamedValue, mb)).toBe('dev-token;rev=2-eu');
+      expect(toCanonicalName('dev-token;rev=2-eu', ResourceType.NamedValue, mb)).toBe('token;rev=2');
+    });
   });
 
   // ─── isInEnvNamespace ────────────────────────────────────────────────────

tests/unit/services/resource-publisher.test.ts

@@ -1442,5 +1442,28 @@ describe('resource-publisher', () => {
       expect(prefersLegacyAuthOverride('my-api', undefined)).toBe(false);
       expect(prefersLegacyAuthOverride('my-api', { 'my-api': { properties: { path: '/x' } } })).toBe(false);
     });
+
+    it('matches by the exact resource key (revision names are not base-inherited)', () => {
+      // A revision-specific legacy override is honored only under its full key —
+      // this is why the caller looks up the full API name, not the stripped base.
+      const revisionSection = {
+        'my-api;rev=2': {
+          properties: {
+            authenticationSettings: { oAuth2: { authorizationServerId: 'rev-server' } },
+          },
+        },
+      };
+      expect(prefersLegacyAuthOverride('my-api;rev=2', revisionSection)).toBe(true);
+
+      // A base override must NOT leak into a revision publish (keys differ).
+      const baseSection = {
+        'my-api': {
+          properties: {
+            authenticationSettings: { oAuth2: { authorizationServerId: 'base-server' } },
+          },
+        },
+      };
+      expect(prefersLegacyAuthOverride('my-api;rev=2', baseSection)).toBe(false);
+    });
   });
 });