From 84a4430266c05d7a3cf1c8ece8739c6bd70814d6 Mon Sep 17 00:00:00 2001
From: znetsixe <zn375ix3@gmail.com>
Date: Tue, 12 May 2026 15:06:43 +0200
Subject: [PATCH] fix(configManager): deep-merge domainConfig so general.id and
 asset.model survive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

buildConfig() was doing Object.assign(config, domainConfig) — a shallow
merge. When buildDomainConfig returned subsets like {general: {unit}} or
{asset: {curveUnits}}, the assign replaced config.general / config.asset
wholesale, wiping general.id (nodeId), general.name, asset.model, and
asset.supplier that buildConfig had just populated.

Downstream consequences:
- MGC.onRegister('machine') keys by config.general.id; two children with
  the same null id collide and the second registration is rejected.
- rotatingMachine curve-lookup uses asset.model; with model "Unknown" the
  pump curve fails to resolve.

Replace Object.assign with an in-place recursive _deepMerge: arrays and
primitives in src replace dst, plain objects merge key-by-key so siblings
on dst survive.

Two regression tests added — single-config field survival and the MGC
multi-child id-collision case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/configs/index.js        | 33 +++++++++++++++++++++++++++--
 test/config-manager.test.js | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/src/configs/index.js b/src/configs/index.js
index c34fada..ed879ca 100644
--- a/src/configs/index.js
+++ b/src/configs/index.js
@@ -130,12 +130,41 @@ class ConfigManager {
             };
         }
 
-        // Merge domain-specific sections
-        Object.assign(config, domainConfig);
+        // Merge domain-specific sections. Must be a DEEP merge: domainConfig
+        // commonly returns subsets of `general` / `asset` (e.g. {general:
+        // {unit}}, {asset: {curveUnits}}) and a shallow assign would wipe out
+        // sibling keys this method just populated — notably `general.id`
+        // (nodeId) and `asset.model`, causing child-registration id collisions
+        // and curve-lookup failures downstream.
+        ConfigManager._deepMerge(config, domainConfig);
 
         return config;
     }
 
+    static _isPlainObject(v) {
+        return Object.prototype.toString.call(v) === '[object Object]';
+    }
+
+    /**
+     * In-place recursive merge. Arrays and primitives in `src` replace `dst`;
+     * plain objects are merged key-by-key so siblings on `dst` survive.
+     */
+    static _deepMerge(dst, src) {
+        if (!ConfigManager._isPlainObject(src)) return dst;
+        for (const key of Object.keys(src)) {
+            const v = src[key];
+            if (Array.isArray(v)) {
+                dst[key] = [...v];
+            } else if (ConfigManager._isPlainObject(v)) {
+                if (!ConfigManager._isPlainObject(dst[key])) dst[key] = {};
+                ConfigManager._deepMerge(dst[key], v);
+            } else {
+                dst[key] = v;
+            }
+        }
+        return dst;
+    }
+
     /**
      * Migrate a config object from one version to another by applying
      * registered migration functions in sequence.
diff --git a/test/config-manager.test.js b/test/config-manager.test.js
index e7147a1..3ac5e43 100644
--- a/test/config-manager.test.js
+++ b/test/config-manager.test.js
@@ -3,6 +3,48 @@ const assert = require('node:assert/strict');
 
 const ConfigManager = require('../src/configs/index.js');
 
+test('buildConfig deep-merges domainConfig so general.id and asset.model survive', () => {
+  // Regression: previously this used Object.assign (shallow), so a
+  // buildDomainConfig that returned {general:{unit:'m3/h'}, asset:{curveUnits:...}}
+  // wiped general.id (→ null via schema default) and asset.model (→ "Unknown"),
+  // which collapsed MGC child registration (id collision) and broke curve loading.
+  const manager = new ConfigManager('.');
+  const uiConfig = {
+    name: 'PumpA',
+    model: 'hidrostal-H05K-S03R',
+    supplier: 'Hidrostal',
+    category: 'pump',
+    assetType: 'Centrifugal',
+    unit: 'm3/h',
+  };
+  const domainConfig = {
+    general: { unit: 'm3/h' },
+    asset:   { curveUnits: { pressure: 'mbar', flow: 'm3/h', power: 'kW', control: '%' } },
+  };
+  const cfg = manager.buildConfig('rotatingMachine', uiConfig, 'node-abc-123', domainConfig);
+
+  assert.equal(cfg.general.id,   'node-abc-123', 'general.id must come from nodeId');
+  assert.equal(cfg.general.name, 'PumpA',         'general.name must come from uiConfig.name');
+  assert.equal(cfg.general.unit, 'm3/h',          'general.unit must come from domainConfig');
+  assert.equal(cfg.asset.model,    'hidrostal-H05K-S03R', 'asset.model must come from uiConfig.model');
+  assert.equal(cfg.asset.supplier, 'Hidrostal',           'asset.supplier must survive merge');
+  assert.deepEqual(cfg.asset.curveUnits, domainConfig.asset.curveUnits,
+    'asset.curveUnits must come from domainConfig');
+});
+
+test('buildConfig: two nodes with same uiConfig keep distinct general.id', () => {
+  // Regression: MGC.onRegister('machine') keys by config.general.id; if two
+  // machines collapse to the same id (e.g. null), the second is rejected.
+  const manager = new ConfigManager('.');
+  const ui = { model: 'hidrostal-H05K-S03R', supplier: 'Hidrostal', unit: 'm3/h' };
+  const dom = { general: { unit: 'm3/h' }, asset: { curveUnits: { pressure: 'mbar' } } };
+  const a = manager.buildConfig('rotatingMachine', ui, 'red-node-A', dom);
+  const b = manager.buildConfig('rotatingMachine', ui, 'red-node-B', dom);
+  assert.notEqual(a.general.id, b.general.id);
+  assert.equal(a.general.id, 'red-node-A');
+  assert.equal(b.general.id, 'red-node-B');
+});
+
 test('can read known config and report existence', () => {
   const manager = new ConfigManager('.');
   assert.equal(manager.hasConfig('measurement'), true);