fix: explicit per-field semantics for mergeSiteConfig app merging

App fields fall into three groups with distinct merge semantics:
dictionaries (config, provides) are usefully deep-merged across calls;
arrays of self-contained module declarations (slots, routes, providers,
externalScripts) only make sense as full replacements, since element-wise
merging mashes positionally-corresponding items from unrelated apps;
appId is the merge key. The previous lodash.merge of whole App objects
treated all of these uniformly, doing the right thing for config and
something incoherent (or actively harmful) for the others.

Replace the generic deep-merge with mergeApps/mergeApp that name those
semantics explicitly and use Object.getOwnPropertyDescriptors so own
properties (including getters) survive the merge. The descriptor-
preserving copy fixes a latent footgun for any App that uses a lazy
field: under the old behavior the getter was invoked mid-merge against
a still-empty siteConfig.apps and its return frozen onto the merged
copy. createLegacyPluginApp's slots getter is the first concrete victim,
but the contract now holds for any future lazy field.

Add a regression test asserting a lazy slots getter remains a getter
after mergeSiteConfig.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: arbrandes

runtime/config/index.test.ts

@@ -192,6 +192,31 @@ describe('mergeSiteConfig', () => {
       const app = getSiteConfig().apps!.find(a => a.appId === 'test-app')!;
       expect(app.config!.NESTED).toEqual({ a: 1, b: 3, c: 4 });
     });
+
+    it('should preserve a lazy `slots` getter through the merge', () => {
+      /* Regression: lodash.merge invokes any getter on the source app while
+       * `siteConfig.apps` is still the pre-merge value, then snapshots the
+       * return value onto the merged copy. Apps with lazy `slots` getters
+       * (e.g. createLegacyPluginApp) need the getter to survive so it can
+       * resolve later, against the post-merge sibling apps. */
+      let getterCalls = 0;
+      const lazyApp: any = {
+        appId: 'lazy-app',
+        get slots() {
+          getterCalls += 1;
+          return [{ slotId: 'fake.slot', id: 'w', op: 'widgetAppend', element: null }];
+        },
+      };
+
+      mergeSiteConfig({ apps: [lazyApp] });
+
+      const merged = getSiteConfig().apps!.find(a => a.appId === 'lazy-app')!;
+      const callsAfterMerge = getterCalls;
+      // Reading slots should still hit the getter (i.e. it survived as a getter).
+      void merged.slots;
+      void merged.slots;
+      expect(getterCalls).toBeGreaterThan(callsAfterMerge);
+    });
   });
 
   describe('app merging (limitAppMergeToConfig: true)', () => {

runtime/config/index.ts

@@ -104,6 +104,7 @@ import isEqual from 'lodash/isEqual';
 import keyBy from 'lodash/keyBy';
 import merge from 'lodash/merge';
 import {
+  App,
   AppConfig,
   EnvironmentTypes,
   SiteConfig
@@ -226,11 +227,7 @@ export function mergeSiteConfig(
 
   // if we're doing a full merge, merge the objects
   if (!limitAppMergeToConfig) {
-    siteConfig.apps = Object.values(merge(
-      {},
-      keyBy(siteConfig.apps || [], 'appId'),
-      keyBy(newApps, 'appId')
-    ));
+    siteConfig.apps = mergeApps(siteConfig.apps || [], newApps);
     publish(CONFIG_CHANGED);
     return;
   }
@@ -243,18 +240,90 @@ export function mergeSiteConfig(
   }
 
   // handle config-only merging
-  const newAppsById = keyBy(newApps, 'appId');
-  siteConfig.apps = siteConfig.apps.map((app) => {
-    const newApp = newAppsById[app.appId];
-    if (!newApp?.config) {
-      return app;
-    }
-    return { ...app, config: merge({}, app.config, newApp.config) };
-  });
+  siteConfig.apps = mergeApps(siteConfig.apps, newApps, { configOnly: true });
 
   publish(CONFIG_CHANGED);
 }
 
+/*
+ * Merge two App[] by appId. Existing apps stay in their original positions
+ * (with their pair-merged counterpart from `newApps` substituted in when
+ * present); apps in `newApps` not already in `oldApps` append at the end.
+ * With `{ configOnly: true }`, no apps are added and apps not appearing in
+ * `newApps` pass through unchanged. Per-pair merging is delegated to
+ * `mergeApp`.
+ */
+function mergeApps(
+  oldApps: App[],
+  newApps: App[],
+  options: { configOnly?: boolean } = {},
+): App[] {
+  const incomingByAppId = keyBy(newApps, 'appId');
+
+  // Phase 1: walk existing apps in their original order, pair-merging any
+  // that have a counterpart in newApps.
+  const updatedExisting = oldApps.map((oldApp) => {
+    const newApp = incomingByAppId[oldApp.appId];
+    return newApp ? mergeApp(oldApp, newApp, options) : oldApp;
+  });
+
+  // configOnly mode never adds apps, so we're done.
+  if (options.configOnly) {
+    return updatedExisting;
+  }
+
+  // Phase 2: append apps from newApps that weren't already in oldApps.
+  const existingIds = new Set(oldApps.map((a) => a.appId));
+  const additions = newApps.filter((a) => !existingIds.has(a.appId));
+  return [...updatedExisting, ...additions];
+}
+
+/*
+ * Merge a pair of Apps with the same appId. Deep-merges `config` (and, in the
+ * full-merge case, `provides`); other fields take `newApp`'s value verbatim.
+ * The result is built via `Object.getOwnPropertyDescriptors` so any lazy
+ * getters survive: a snapshot via `lodash.merge` or spread would invoke the
+ * getter at merge time and freeze its return value, which is typically empty
+ * mid-init. Per-field replacement is also the only sensible behavior for the
+ * array fields (`slots`/`routes`/`providers`/`externalScripts`), which don't
+ * survive element-wise merging anyway.
+ */
+function mergeApp(
+  oldApp: App,
+  newApp: App,
+  options: { configOnly?: boolean } = {},
+): App {
+  // configOnly mode: preserve `oldApp` (identity, slots, etc.) and deep-merge
+  // only `newApp.config` on top.
+  if (options.configOnly) {
+    if (!newApp.config) {
+      return oldApp;
+    }
+    return cloneAppDescriptors(oldApp, {
+      config: merge({}, oldApp.config, newApp.config),
+    });
+  }
+
+  // Full mode: take `newApp` (identity, slots, etc.) and deep-merge `config`
+  // and `provides` from `oldApp`. Other fields take `newApp`'s value verbatim.
+  const deepMerged: Record<string, unknown> = {};
+  if (oldApp.config !== undefined || newApp.config !== undefined) {
+    deepMerged.config = merge({}, oldApp.config, newApp.config);
+  }
+  if (oldApp.provides !== undefined || newApp.provides !== undefined) {
+    deepMerged.provides = merge({}, oldApp.provides, newApp.provides);
+  }
+  return cloneAppDescriptors(newApp, deepMerged);
+}
+
+function cloneAppDescriptors(source: App, overrides: Record<string, unknown>): App {
+  const descriptors = Object.getOwnPropertyDescriptors(source);
+  for (const [key, value] of Object.entries(overrides)) {
+    descriptors[key] = { value, writable: true, enumerable: true, configurable: true };
+  }
+  return Object.create(Object.getPrototypeOf(source), descriptors) as App;
+}
+
 const appConfigs: Record<string, AppConfig> = {};
 
 /**