fix: connector review follow-ups (shell gating, nav cycles, batch errors, cleanups)

CONTENT-MODEL.md

@@ -23,7 +23,7 @@ storefront's `LayoutConfig`. A slot only renders if its template declares it. Yo
 | SAP template (page type) | SAP slot positions | Contentstack field uid |
 |---|---|---|
 | **Shell** (header/footer, every page) | `SiteLogo`, `SearchBox`, `MiniCart`, `NavigationBar`, `SiteContext`, `SiteLinks`, `HeaderLinks`, `Footer` | `site_logo`, `search_box`, `mini_cart`, `navigation_bar`, `site_context`, `site_links`, `header_links`, `footer` |
-| **LandingPage2Template** (home/landing) | `Section1`, `Section2`, `Section2A`, `Section2B`, `Section2C`, `Section3`, `Section4`, `Section5` | `section1`, `section2`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` |
+| **LandingPage2Template** (home/landing) | `Section1`, `Section2A`, `Section2B`, `Section2C`, `Section3`, `Section4`, `Section5` | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` |
 | **ContentPage1Template** (FAQ, terms, …) | `Section1`, `Section2A/B/C`, `Section3`, `BodyContent`, `SideContent` | `section1`, `section2_a/b/c`, `section3`, `body_content`, `side_content` |
 | **ProductDetailsPageTemplate** (PDP) | `Summary`, `UpSelling`, `CrossSelling`, `Tabs`, `PlaceholderContentSlot` | `summary`, `up_selling`, `cross_selling`, `tabs`, `placeholder_content_slot` |
 | **ProductListPageTemplate** (PLP/category/search) | `ProductLeftRefinements`, `ProductGridSlot`, `ProductListSlot`, `SearchResultsGridSlot` | `product_left_refinements`, `product_grid_slot`, `product_list_slot`, `search_results_grid_slot` |
@@ -49,7 +49,7 @@ Each **slot field is a multi-reference** to the component content types below.
 
 | Content type (uid) | Template | Slot fields (reference) |
 |---|---|---|
-| `landing_page` | LandingPage2Template | `section1`, `section2`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` |
+| `landing_page` | LandingPage2Template | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `section4`, `section5` |
 | `content_page` | ContentPage1Template | `section1`, `section2_a`, `section2_b`, `section2_c`, `section3`, `body_content`, `side_content` |
 | `product_page` | ProductDetailsPageTemplate | `summary`, `up_selling`, `cross_selling`, `tabs`, `placeholder_content_slot` |
 | `category_page` | ProductListPageTemplate | `product_left_refinements`, `product_grid_slot`, `product_list_slot`, `search_results_grid_slot` |

README.md

@@ -171,7 +171,6 @@ block, with each field's JSDoc on hover. The complete set:
 | `pageTypeMapping` | per-`PageType` `{ contentTypeUid, slugField?, sharedSlug? }` (shared-layout pages) |
 | `additionalSlotFields` | extra `{ fieldUid: 'SapSlotPosition' }` beyond the built-in slot map |
 | `componentContentType` | content type for standalone component lookups (else components ship in pages) |
-| `componentTypeMapping` | block uid → SAP typeCode (for author-named blocks without a `type_code`) |
 | `includeReferences` | reference fields to expand; defaults to all slot + header/footer fields |
 | `accessControl` | presentation-level gating — see below |
 | `timeoutMs` | `10000` — Delivery API call timeout |

import-export/starter-pack/content_types/landing_page.json

@@ -75,29 +75,6 @@
         "instruction": "Maps to SAP slot Section1. Top hero band. Leave empty to keep SAP's."
       }
     },
-    {
-      "display_name": "Section 2",
-      "uid": "section2",
-      "data_type": "reference",
-      "reference_to": [
-        "simple_responsive_banner_component",
-        "simple_banner_component",
-        "product_carousel_component",
-        "cms_paragraph_component",
-        "cms_tab_paragraph_component",
-        "cms_link_component",
-        "cms_flex_component"
-      ],
-      "mandatory": false,
-      "multiple": true,
-      "unique": false,
-      "non_localizable": false,
-      "field_metadata": {
-        "ref_multiple": true,
-        "ref_multiple_content_types": true,
-        "instruction": "Maps to SAP slot Section2. Leave empty to keep SAP's."
-      }
-    },
     {
       "display_name": "Section 2A",
       "uid": "section2_a",

import-export/starter-pack/generate-content-types.mjs

@@ -178,7 +178,9 @@ const contentTypes = [
     'CMS page on SAP LandingPage2Template (home / marketing landing).',
     [...pageMeta('LandingPage2Template'),
      slot('section1', 'Section 1', 'Section1', EDITORIAL, 'Top hero band. '),
-     slot('section2', 'Section 2', 'Section2'),
+     // NB: LandingPage2Template has no bare `Section2` render position (that
+     // belongs to CategoryPageTemplate), so this page type intentionally omits a
+     // `section2` slot field — only Section2A/2B/2C exist here.
      slot('section2_a', 'Section 2A', 'Section2A'),
      slot('section2_b', 'Section 2B', 'Section2B'),
      slot('section2_c', 'Section 2C', 'Section2C'),

src/client/contentstack-client.service.ts

@@ -54,11 +54,11 @@
   private _stack?: Stack;
 
   constructor(
     protected config: ContentstackConfig,
     protected transferState: TransferState,
     protected logger: LoggerService,
     protected restrictions: ContentstackRestrictionsService,
     @Inject(PLATFORM_ID) protected platformId: object,
   ) {}
 
   /** Lazily build (and memoize) the Delivery SDK stack from config. */
@@ -229,10 +229,13 @@
     title?: string,
     includeRefs: string[] = [],
     locale?: string,
+    access?: EntryAccessOptions,
   ): Observable<ContentstackCmsPageEntry | undefined> {
     const csLocale = this.resolveLocale(locale);
     const key = makeStateKey<ContentstackCmsPageEntry | undefined>(
-      `cs-global:${contentTypeUid}:${title ?? '*'}:${csLocale ?? '*'}`,
+      `cs-global:${contentTypeUid}:${title ?? '*'}:${csLocale ?? '*'}${this.restrictions.cacheKeySuffix(
+        access?.permissions,
+      )}`,
     );
     return this.withTransferState(key, () => {
       // Resolve the shell's component references inline (same as the page path),
@@ -252,7 +255,14 @@
         query.where('title', QueryOperation.EQUALS, title);
       }
       return query.find<ContentstackCmsPageEntry>().then((res) => {
-        const entry = res?.entries?.[0];
+        let entry = res?.entries?.[0];
+        // Filter gated content on every fetch (before the SSR TransferState write,
+        // and again on any client-side re-fetch) — the shell is gated like the
+        // page. gateRoot is false: the shell entry itself is never hidden, only
+        // its restricted nested components are stripped.
+        if (entry && access) {
+          entry = this.restrictions.sanitizeForTransfer(entry, access.permissions, false);
+        }
         if (entry && this.config.contentstack?.delivery?.livePreview) {
           this.tagForLivePreview(entry, contentTypeUid);
         }
@@ -324,30 +334,38 @@
         access?.permissions,
       )}`,
     );
-    return this.withTransferState(key, () => {
-      let entries = this.stack.contentType(contentTypeUid).entry();
-      if (csLocale) {
-        entries = entries.locale(csLocale);
-        if (this.includeFallback) {
-          entries = entries.includeFallback();
+    return this.withTransferState(
+      key,
+      () => {
+        let entries = this.stack.contentType(contentTypeUid).entry();
+        if (csLocale) {
+          entries = entries.locale(csLocale);
+          if (this.includeFallback) {
+            entries = entries.includeFallback();
+          }
         }
-      }
-      return entries
-        .query()
-        .where('uid', QueryOperation.INCLUDES, uids)
-        .find<ContentstackEntry>()
-        .then((res) => {
-          const list = res?.entries ?? [];
-          // Filter gated content BEFORE it is persisted to TransferState:
-          // each restricted entry is redacted to a tags-only stub so the adapter
-          // still counts it as "found" (no OCC refetch) without shipping content.
-          return access
-            ? list.map((entry) =>
-                this.restrictions.sanitizeForTransfer(entry, access.permissions, access.gateRoot),
-              )
-            : list;
-        });
-    });
+        return entries
+          .query()
+          .where('uid', QueryOperation.INCLUDES, uids)
+          .find<ContentstackEntry>()
+          .then((res) => {
+            const list = res?.entries ?? [];
+            // Filter gated content BEFORE it is persisted to TransferState:
+            // each restricted entry is redacted to a tags-only stub so the adapter
+            // still counts it as "found" (no OCC refetch) without shipping content.
+            return access
+              ? list.map((entry) =>
+                  this.restrictions.sanitizeForTransfer(entry, access.permissions, access.gateRoot),
+                )
+              : list;
+          });
+      },
+      // Failure fallback: an empty array, never `undefined` — the return type is
+      // `ContentstackEntry[]` and callers (component adapter) flat-map the result,
+      // so a failed batch must degrade to "no entries" (→ OCC/shell fallback),
+      // not to a value that throws on `.map`/`.filter`.
+      [],
+    );
   }
 
   /**

src/cms/access/contentstack-restrictions.service.spec.ts

@@ -110,6 +110,14 @@ describe('ContentstackRestrictionsService', () => {
       // Different audiences get different keys (no cross-serving).
       expect(svc.cacheKeySuffix(new Set(['_require-login']))).not.toBe(a);
     });
+
+    it('is injective even when a token contains the join delimiter', () => {
+      // Without per-token encoding, {'a|b'} and {'a','b'} would collide on `|`,
+      // letting one audience receive another's permission-filtered cache entry.
+      const single = svc.cacheKeySuffix(new Set(['a|b']));
+      const pair = svc.cacheKeySuffix(new Set(['a', 'b']));
+      expect(single).not.toBe(pair);
+    });
   });
 
   describe('redactEntry()', () => {

src/cms/access/contentstack-restrictions.service.ts

@@ -17,7 +17,7 @@
  */
 @Injectable({ providedIn: 'root' })
 export class ContentstackRestrictionsService {
   constructor(protected config: ContentstackConfig) {}
 
   /** Whether gating is switched on (`accessControl.enabled`). */
   enabled(): boolean {
@@ -117,7 +117,12 @@
     if (!permissions || !permissions.size) {
       return '';
     }
-    return `:acl=${[...permissions].sort().join('|')}`;
+    // Encode each token before joining so the `|` delimiter can never collide:
+    // `encodeURIComponent` percent-escapes any literal `|` (→ `%7C`), so the
+    // suffix is injective — `{'a|b'}` and `{'a','b'}` map to distinct keys, and a
+    // token that happens to contain the delimiter can't leak another audience's
+    // cached, permission-filtered payload.
+    return `:acl=${[...permissions].sort().map(encodeURIComponent).join('|')}`;
   }
 
   /** Whether a value is a Contentstack entry node (an object carrying a string uid). */

src/cms/adapters/contentstack-cms-component.adapter.spec.ts

@@ -1,4 +1,4 @@
-import { Observable, of } from 'rxjs';
+import { Observable, of, throwError } from 'rxjs';
 import { ContentstackCmsComponentAdapter } from './contentstack-cms-component.adapter';
 import { ContentstackRestrictionsService } from '../access/contentstack-restrictions.service';
 import { ContentstackComponentTypeRegistry } from '../model/contentstack-component-type.registry';
@@ -414,4 +414,49 @@ describe('ContentstackCmsComponentAdapter', () => {
       expect(occComponentAdapter.findComponentsByIds).not.toHaveBeenCalled();
     });
   });
+
+  describe('batch-fetch error handling (#3)', () => {
+    const entry = (uid: string, type: string) => ({
+      uid,
+      _content_type_uid: type,
+      created_at: '2026-01-01T00:00:00.000Z',
+    });
+
+    it('degrades a failed content-type group to empty instead of throwing, keeping healthy groups', () => {
+      // Two distinct learned types → two forkJoin branches; one fails outright.
+      const getEntriesByUids = jest.fn((type: string) =>
+        type === 'type_b'
+          ? throwError(() => new Error('CMS down'))
+          : of([entry('bltaaaa', 'type_a')]),
+      );
+      const { adapter } = create({
+        registry: { bltaaaa: 'type_a', bltbbbb: 'type_b' },
+        cs: { componentContentType: undefined }, // rely on learned types only
+        client: { getEntriesByUids },
+      });
+
+      let res!: { uid: string }[];
+      expect(() => {
+        res = firstValue(adapter.findComponentsByIds(['bltaaaa', 'bltbbbb'], ctx));
+      }).not.toThrow();
+      const uids = res.map((c) => c.uid);
+      expect(uids).toContain('bltaaaa'); // healthy group resolved
+      expect(uids).toContain('bltbbbb'); // failed group degraded to a benign shell
+    });
+
+    it('tolerates a client that emits a non-array (undefined) without crashing', () => {
+      const getEntriesByUids = jest.fn().mockReturnValue(of(undefined as never));
+      const { adapter } = create({
+        registry: { bltaaaa: 'type_a' },
+        cs: { componentContentType: undefined },
+        client: { getEntriesByUids },
+      });
+
+      let res!: { uid: string }[];
+      expect(() => {
+        res = firstValue(adapter.findComponentsByIds(['bltaaaa'], ctx));
+      }).not.toThrow();
+      expect(Array.isArray(res)).toBe(true);
+    });
+  });
 });

src/cms/adapters/contentstack-cms-component.adapter.ts

@@ -1,6 +1,6 @@
 import { Inject, Injectable, Optional } from '@angular/core';
 import { Observable, combineLatest, forkJoin, of } from 'rxjs';
-import { map, switchMap } from 'rxjs/operators';
+import { catchError, map, switchMap } from 'rxjs/operators';
 import {
   CmsComponent,
   CmsComponentAdapter,
@@ -39,10 +39,10 @@
 @Injectable({ providedIn: 'root' })
 export class ContentstackCmsComponentAdapter implements CmsComponentAdapter {
   constructor(
     protected client: ContentstackClientService,
     protected normalizer: ContentstackCmsComponentNormalizer,
     protected config: ContentstackConfig,
     protected logger: LoggerService,
     protected languageService: LanguageService,
     protected occComponentAdapter: OccCmsComponentAdapter,
     protected restrictions: ContentstackRestrictionsService,
@@ -168,13 +168,24 @@
       switchMap(([locale, permissions]) =>
         forkJoin(
           [...byType.entries()].map(([type, uids]) =>
-            permissions
+            (permissions
               ? this.client.getEntriesByUids(type, uids, locale, { permissions, gateRoot: true })
-              : this.client.getEntriesByUids(type, uids, locale),
+              : this.client.getEntriesByUids(type, uids, locale)
+            ).pipe(
+              // Defense in depth: one content-type group failing must not fail the
+              // whole batch (forkJoin errors if any source errors) — degrade that
+              // group to no entries so its ids fall back to OCC/shells below.
+              catchError(() => of([] as ContentstackEntry[])),
+            ),
           ),
         ).pipe(
           switchMap((groups: ContentstackEntry[][]) => {
-            const entries = groups.flat();
+            // Guard against a malformed group (e.g. a client that ever emits a
+            // non-array) so `.flat()`/`.map()` can't throw on `undefined`.
+            const entries = groups
+              .filter((g): g is ContentstackEntry[] => Array.isArray(g))
+              .flat()
+              .filter((e): e is ContentstackEntry => !!e);
             const accessible = permissions
               ? entries.filter((entry) => this.restrictions.isEntryAccessible(entry, permissions))
               : entries;

src/cms/adapters/contentstack-cms-page.adapter.spec.ts

@@ -331,6 +331,64 @@ describe('ContentstackCmsPageAdapter', () => {
       expect(client.getGlobalSlots).not.toHaveBeenCalled();
     });
 
+    describe('gating the shell (#4)', () => {
+      const GATING = {
+        accessControl: { enabled: true, accessField: 'access_tags', rolePrefix: '_require-' },
+      };
+
+      it('threads permissions + gateRoot:false into the shell fetch when gating is on', () => {
+        const getGlobalSlots = jest.fn().mockReturnValue(of(undefined));
+        const { adapter } = create({
+          cs: { ...GATING, globalSlots: { contentType: 'global_slots', title: 'Global' } },
+          user: { roles: ['b2badmingroup'] },
+          client: { getPageBySlug: jest.fn().mockReturnValue(of(undefined)), getGlobalSlots },
+        });
+
+        firstValue(adapter.load(ctx('home')));
+        const access = getGlobalSlots.mock.calls[0][4];
+        expect(access?.permissions).toBeInstanceOf(Set);
+        expect(access?.permissions.has('_require-login')).toBe(true);
+        expect(access?.permissions.has('_require-b2badmingroup')).toBe(true);
+        // The shell root is never hidden — only restricted nested components drop.
+        expect(access?.gateRoot).toBe(false);
+      });
+
+      it('passes the permission set into buildStructure so restricted shell components drop on CSR', () => {
+        const buildStructure = jest.fn().mockReturnValue({ slots: {}, components: [] });
+        const { adapter } = create({
+          cs: { ...GATING, globalSlots: { contentType: 'global_slots' } },
+          user: { roles: [] }, // logged in, no roles
+          client: {
+            getPageBySlug: jest.fn().mockReturnValue(of(undefined)),
+            getGlobalSlots: jest.fn().mockReturnValue(of({ uid: 'global-entry' })),
+          },
+          normalizer: { buildStructure },
+          occ: {
+            load: jest
+              .fn()
+              .mockReturnValue(of({ page: { template: 'T', slots: {} }, components: [] })),
+          },
+        });
+
+        firstValue(adapter.load(ctx('home')));
+        const permissions = buildStructure.mock.calls[0][1] as Set<string>;
+        expect(permissions).toBeInstanceOf(Set);
+        expect(permissions.has('_require-login')).toBe(true);
+      });
+
+      it('omits the access arg (unchanged behavior) when gating is off', () => {
+        const getGlobalSlots = jest.fn().mockReturnValue(of(undefined));
+        const { adapter } = create({
+          cs: { globalSlots: { contentType: 'global_slots' } },
+          client: { getPageBySlug: jest.fn().mockReturnValue(of(undefined)), getGlobalSlots },
+        });
+
+        firstValue(adapter.load(ctx('home')));
+        // 4 args only — no 5th access argument when access control is off.
+        expect(getGlobalSlots.mock.calls[0][4]).toBeUndefined();
+      });
+    });
+
     it('always requests the flat-nav include chain for every nav field, independent of depth', () => {
       const { adapter, client } = create({
         cs: { globalSlots: { contentType: 'global_slots' } },