瀏覽代碼

feat: a Configurator node that supplies settings to other nodes

Lifts settings out of a node into a node of its own, so variations can be
duplicated and swapped, and settings shared by several nodes - serverUrl,
credentialId - are written once. A separate node type from Set, copied from it:
Set puts values into the data stream, Configurator replaces settings on the
nodes it is connected to. Set is untouched.

A config edge is marked by its TARGET INPUT, not by the source's output. The
design said to use a _config marker the way _pause and _stop are marked, and
that turned out not to work: a Configurator on a branch that was not taken never
runs and so has no output to inspect, yet the engine still has to know that edge
carried configuration - otherwise the target is skipped along with it and
variation-by-Switch cannot work at all. The connection is present either way, so
the connection carries the meaning. The node still returns _config, and the
engine reads it when present.

Semantics:

- The overlay is applied last and verbatim, after the node's own expressions.
  Its values were already computed when the Configurator ran, and evaluating
  them twice would mangle any text containing braces - a prompt being exactly
  that field.
- A Configurator that was skipped contributes nothing and does not skip its
  target. Config edges are invisible to the skip decision entirely, including
  to the has-connections test, so a node whose only edge supplies configuration
  is not mistaken for one whose data source went inactive.
- Values from several live Configurators merge. The same key from two of them
  fails, naming the key and both nodes, rather than being resolved by an
  execution order nobody can see.
- A key the target's schema does not have is reported in _ignoredConfigKeys,
  not fatal. One shared Configurator legitimately feeds node types that accept
  different subsets of its keys, so failing would make the shared case
  impossible.
- What was applied is recorded on the node's output as _appliedConfig. Once
  settings can come from elsewhere, a node's stored config no longer says what
  it ran with, and this is the only thing that closes that gap.

Applied in BOTH walks. executeLoopBody has now diverged seven times by being
forgotten; this is not the eighth.

Values are coerced from the editor's strings to what they plainly are, so "28"
reaches a numeric setting as 28. Only unambiguous forms convert - "1.2.3" and
"007" stay strings.

Verified: an overlay replaces the node's own values; a Switch selects between
two variations with the unchosen one skipped and the target still running; two
Configurators claiming one key fail naming both; an unknown key is ignored and
reported. Full suite 52/52.

Editor work - picking a setting from a connected node, disabling it on the
target, and the unused/incompatible warnings - is not in this commit.
fszontagh 1 月之前
父節點
當前提交
811a1b9f65

+ 138 - 0
nodes/core/configurator.js

@@ -0,0 +1,138 @@
+/**
+ * @node configurator
+ * @name Configurator
+ * @category flow-control
+ * @version 1.0.0
+ * @description Hold settings for other nodes, so variations can be swapped and shared settings written once
+ * @icon sliders-horizontal
+ */
+
+const configSchema = {
+    type: 'object',
+    properties: {
+        settings: {
+            type: 'array',
+            title: 'Settings',
+            description: 'The settings to apply to every node this is connected to. Names must match the target node\'s own setting names - open a connected node and copy one across rather than typing it',
+            items: {
+                type: 'object',
+                properties: {
+                    name: {
+                        type: 'string',
+                        title: 'Setting',
+                        description: 'The target node\'s setting name, such as prompt, steps or credentialId'
+                    },
+                    value: {
+                        type: 'string',
+                        title: 'Value',
+                        description: 'A literal value, or an expression such as {{data.result.prompt}}. Expressions are resolved here, once, before the value reaches the target'
+                    }
+                }
+            }
+        },
+        label: {
+            type: 'string',
+            title: 'Label',
+            description: 'What this set of settings is for, such as "Portrait preset" or "Production server". Shown on the target node beside the settings it supplies'
+        }
+    }
+};
+
+const inputSchema = {
+    type: 'object',
+    properties: {
+        data: { type: 'any' }
+    }
+};
+
+const outputSchema = {
+    type: 'object',
+    properties: {
+        _config: { type: 'object', description: 'The settings, as the engine consumes them' },
+        settings: { type: 'object', description: 'The same settings, readable in the run log' },
+        count: { type: 'number', description: 'How many settings were supplied' },
+        label: { type: 'string' }
+    }
+};
+
+// Values are typed as strings in the editor, but a target's setting is often a
+// number or a boolean - steps is a number, diffusion_flash_attn a boolean. A
+// string "28" reaching a numeric field is the kind of mismatch that is accepted
+// silently and then behaves oddly much later, so an unambiguous literal is
+// converted to what it plainly is.
+//
+// Only exact forms convert. "1.2.3" stays a string, and so does "007", because
+// a leading zero usually means an identifier rather than the number seven.
+function coerce(value) {
+    if (typeof value !== 'string') {
+        return value;
+    }
+
+    const trimmed = value.trim();
+    if (trimmed === '') {
+        return value;
+    }
+    if (trimmed === 'true') return true;
+    if (trimmed === 'false') return false;
+    if (trimmed === 'null') return null;
+
+    if (/^-?(0|[1-9][0-9]*)(\.[0-9]+)?$/.test(trimmed)) {
+        const asNumber = Number(trimmed);
+        if (isFinite(asNumber)) {
+            return asNumber;
+        }
+    }
+
+    return value;
+}
+
+async function execute(config, input, context) {
+    const settings = config.settings || [];
+    const label = config.label || '';
+
+    const values = {};
+    let count = 0;
+
+    for (let i = 0; i < settings.length; i++) {
+        const entry = settings[i] || {};
+        const name = String(entry.name || '').trim();
+
+        if (!name) {
+            continue;
+        }
+        // A name beginning with an underscore would collide with the engine's
+        // own markers, which are stripped rather than applied.
+        if (name.charAt(0) === '_') {
+            throw new Error('Configurator: "' + name + '" cannot be used as a setting name - ' +
+                'names beginning with an underscore are reserved by the engine');
+        }
+        if (Object.prototype.hasOwnProperty.call(values, name)) {
+            // Inside one Configurator this is a plain mistake with a plain fix,
+            // and the later entry silently winning is exactly the behaviour
+            // this node exists to remove.
+            throw new Error('Configurator: "' + name + '" is set twice in this node. ' +
+                'Remove one of them');
+        }
+
+        values[name] = coerce(entry.value);
+        count++;
+    }
+
+    if (count === 0) {
+        smartbotic.log.warn('Configurator: no settings to apply' + (label ? ' (' + label + ')' : ''));
+    } else {
+        smartbotic.log.info('Configurator: supplying ' + count + ' setting(s)' +
+            (label ? ' (' + label + ')' : '') + ': ' + Object.keys(values).join(', '));
+    }
+
+    // _config is what the engine reads. The plain copy alongside it is what a
+    // person reads in the run log, where the underscore-prefixed key is stripped.
+    return {
+        _config: values,
+        settings: values,
+        count: count,
+        label: label
+    };
+}
+
+module.exports = { configSchema, inputSchema, outputSchema, execute };

+ 183 - 3
src/runner/workflow_engine.cpp

@@ -10,6 +10,12 @@
 
 namespace smartbotic::runner {
 
+// Defined further down, next to the rest of the configuration-overlay code.
+static std::vector<std::string> applyConfigOverlay(nlohmann::json& config,
+                                                   const nlohmann::json& overlay,
+                                                   const nlohmann::json& config_schema);
+
+
 using namespace common;
 
 std::string nodeStatusToString(NodeStatus status) {
@@ -680,7 +686,26 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 }
                 evaluated_node.config = evaluateExpressions(defaulted_config, input, result.node_results, workflow);
 
-                if (!t_disabled_reference_error.empty()) {
+                std::string overlay_conflict;
+                auto overlay = collectConfigOverlay(node_id, workflow, result.node_results,
+                                                    overlay_conflict);
+                std::vector<std::string> overlay_ignored;
+                if (overlay_conflict.empty()) {
+                    overlay_ignored = applyConfigOverlay(
+                        evaluated_node.config, overlay,
+                        node_def_for_defaults ? node_def_for_defaults->config_schema
+                                              : nlohmann::json::object());
+                }
+
+                if (!overlay_conflict.empty()) {
+                    node_result.node_id = node_id;
+                    node_result.status = NodeStatus::Failed;
+                    node_result.input = input;
+                    node_result.output = nlohmann::json::object();
+                    node_result.error = overlay_conflict;
+                    node_result.started_at = TimeUtils::nowMs();
+                    node_result.finished_at = node_result.started_at;
+                } else if (!t_disabled_reference_error.empty()) {
                     node_result.node_id = node_id;
                     node_result.status = NodeStatus::Failed;
                     node_result.input = input;
@@ -692,6 +717,18 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     node_result = executeNode(evaluated_node, input, result.execution_id, workflow,
                                                node_def_for_defaults);
                 }
+
+                // Record what was applied. Once a node's settings can come from
+                // elsewhere, its stored config no longer says what it ran with,
+                // and this is the only place that closes that gap.
+                if (!overlay.empty() && node_result.status == NodeStatus::Completed) {
+                    node_result.output["_appliedConfig"] = overlay;
+                    if (!overlay_ignored.empty()) {
+                        node_result.output["_ignoredConfigKeys"] = overlay_ignored;
+                        LOG_WARN("Node {} was given {} setting(s) it does not have",
+                                 node_id, overlay_ignored.size());
+                    }
+                }
                 result.node_results[node_id] = node_result;
 
                 if (callback) {
@@ -1539,6 +1576,110 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
     return result;
 }
 
+// A connection landing on this reserved input carries configuration for the
+// target rather than data for it.
+//
+// The marker lives on the CONNECTION, not on the source's output, because a
+// Configurator sitting on a branch that was not taken never runs and so has no
+// output to inspect. The engine still has to know that edge was configuration -
+// otherwise the target would be skipped along with it - and only the connection
+// is present either way.
+static constexpr const char* kConfigInput = "config";
+
+
+// Apply a Configurator's values over a node's config, after the node's own
+// expressions have been evaluated.
+//
+// Last and verbatim, deliberately. The values were already computed when the
+// Configurator ran, so evaluating them again would mangle any text containing
+// braces - and a prompt is exactly the field where that would happen.
+//
+// A key the node's schema does not have is reported rather than fatal: one
+// shared Configurator legitimately feeds several node types that accept
+// different subsets of its keys, so failing would make the shared case
+// impossible. Reporting keeps a typo visible instead of silent.
+static std::vector<std::string> applyConfigOverlay(nlohmann::json& config,
+                                                   const nlohmann::json& overlay,
+                                                   const nlohmann::json& config_schema) {
+    std::vector<std::string> ignored;
+    if (!overlay.is_object() || overlay.empty()) {
+        return ignored;
+    }
+
+    const bool have_schema = config_schema.is_object() &&
+                             config_schema.contains("properties") &&
+                             config_schema["properties"].is_object();
+
+    for (auto it = overlay.begin(); it != overlay.end(); ++it) {
+        if (have_schema && !config_schema["properties"].contains(it.key())) {
+            ignored.push_back(it.key());
+            continue;
+        }
+        config[it.key()] = it.value();
+    }
+    return ignored;
+}
+
+nlohmann::json WorkflowEngine::collectConfigOverlay(
+    const std::string& node_id,
+    const Workflow& workflow,
+    const std::unordered_map<std::string, NodeExecutionResult>& results,
+    std::string& conflict_error) {
+
+    nlohmann::json overlay = nlohmann::json::object();
+    // Which Configurator supplied each key, so a clash can name both of them
+    // rather than reporting that something, somewhere, set it twice.
+    std::unordered_map<std::string, std::string> supplied_by;
+
+    for (const auto& conn : workflow.connections) {
+        if (conn.target_node_id != node_id || conn.target_input != kConfigInput) {
+            continue;
+        }
+
+        auto it = results.find(conn.source_node_id);
+        if (it == results.end() || it->second.status != NodeStatus::Completed) {
+            // Skipped, disabled or failed: contributes nothing. This is how a
+            // Switch picks one variation out of several - the ones it did not
+            // choose simply do not contribute.
+            continue;
+        }
+
+        const auto& output = it->second.output;
+        const nlohmann::json* values = nullptr;
+        if (output.contains("_config") && output["_config"].is_object()) {
+            values = &output["_config"];
+        } else if (output.is_object()) {
+            values = &output;
+        }
+        if (values == nullptr) {
+            continue;
+        }
+
+        for (auto value_it = values->begin(); value_it != values->end(); ++value_it) {
+            const std::string& key = value_it.key();
+            if (!key.empty() && key[0] == '_') {
+                continue;  // engine plumbing, never a setting
+            }
+
+            auto existing = supplied_by.find(key);
+            if (existing != supplied_by.end() && existing->second != conn.source_node_id) {
+                // Two live Configurators setting the same key is a contradiction
+                // with no correct answer, so it is reported rather than resolved
+                // by an order nobody can see on the canvas.
+                conflict_error = "Node \"" + node_id + "\" is configured twice for \"" + key +
+                                 "\": by \"" + existing->second + "\" and by \"" +
+                                 conn.source_node_id + "\". Remove it from one of them";
+                return overlay;
+            }
+
+            overlay[key] = value_it.value();
+            supplied_by[key] = conn.source_node_id;
+        }
+    }
+
+    return overlay;
+}
+
 nlohmann::json WorkflowEngine::collectNodeInput(
     const std::string& node_id,
     const Workflow& workflow,
@@ -1550,6 +1691,14 @@ nlohmann::json WorkflowEngine::collectNodeInput(
 
     for (const auto& conn : workflow.connections) {
         if (conn.target_node_id == node_id) {
+            // Configuration edges are invisible here, including to the skip
+            // decision below. A Configurator that was not chosen must not drag
+            // its target down with it - the target still runs, just without
+            // those values.
+            if (conn.target_input == kConfigInput) {
+                continue;
+            }
+
             auto it = results.find(conn.source_node_id);
             if (it != results.end()) {
                 // Check if source node was skipped - propagate skip
@@ -1620,7 +1769,10 @@ nlohmann::json WorkflowEngine::collectNodeInput(
         // Check if there were any connections at all
         bool has_connections = false;
         for (const auto& conn : workflow.connections) {
-            if (conn.target_node_id == node_id) {
+            // Config edges do not count. A node whose only connection supplies
+            // configuration has no data source, which is not the same as having
+            // a data source that went inactive.
+            if (conn.target_node_id == node_id && conn.target_input != kConfigInput) {
                 has_connections = true;
                 break;
             }
@@ -2177,8 +2329,29 @@ bool WorkflowEngine::executeLoopBody(
             }
             evaluated_body_node.config = evaluateExpressions(defaulted_body_config, node_input, merged_results, workflow);
 
+            // The same overlay the main walk applies. Forgetting it here is how
+            // this walk has diverged seven times already.
+            std::string body_overlay_conflict;
+            auto body_overlay = collectConfigOverlay(body_node_id, workflow, merged_results,
+                                                     body_overlay_conflict);
+            std::vector<std::string> body_overlay_ignored;
+            if (body_overlay_conflict.empty()) {
+                body_overlay_ignored = applyConfigOverlay(
+                    evaluated_body_node.config, body_overlay,
+                    body_node_def_for_defaults ? body_node_def_for_defaults->config_schema
+                                               : nlohmann::json::object());
+            }
+
             NodeExecutionResult body_result;
-            if (!t_disabled_reference_error.empty()) {
+            if (!body_overlay_conflict.empty()) {
+                body_result.node_id = body_node_id;
+                body_result.status = NodeStatus::Failed;
+                body_result.input = node_input;
+                body_result.output = nlohmann::json::object();
+                body_result.error = body_overlay_conflict;
+                body_result.started_at = TimeUtils::nowMs();
+                body_result.finished_at = body_result.started_at;
+            } else if (!t_disabled_reference_error.empty()) {
                 body_result.node_id = body_node_id;
                 body_result.status = NodeStatus::Failed;
                 body_result.input = node_input;
@@ -2191,6 +2364,13 @@ bool WorkflowEngine::executeLoopBody(
                                            body_node_def_for_defaults);
             }
 
+            if (!body_overlay.empty() && body_result.status == NodeStatus::Completed) {
+                body_result.output["_appliedConfig"] = body_overlay;
+                if (!body_overlay_ignored.empty()) {
+                    body_result.output["_ignoredConfigKeys"] = body_overlay_ignored;
+                }
+            }
+
             rejectPauseInLoopBody(body_result);
 
             // Only a node that actually ran counts. A node on a branch that was

+ 8 - 0
src/runner/workflow_engine.hpp

@@ -236,6 +236,14 @@ private:
                                    const Workflow& workflow,
                                    const std::optional<NodeDefinition>& prefetched_node_def = std::nullopt);
 
+    // Gather the settings any connected Configurator supplies for this node.
+    // conflict_error is set when two live Configurators claim the same key.
+    nlohmann::json collectConfigOverlay(
+        const std::string& node_id,
+        const Workflow& workflow,
+        const std::unordered_map<std::string, NodeExecutionResult>& results,
+        std::string& conflict_error);
+
     nlohmann::json collectNodeInput(const std::string& node_id,
                                     const Workflow& workflow,
                                     const std::unordered_map<std::string, NodeExecutionResult>& results);

+ 22 - 0
tests/nodes/configurator-basic.json

@@ -0,0 +1,22 @@
+{
+  "name": "verify-configurator-basic",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "cfg", "name": "Preset", "type": "configurator", "position": {"x": -150, "y": 100},
+     "config": {"label": "Preset", "settings": [
+       {"name": "message", "value": "from the configurator"},
+       {"name": "mode", "value": "stop"}
+     ]}},
+    {"id": "target", "name": "Target", "type": "stop-and-error", "position": {"x": 0, "y": 200},
+     "config": {"message": "the node's own value", "mode": "error"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "cfg", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "data"},
+    {"sourceNodeId": "cfg", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "target": {"status": "completed", "output": {"stopped": true, "reason": "from the configurator"}}
+  }
+}

+ 22 - 0
tests/nodes/configurator-conflict.json

@@ -0,0 +1,22 @@
+{
+  "name": "verify-configurator-conflict",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "cfg1", "name": "One", "type": "configurator", "position": {"x": -150, "y": 100},
+     "config": {"settings": [{"name": "message", "value": "first"}]}},
+    {"id": "cfg2", "name": "Two", "type": "configurator", "position": {"x": 150, "y": 100},
+     "config": {"settings": [{"name": "message", "value": "second"}]}},
+    {"id": "target", "name": "Target", "type": "stop-and-error", "position": {"x": 0, "y": 200},
+     "config": {"message": "own", "mode": "stop"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "cfg1", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "cfg2", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "data"},
+    {"sourceNodeId": "cfg1", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"},
+    {"sourceNodeId": "cfg2", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"}
+  ],
+  "expect": {
+    "target": {"status": "failed", "errorContains": "configured twice"}
+  }
+}

+ 23 - 0
tests/nodes/configurator-unknown-key.json

@@ -0,0 +1,23 @@
+{
+  "name": "verify-configurator-unknown-key",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "cfg", "name": "Shared", "type": "configurator", "position": {"x": -150, "y": 100},
+     "config": {"settings": [
+       {"name": "message", "value": "applied"},
+       {"name": "mode", "value": "stop"},
+       {"name": "notASetting", "value": "ignored"}
+     ]}},
+    {"id": "target", "name": "Target", "type": "stop-and-error", "position": {"x": 0, "y": 200},
+     "config": {"message": "own", "mode": "error"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "cfg", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "data"},
+    {"sourceNodeId": "cfg", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "target": {"status": "completed", "output": {"stopped": true, "reason": "applied", "_ignoredConfigKeys": ["notASetting"]}}
+  }
+}

+ 30 - 0
tests/nodes/configurator-variation.json

@@ -0,0 +1,30 @@
+{
+  "name": "verify-configurator-variation",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "src", "name": "Src", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { pick: 'b' };"}},
+    {"id": "gate", "name": "Gate", "type": "if-condition", "position": {"x": 0, "y": 200},
+     "config": {"combineWith": "and", "conditions": [{"field": "data.result.pick", "operator": "equals", "value": "a"}]}},
+    {"id": "cfgA", "name": "VariantA", "type": "configurator", "position": {"x": -200, "y": 300},
+     "config": {"label": "A", "settings": [{"name": "message", "value": "variant A"}, {"name": "mode", "value": "stop"}]}},
+    {"id": "cfgB", "name": "VariantB", "type": "configurator", "position": {"x": 200, "y": 300},
+     "config": {"label": "B", "settings": [{"name": "message", "value": "variant B"}, {"name": "mode", "value": "stop"}]}},
+    {"id": "target", "name": "Target", "type": "stop-and-error", "position": {"x": 0, "y": 400},
+     "config": {"message": "unconfigured", "mode": "error"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "src", "targetInput": "data"},
+    {"sourceNodeId": "src", "sourceOutput": "main", "targetNodeId": "gate", "targetInput": "data"},
+    {"sourceNodeId": "gate", "sourceOutput": "true", "targetNodeId": "cfgA", "targetInput": "data"},
+    {"sourceNodeId": "gate", "sourceOutput": "false", "targetNodeId": "cfgB", "targetInput": "data"},
+    {"sourceNodeId": "src", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "data"},
+    {"sourceNodeId": "cfgA", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"},
+    {"sourceNodeId": "cfgB", "sourceOutput": "main", "targetNodeId": "target", "targetInput": "config"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "target": {"status": "completed", "output": {"stopped": true, "reason": "variant B"}}
+  },
+  "expectMissing": ["cfgA"]
+}