fix: refuse a node whose schema fails to parse instead of storing it degraded

A node's configSchema/inputSchema/outputSchema is authored as a JS object
literal. When jsLiteralToJson or the JSON parse afterward failed, parseFromCode
logged a warning and quietly left the field at its default (null), and
migration stored that degraded node anyway - indistinguishable from a node
that genuinely declares no settings, and capable of silently overwriting a
previously stored good version with a bad one.

Migration now refuses a node whose schema was present in the source but did
not parse: it is skipped (a good stored version is left untouched) and named,
with the parse failure reason, in both the log (as before) and the migrate
endpoint's JSON response, under rejected/rejectedCount. A field that is simply
absent from the source - inputSchema on most trigger nodes - is still not an
error.

src/webserver/api/node_controller.cpp

@@ -435,16 +435,29 @@ void NodeController::migrateNodes(const httplib::Request& req, httplib::Response
         return;
     }
 
+    const auto& migration = result.value();
+
     // Notify runners of all newly migrated nodes
     if (sync_service_) {
-        for (const auto& node : result.value()) {
+        for (const auto& node : migration.migrated) {
             sync_service_->notifyNodeCreated(node);
         }
     }
 
+    nlohmann::json rejected_json = nlohmann::json::array();
+    for (const auto& rejection : migration.rejected) {
+        rejected_json.push_back(rejection.toJson());
+    }
+
+    // A rejected node is still reported as an overall success - the caller
+    // asked to migrate a directory, and every node that could be migrated
+    // was. rejectedCount/rejected is how a bad edit surfaces here instead of
+    // only in the log.
     sendJson(res, {
         {"success", true},
-        {"migratedCount", result.value().size()}
+        {"migratedCount", migration.migrated.size()},
+        {"rejectedCount", migration.rejected.size()},
+        {"rejected", rejected_json}
     });
 }
 

src/webserver/nodes/node_store.cpp

@@ -363,30 +363,66 @@ StoredNode StoredNode::parseFromCode(const std::string& code, const std::string&
         }
     }
 
-    // Parse configSchema
-    std::regex config_start_regex(R"(const\s+configSchema\s*=\s*)");
-    if (std::regex_search(code, match, config_start_regex)) {
-        size_t start_pos = match.position() + match.length();
+    // Parse configSchema / inputSchema / outputSchema. All three are declared
+    // and read the same way: a JS object literal that jsLiteralToJson turns
+    // into JSON text. A field that is simply absent from the source (most
+    // trigger nodes never declare inputSchema) is legitimate and not an
+    // error. A field that IS present but does not parse - an unbalanced
+    // literal, or text jsLiteralToJson produced that nlohmann::json rejects -
+    // is recorded in node.schema_parse_errors so migration can refuse to
+    // store a degraded node instead of silently writing null.
+    auto parse_schema_field = [&](const char* field_name, nlohmann::json& out) -> std::string {
+        std::regex start_regex(std::string(R"(const\s+)") + field_name + R"(\s*=\s*)");
+        std::smatch field_match;
+        if (!std::regex_search(code, field_match, start_regex)) {
+            return "";   // not declared at all - not an error
+        }
+        size_t start_pos = field_match.position() + field_match.length();
         size_t brace_pos = code.find('{', start_pos);
-        if (brace_pos != std::string::npos) {
-            std::string jsonStr = jsLiteralToJson(code, brace_pos);
-            if (!jsonStr.empty()) {
-                try {
-                    node.config_schema = nlohmann::json::parse(jsonStr);
-
-                    // Record the order the settings were written in.
-                    //
-                    // nlohmann::json sorts an object's keys, so by the time a
-                    // schema reaches the editor "width" has drifted to the end
-                    // and "height" near the front, whatever the node author
-                    // wrote. ordered_json keeps insertion order, so parsing a
-                    // second time with it recovers the real order.
-                    //
-                    // It is stored as an ARRAY rather than by keeping the whole
-                    // schema ordered, because the schema is written to the
-                    // database and read back, and nothing here controls whether
-                    // that round trip preserves an object's key order. An array
-                    // has an order by definition and survives any of it.
+        if (brace_pos == std::string::npos) {
+            const std::string msg = "no object literal found after the declaration";
+            LOG_WARN("Failed to parse {} for {}: {}", field_name, node.id, msg);
+            return msg;
+        }
+        std::string jsonStr = jsLiteralToJson(code, brace_pos);
+        if (jsonStr.empty()) {
+            const std::string msg = "the object literal is not balanced";
+            LOG_WARN("Failed to parse {} for {}: {}", field_name, node.id, msg);
+            return msg;
+        }
+        try {
+            out = nlohmann::json::parse(jsonStr);
+        } catch (const nlohmann::json::exception& e) {
+            LOG_WARN("Failed to parse {} for {}: {}", field_name, node.id, e.what());
+            return e.what();
+        }
+        return "";
+    };
+
+    {
+        std::string err = parse_schema_field("configSchema", node.config_schema);
+        if (!err.empty()) {
+            node.schema_parse_errors.push_back("configSchema - " + err);
+        } else if (!node.config_schema.is_null()) {
+            // Record the order the settings were written in.
+            //
+            // nlohmann::json sorts an object's keys, so by the time a
+            // schema reaches the editor "width" has drifted to the end
+            // and "height" near the front, whatever the node author
+            // wrote. ordered_json keeps insertion order, so parsing a
+            // second time with it recovers the real order.
+            //
+            // It is stored as an ARRAY rather than by keeping the whole
+            // schema ordered, because the schema is written to the
+            // database and read back, and nothing here controls whether
+            // that round trip preserves an object's key order. An array
+            // has an order by definition and survives any of it.
+            std::regex config_start_regex(R"(const\s+configSchema\s*=\s*)");
+            if (std::regex_search(code, match, config_start_regex)) {
+                size_t start_pos = match.position() + match.length();
+                size_t brace_pos = code.find('{', start_pos);
+                if (brace_pos != std::string::npos) {
+                    std::string jsonStr = jsLiteralToJson(code, brace_pos);
                     auto ordered = nlohmann::ordered_json::parse(jsonStr);
                     if (ordered.contains("properties") && ordered["properties"].is_object()) {
                         auto ui_order = nlohmann::json::array();
@@ -396,44 +432,22 @@ StoredNode StoredNode::parseFromCode(const std::string& code, const std::string&
                         }
                         node.config_schema["uiOrder"] = ui_order;
                     }
-                } catch (const nlohmann::json::exception& e) {
-                    LOG_WARN("Failed to parse configSchema for {}: {}", node.id, e.what());
                 }
             }
         }
     }
 
-    // Parse inputSchema
-    std::regex input_start_regex(R"(const\s+inputSchema\s*=\s*)");
-    if (std::regex_search(code, match, input_start_regex)) {
-        size_t start_pos = match.position() + match.length();
-        size_t brace_pos = code.find('{', start_pos);
-        if (brace_pos != std::string::npos) {
-            std::string jsonStr = jsLiteralToJson(code, brace_pos);
-            if (!jsonStr.empty()) {
-                try {
-                    node.input_schema = nlohmann::json::parse(jsonStr);
-                } catch (const nlohmann::json::exception& e) {
-                    LOG_WARN("Failed to parse inputSchema for {}: {}", node.id, e.what());
-                }
-            }
+    {
+        std::string err = parse_schema_field("inputSchema", node.input_schema);
+        if (!err.empty()) {
+            node.schema_parse_errors.push_back("inputSchema - " + err);
         }
     }
 
-    // Parse outputSchema
-    std::regex output_start_regex(R"(const\s+outputSchema\s*=\s*)");
-    if (std::regex_search(code, match, output_start_regex)) {
-        size_t start_pos = match.position() + match.length();
-        size_t brace_pos = code.find('{', start_pos);
-        if (brace_pos != std::string::npos) {
-            std::string jsonStr = jsLiteralToJson(code, brace_pos);
-            if (!jsonStr.empty()) {
-                try {
-                    node.output_schema = nlohmann::json::parse(jsonStr);
-                } catch (const nlohmann::json::exception& e) {
-                    LOG_WARN("Failed to parse outputSchema for {}: {}", node.id, e.what());
-                }
-            }
+    {
+        std::string err = parse_schema_field("outputSchema", node.output_schema);
+        if (!err.empty()) {
+            node.schema_parse_errors.push_back("outputSchema - " + err);
         }
     }
 
@@ -528,6 +542,13 @@ StoredNode StoredNode::parseFromCode(const std::string& code, const std::string&
     return node;
 }
 
+nlohmann::json NodeMigrationRejection::toJson() const {
+    nlohmann::json j;
+    j["nodeId"] = node_id;
+    j["reasons"] = reasons;
+    return j;
+}
+
 // NodeStore implementation
 NodeStore::NodeStore(storage::StorageClient& storage)
     : storage_(storage) {}
@@ -622,12 +643,13 @@ Result<void> NodeStore::remove(const std::string& id) {
     return {};
 }
 
-Result<std::vector<StoredNode>> NodeStore::migrateFromFiles(const std::filesystem::path& nodes_dir) {
+Result<NodeMigrationResult> NodeStore::migrateFromFiles(const std::filesystem::path& nodes_dir) {
     if (!std::filesystem::exists(nodes_dir)) {
         return Error(ErrorCode::NotFound, "Nodes directory not found: " + nodes_dir.string());
     }
 
-    std::vector<StoredNode> migrated_nodes;
+    NodeMigrationResult migration_result;
+    auto& migrated_nodes = migration_result.migrated;
     int errors = 0;
 
     for (const auto& entry : std::filesystem::recursive_directory_iterator(nodes_dir)) {
@@ -661,6 +683,23 @@ Result<std::vector<StoredNode>> NodeStore::migrateFromFiles(const std::filesyste
             node.id = entry.path().stem().string();
         }
 
+        // A node whose configSchema/inputSchema/outputSchema was present in
+        // the source but failed to parse must never be written as a
+        // degraded (null-schema) version, and must never overwrite a
+        // previously stored good version. Refuse it, name it and the reason
+        // in the result the caller gets back, and move on - the log warning
+        // from parseFromCode already recorded the same thing.
+        if (!node.schema_parse_errors.empty()) {
+            NodeMigrationRejection rejection;
+            rejection.node_id = node.id;
+            rejection.reasons = node.schema_parse_errors;
+            migration_result.rejected.push_back(rejection);
+            LOG_WARN("Rejected node from migration: {} ({} schema error(s))",
+                     node.id, node.schema_parse_errors.size());
+            errors++;
+            continue;
+        }
+
         // Check if already exists
         auto existing = get(node.id);
         if (existing.ok()) {
@@ -705,8 +744,9 @@ Result<std::vector<StoredNode>> NodeStore::migrateFromFiles(const std::filesyste
         migrated_nodes.push_back(node);
     }
 
-    LOG_INFO("Migration complete: {} nodes migrated, {} errors", migrated_nodes.size(), errors);
-    return migrated_nodes;
+    LOG_INFO("Migration complete: {} nodes migrated, {} errors, {} rejected",
+             migrated_nodes.size(), errors, migration_result.rejected.size());
+    return migration_result;
 }
 
 } // namespace smartbotic::webserver::nodes

src/webserver/nodes/node_store.hpp

@@ -45,11 +45,32 @@ struct StoredNode {
     int64_t created_at = 0;
     int64_t updated_at = 0;
 
+    // Populated by parseFromCode when configSchema, inputSchema or outputSchema
+    // is present in the source but fails to parse. A field that is simply
+    // absent (common for trigger nodes) is not an error and leaves this empty.
+    // Transient - not persisted, and not part of toJson/fromJson.
+    std::vector<std::string> schema_parse_errors;
+
     nlohmann::json toJson() const;
     static StoredNode fromJson(const nlohmann::json& j);
     static StoredNode parseFromCode(const std::string& code, const std::string& default_category = "custom");
 };
 
+// A node whose source declared configSchema/inputSchema/outputSchema but the
+// literal failed to parse. Migration refuses to store a degraded version of
+// such a node - see NodeStore::migrateFromFiles.
+struct NodeMigrationRejection {
+    std::string node_id;
+    std::vector<std::string> reasons;
+
+    nlohmann::json toJson() const;
+};
+
+struct NodeMigrationResult {
+    std::vector<StoredNode> migrated;
+    std::vector<NodeMigrationRejection> rejected;
+};
+
 // Node store - manages node definitions in database
 class NodeStore {
 public:
@@ -63,7 +84,7 @@ public:
     common::Result<void> remove(const std::string& id);
 
     // Utility methods
-    common::Result<std::vector<StoredNode>> migrateFromFiles(const std::filesystem::path& nodes_dir);
+    common::Result<NodeMigrationResult> migrateFromFiles(const std::filesystem::path& nodes_dir);
 
 private:
     storage::StorageClient& storage_;

src/webserver/webserver_service.cpp

@@ -106,7 +106,13 @@ WebServerService::WebServerService(const WebServerServiceConfig& config)
     // Migrate/sync nodes from disk - updates existing nodes if code has changed
     auto migrate_result = node_store_->migrateFromFiles("./nodes");
     if (migrate_result.ok()) {
-        LOG_INFO("Node migration: {} new nodes imported", migrate_result.value().size());
+        LOG_INFO("Node migration: {} nodes imported/updated, {} rejected",
+                 migrate_result.value().migrated.size(), migrate_result.value().rejected.size());
+        for (const auto& rejection : migrate_result.value().rejected) {
+            for (const auto& reason : rejection.reasons) {
+                LOG_WARN("Node migration rejected {}: {}", rejection.node_id, reason);
+            }
+        }
     } else {
         LOG_WARN("Node migration failed: {}", migrate_result.error().message());
     }