feat(commandRegistry): unify command envelope — origin, unit shorthand, always-convert

Shared command-dispatch layer used by every EVOLV node:
- Always-convert: numeric strings ("60") and {value:"60"} now normalise +
  convert like numbers; closes the gap where strings reached handlers raw.
- unit: 'm3/h' shorthand on descriptors; measure is derived from the unit
  (legacy units:{measure,default} still accepted, measure re-derived).
  Unrecognised declared unit throws at construction.
- msg.origin stamped on every dispatch (parent|GUI|fysical, default parent).
- Opt-in gated:true arbitration: accept only if origin in
  source.config.mode.allowedSources[currentMode]; advisory allow-all when a
  node has no mode model. Handles Set- or array-valued allowedSources.

+18 registry tests (45 total, green). All consumer nodes verified green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/nodered/commandRegistry.js

@@ -26,14 +26,60 @@ function _describeUnit(unit) {
     try { return convert().describe(unit); } catch (_) { return null; }
 }
 
+// A numeric scalar is a finite number, or a non-empty string that parses to a
+// finite number ("60", "1.5"). Node-RED inject/`change` nodes and upstream MQTT
+// payloads routinely arrive as strings; treating them as non-numeric here is the
+// gap that let values reach a handler unconverted.
+function _asNumber(x) {
+    if (typeof x === 'number') return Number.isFinite(x) ? x : null;
+    if (typeof x === 'string' && x.trim() !== '') {
+        const n = Number(x);
+        return Number.isFinite(n) ? n : null;
+    }
+    return null;
+}
+
 function _extractValueAndUnit(msg) {
     if (!msg || typeof msg !== 'object') return null;
     const p = msg.payload;
-    if (typeof p === 'number') return { value: p, unit: msg.unit };
-    if (p && typeof p === 'object' && typeof p.value === 'number') {
-        return { value: p.value, unit: p.unit ?? msg.unit };
+    if (p && typeof p === 'object') {
+        const value = _asNumber(p.value);
+        if (value === null) return null;
+        return { value, unit: p.unit ?? msg.unit };
     }
-    return null;
+    const value = _asNumber(p);
+    if (value === null) return null;
+    return { value, unit: msg.unit };
+}
+
+// Derive the dimensional measure (e.g. 'volumeFlowRate') from a unit string.
+// Returns null when convert doesn't recognise the unit.
+function _measureOf(unit) {
+    const desc = _describeUnit(unit);
+    return desc ? desc.measure : null;
+}
+
+// Command origin = which control authority issued this message (the rotatingMachine
+// `allowedSources` vocabulary: 'parent' = automation/parent controller, 'GUI' =
+// SCADA/HMI operator, 'fysical' = physical buttons). Default 'parent'. Named
+// `origin` on the message because `source` is already the domain instance handed
+// to handlers.
+const DEFAULT_ORIGIN = 'parent';
+
+function _resolveOrigin(msg, descriptor) {
+    const o = msg && typeof msg.origin === 'string' && msg.origin.trim() !== ''
+        ? msg.origin.trim()
+        : (descriptor.defaultOrigin || DEFAULT_ORIGIN);
+    return o;
+}
+
+// allowedSources values may be a Set (post config processing, as rotatingMachine
+// stores them) or a plain array (raw config / other nodes). Accept both.
+function _setHas(coll, value) {
+    if (!coll) return false;
+    if (typeof coll.has === 'function') return coll.has(value);
+    if (Array.isArray(coll)) return coll.includes(value);
+    return false;
 }
 
 class CommandRegistry {
@@ -76,6 +122,8 @@ class CommandRegistry {
             payloadSchema: cmd.payloadSchema || null,
             description: typeof cmd.description === 'string' ? cmd.description : null,
             units,
+            gated: cmd.gated === true,
+            defaultOrigin: typeof cmd.defaultOrigin === 'string' ? cmd.defaultOrigin : null,
             handler: cmd.handler,
         };
         this._byKey.set(cmd.topic, descriptor);
@@ -87,12 +135,25 @@ class CommandRegistry {
     }
 
     _validateUnits(cmd) {
-        if (cmd.units === undefined || cmd.units === null) return null;
-        const { measure, default: def } = cmd.units;
-        if (typeof measure !== 'string' || measure.length === 0 ||
-            typeof def !== 'string' || def.length === 0) {
+        // Two ways to declare the unit, normalised to the same internal shape:
+        //   unit: 'm3/h'                          (preferred — measure derived)
+        //   units: { default: 'm3/h' }            (measure derived)
+        //   units: { measure, default: 'm3/h' }   (legacy — measure ignored, derived)
+        // The measure is always derived from the unit so it can never drift from it.
+        let def;
+        if (typeof cmd.unit === 'string') def = cmd.unit;
+        else if (cmd.units === undefined || cmd.units === null) return null;
+        else if (typeof cmd.units === 'string') def = cmd.units;
+        else def = cmd.units.default;
+
+        if (typeof def !== 'string' || def.length === 0) {
             throw new TypeError(
-                `command '${cmd.topic}' units requires { measure: string, default: string }`);
+                `command '${cmd.topic}' requires a unit string (unit: 'm3/h' or units: { default: 'm3/h' })`);
+        }
+        const measure = _measureOf(def);
+        if (!measure) {
+            throw new TypeError(
+                `command '${cmd.topic}' declares unit '${def}' which convert does not recognise`);
         }
         return { measure, default: def };
     }
@@ -137,11 +198,31 @@ class CommandRegistry {
             return;
         }
         if (topic !== descriptor.topic) this._noteAlias(topic, descriptor.topic, log);
+        // Always stamp the command origin so handlers + gating can rely on it.
+        msg.origin = _resolveOrigin(msg, descriptor);
+        if (!this._originAllowed(descriptor, source, msg.origin, log)) return;
         if (descriptor.units) this._normaliseUnits(descriptor, msg, log);
         if (!this._validatePayload(descriptor, msg, log)) return;
         return descriptor.handler(source, msg, ctx);
     }
 
+    // Mode-gated control-authority arbitration. Opt-in per command via
+    // `gated: true`. The asset's mode (e.g. rotatingMachine's auto /
+    // virtualControl / fysicalControl) decides which origins it accepts via
+    // `source.config.mode.allowedSources[mode]`. Release = changing the mode.
+    // Nodes without a mode model are advisory (allow-all) so this is inert
+    // until a node opts in — never a silent behaviour change.
+    _originAllowed(descriptor, source, origin, log) {
+        if (!descriptor.gated) return true;
+        const allowedSources = source && source.config && source.config.mode
+            ? source.config.mode.allowedSources : null;
+        const mode = source ? source.currentMode : undefined;
+        if (!allowedSources || !mode) return true;     // no mode model → advisory
+        if (_setHas(allowedSources[mode], origin)) return true;
+        log.warn?.(`${descriptor.topic}: origin '${origin}' not allowed in mode '${mode}'`);
+        return false;
+    }
+
     _noteAlias(alias, canonical, log) {
         const prev = this._deprecationCounts.get(alias) || 0;
         this._deprecationCounts.set(alias, prev + 1);

test/basic/commandRegistry.basic.test.js

@@ -394,43 +394,163 @@ test('units: object payload {value} without unit falls back to default unit sile
     assert.equal(logger._calls.warn.length, 0);
 });
 
-test('units: non-numeric payload (no normalisation applied) passes through to handler', async () => {
+test('units: non-NUMERIC string payload (not normalisable) passes through to handler', async () => {
     const logger = makeLogger();
     const seen = [];
     const reg = createRegistry([{
         topic: 'set.demand',
-        units: { measure: 'volumeFlowRate', default: 'm3/h' },
+        unit: 'm3/h',
         handler: (_s, msg) => { seen.push(msg.payload); },
     }], { logger });
 
-    // string payload — not normalisable. Should not crash; handler still fires.
+    // non-numeric string — not normalisable. Should not crash; handler still fires.
     await reg.dispatch({ topic: 'set.demand', payload: 'magic' }, {}, {});
     assert.equal(seen.length, 1);
     assert.equal(seen[0], 'magic');
 });
 
-test('units: missing default field throws at construction', () => {
+test('units: NUMERIC string payload is always converted (closes the string gap)', async () => {
+    const logger = makeLogger();
+    const seen = [];
+    const reg = createRegistry([{
+        topic: 'set.demand',
+        unit: 'm3/h',
+        handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); },
+    }], { logger });
+
+    // "1" m3/s must convert to 3600 m3/h — same as the numeric-payload case.
+    await reg.dispatch({ topic: 'set.demand', payload: '1', unit: 'm3/s' }, {}, {});
+    assert.equal(seen.length, 1);
+    assert.ok(Math.abs(seen[0].payload - 3600) < 1e-6, `expected 3600, got ${seen[0].payload}`);
+    assert.equal(seen[0].unit, 'm3/h');
+    assert.equal(logger._calls.warn.length, 0);
+});
+
+test('units: { value: numeric-string } object payload is converted too', async () => {
+    const seen = [];
+    const reg = createRegistry([{
+        topic: 'set.pressure',
+        unit: 'Pa',
+        handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); },
+    }]);
+    await reg.dispatch({ topic: 'set.pressure', payload: { value: '5', unit: 'mbar' } }, {}, {});
+    assert.ok(Math.abs(seen[0].payload - 500) < 1e-6, `expected 500, got ${seen[0].payload}`);
+    assert.equal(seen[0].unit, 'Pa');
+});
+
+test('unit: shorthand declares the unit and DERIVES the measure', async () => {
+    const seen = [];
+    const reg = createRegistry([{
+        topic: 'set.demand',
+        unit: 'm3/h',
+        handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); },
+    }]);
+    await reg.dispatch({ topic: 'set.demand', payload: 1, unit: 'm3/s' }, {}, {});
+    assert.ok(Math.abs(seen[0].payload - 3600) < 1e-6);
+    assert.equal(seen[0].unit, 'm3/h');
+});
+
+test('units: { default } without measure derives the measure (measure no longer required)', () => {
+    const reg = createRegistry([{
+        topic: 'set.demand',
+        units: { default: 'm3/h' },
+        handler: () => {},
+    }]);
+    assert.deepEqual(reg.list()[0].units, { measure: 'volumeFlowRate', default: 'm3/h' });
+});
+
+test('units: legacy { measure, default } still works; measure is re-derived from the unit', () => {
+    const reg = createRegistry([{
+        topic: 'set.demand',
+        units: { measure: 'totallyWrong', default: 'm3/h' },
+        handler: () => {},
+    }]);
+    // declared measure ignored — derived from the unit so it can never drift.
+    assert.deepEqual(reg.list()[0].units, { measure: 'volumeFlowRate', default: 'm3/h' });
+});
+
+test('units: missing unit/default throws at construction', () => {
     assert.throws(() => createRegistry([{
         topic: 'set.demand',
         units: { measure: 'volumeFlowRate' },
         handler: () => {},
-    }]), /units requires/);
+    }]), /requires a unit string/);
 });
 
-test('units: missing measure field throws at construction', () => {
+test('units: unrecognised declared unit throws at construction (descriptor bug caught early)', () => {
     assert.throws(() => createRegistry([{
         topic: 'set.demand',
-        units: { default: 'm3/h' },
+        unit: 'flarbargs',
         handler: () => {},
-    }]), /units requires/);
+    }]), /convert does not recognise/);
 });
 
 test('units: descriptor.units surfaces in list() output', () => {
     const reg = createRegistry([
-        { topic: 'set.demand', units: { measure: 'volumeFlowRate', default: 'm3/h' }, handler: () => {} },
-        { topic: 'set.mode',                                                          handler: () => {} },
+        { topic: 'set.demand', unit: 'm3/h', handler: () => {} },
+        { topic: 'set.mode',                 handler: () => {} },
     ]);
     const list = reg.list();
     assert.deepEqual(list[0].units, { measure: 'volumeFlowRate', default: 'm3/h' });
     assert.equal(list[1].units, null);
 });
+
+// ---------------------------------------------------------------------------
+// origin — command provenance + mode-gated control-authority arbitration
+// ---------------------------------------------------------------------------
+
+test('origin: defaults to parent and is stamped on msg before the handler runs', async () => {
+    const seen = [];
+    const reg = createRegistry([{ topic: 'set.mode', handler: (_s, msg) => { seen.push(msg.origin); } }]);
+    await reg.dispatch({ topic: 'set.mode', payload: 'auto' }, {}, {});
+    assert.equal(seen[0], 'parent');
+});
+
+test('origin: explicit msg.origin is preserved and trimmed', async () => {
+    const seen = [];
+    const reg = createRegistry([{ topic: 'set.mode', handler: (_s, msg) => { seen.push(msg.origin); } }]);
+    await reg.dispatch({ topic: 'set.mode', payload: 'auto', origin: ' GUI ' }, {}, {});
+    assert.equal(seen[0], 'GUI');
+});
+
+test('origin: defaultOrigin descriptor overrides the global parent default', async () => {
+    const seen = [];
+    const reg = createRegistry([{ topic: 'set.x', defaultOrigin: 'fysical', handler: (_s, msg) => { seen.push(msg.origin); } }]);
+    await reg.dispatch({ topic: 'set.x' }, {}, {});
+    assert.equal(seen[0], 'fysical');
+});
+
+test('origin gating: non-gated command is never blocked, even with a mode model', async () => {
+    let invoked = false;
+    const reg = createRegistry([{ topic: 'set.x', handler: () => { invoked = true; } }]);
+    const source = { currentMode: 'fysicalControl', config: { mode: { allowedSources: { fysicalControl: ['fysical'] } } } };
+    await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, source, {});
+    assert.equal(invoked, true);
+});
+
+test('origin gating: gated command rejects an origin disallowed by the current mode', async () => {
+    const logger = makeLogger();
+    let invoked = false;
+    const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { invoked = true; } }], { logger });
+    const source = { currentMode: 'fysicalControl', config: { mode: { allowedSources: { fysicalControl: ['fysical'] } } } };
+    await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, source, {});
+    assert.equal(invoked, false);
+    assert.ok(logger._calls.warn.some((m) => m.includes("origin 'GUI' not allowed in mode 'fysicalControl'")));
+});
+
+test('origin gating: gated command accepts an allowed origin (Set or array allowedSources)', async () => {
+    let count = 0;
+    const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { count += 1; } }]);
+    const arraySrc = { currentMode: 'auto', config: { mode: { allowedSources: { auto: ['parent', 'GUI', 'fysical'] } } } };
+    const setSrc   = { currentMode: 'auto', config: { mode: { allowedSources: { auto: new Set(['parent', 'GUI', 'fysical']) } } } };
+    await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, arraySrc, {});
+    await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, setSrc, {});
+    assert.equal(count, 2);
+});
+
+test('origin gating: gated command on a node WITHOUT a mode model is advisory (allow-all)', async () => {
+    let invoked = false;
+    const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { invoked = true; } }]);
+    await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, { id: 'no-mode' }, {});
+    assert.equal(invoked, true);
+});