smartbotic/.superpowers/review-config-defaults.diff

3bd17d0 fix: apply configSchema defaults at write time and read time

 CMakeLists.txt                                  |  1 +
 lib/common/config_defaults.cpp                  | 39 +++++++++++++++++++++++++
 lib/common/config_defaults.hpp                  | 27 +++++++++++++++++
 src/runner/workflow_engine.cpp                  |  9 +++++-
 src/webserver/api/workflow_controller.cpp       | 39 +++++++++++++++++++++++--
 src/webserver/api/workflow_controller.hpp       |  4 +++
 src/webserver/webserver_service.cpp             |  7 +++--
 tests/nodes/config-defaults-fill.json           | 14 +++++++++
 tests/nodes/config-defaults-preserve-falsy.json | 14 +++++++++
 9 files changed, 148 insertions(+), 6 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4499987..6aef080 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,20 +12,21 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 include(CompilerFlags)
 include(Dependencies)
 include(FindPackages)
 
 # Common library
 add_library(smartbotic_common STATIC
     lib/common/uuid.cpp
     lib/common/time_utils.cpp
     lib/common/error.cpp
     lib/common/string_utils.cpp
+    lib/common/config_defaults.cpp
 )
 target_include_directories(smartbotic_common PUBLIC
     ${CMAKE_CURRENT_SOURCE_DIR}/lib
 )
 target_link_libraries(smartbotic_common PUBLIC
     nlohmann_json::nlohmann_json
     OpenSSL::Crypto
 )
 
 # Logging library
diff --git a/lib/common/config_defaults.cpp b/lib/common/config_defaults.cpp
new file mode 100644
index 0000000..a5bbfd8
--- /dev/null
+++ b/lib/common/config_defaults.cpp
@@ -0,0 +1,39 @@
+#include "common/config_defaults.hpp"
+
+namespace smartbotic::common {
+
+nlohmann::json applyConfigDefaults(const nlohmann::json& config,
+                                    const nlohmann::json& config_schema) {
+    nlohmann::json result = config.is_object() ? config : nlohmann::json::object();
+
+    if (!config_schema.is_object()) {
+        return result;
+    }
+
+    auto properties_it = config_schema.find("properties");
+    if (properties_it == config_schema.end() || !properties_it->is_object()) {
+        return result;
+    }
+
+    for (auto it = properties_it->begin(); it != properties_it->end(); ++it) {
+        const std::string& key = it.key();
+        const nlohmann::json& property_schema = it.value();
+
+        if (!property_schema.is_object()) {
+            continue;
+        }
+
+        auto default_it = property_schema.find("default");
+        if (default_it == property_schema.end()) {
+            continue;
+        }
+
+        if (!result.contains(key)) {
+            result[key] = *default_it;
+        }
+    }
+
+    return result;
+}
+
+} // namespace smartbotic::common
diff --git a/lib/common/config_defaults.hpp b/lib/common/config_defaults.hpp
new file mode 100644
index 0000000..ad504da
--- /dev/null
+++ b/lib/common/config_defaults.hpp
@@ -0,0 +1,27 @@
+#pragma once
+
+#include <nlohmann/json.hpp>
+
+namespace smartbotic::common {
+
+// Applies configSchema-declared defaults to a node config.
+//
+// Returns a copy of `config` with any property from
+// `config_schema["properties"]` that declares a "default" filled in, but
+// only when `config` does not already contain that key. A key that is
+// present - even with a falsy value like false, 0, null, or "" - is a
+// deliberate, stored value and is never overwritten.
+//
+// Scope limit (by design, not an oversight): only top-level properties are
+// considered. Defaults nested inside object properties or array item
+// schemas are NOT applied. Nodes are not currently written with nested
+// config shapes that rely on defaults, so recursing was left out to keep
+// this function's behaviour easy to reason about; revisit if that changes.
+//
+// Tolerant of a missing/null/non-object schema, or one with no
+// "properties" - in all of those cases the config is returned unchanged
+// rather than throwing.
+nlohmann::json applyConfigDefaults(const nlohmann::json& config,
+                                    const nlohmann::json& config_schema);
+
+} // namespace smartbotic::common
diff --git a/src/runner/workflow_engine.cpp b/src/runner/workflow_engine.cpp
index e475d9c..bf2a85d 100644
--- a/src/runner/workflow_engine.cpp
+++ b/src/runner/workflow_engine.cpp
@@ -1,13 +1,14 @@
 #include "workflow_engine.hpp"
 #include "common/uuid.hpp"
 #include "common/time_utils.hpp"
+#include "common/config_defaults.hpp"
 #include "logging/logger.hpp"
 #include <algorithm>
 #include <functional>
 #include <stack>
 #include <unordered_set>
 
 namespace smartbotic::runner {
 
 using namespace common;
 
@@ -641,21 +642,27 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                         {"nodeId", node_id},
                         {"status", "completed"},
                         {"output", node_result.output},
                         {"fromCache", true}
                     });
                 }
             } else {
                 // Evaluate expressions in node config
                 WorkflowNode evaluated_node = *node;
                 t_disabled_reference_error.clear();
-                evaluated_node.config = evaluateExpressions(node->config, input, result.node_results, workflow);
+                nlohmann::json defaulted_config = node->config;
+                auto node_def_for_defaults = registry_.getNode(node->type);
+                if (node_def_for_defaults) {
+                    defaulted_config = smartbotic::common::applyConfigDefaults(
+                        node->config, node_def_for_defaults->config_schema);
+                }
+                evaluated_node.config = evaluateExpressions(defaulted_config, input, result.node_results, workflow);
 
                 if (!t_disabled_reference_error.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 = t_disabled_reference_error;
                     node_result.started_at = TimeUtils::nowMs();
                     node_result.finished_at = node_result.started_at;
                 } else {
diff --git a/src/webserver/api/workflow_controller.cpp b/src/webserver/api/workflow_controller.cpp
index 4be6024..aa0021c 100644
--- a/src/webserver/api/workflow_controller.cpp
+++ b/src/webserver/api/workflow_controller.cpp
@@ -1,13 +1,14 @@
 #include "workflow_controller.hpp"
 #include "common/uuid.hpp"
 #include "common/time_utils.hpp"
+#include "common/config_defaults.hpp"
 #include "logging/logger.hpp"
 #include <grpcpp/grpcpp.h>
 
 namespace smartbotic::webserver::api {
 
 using namespace common;
 
 WorkflowController::WorkflowController(storage::StorageClient& storage,
                                        auth::AuthMiddleware& middleware,
                                        runners::RunnerRegistry& registry,
@@ -16,20 +17,45 @@ WorkflowController::WorkflowController(storage::StorageClient& storage,
                                        WorkflowScheduler& scheduler,
                                        nodes::NodeStore& node_store)
     : storage_(storage)
     , middleware_(middleware)
     , registry_(registry)
     , load_balancer_(load_balancer)
     , ws_server_(ws_server)
     , scheduler_(scheduler)
     , node_store_(node_store) {}
 
+void WorkflowController::materializeNodeConfigDefaults(nlohmann::json& body) {
+    if (!body.contains("nodes") || !body["nodes"].is_array()) {
+        return;
+    }
+
+    for (auto& node : body["nodes"]) {
+        if (!node.is_object()) {
+            continue;
+        }
+
+        std::string node_type = node.value("type", "");
+        if (node_type.empty()) {
+            continue;
+        }
+
+        auto node_result = node_store_.get(node_type);
+        if (node_result.failed()) {
+            continue;
+        }
+
+        auto config = node.value("config", nlohmann::json::object());
+        node["config"] = common::applyConfigDefaults(config, node_result.value().config_schema);
+    }
+}
+
 void WorkflowController::registerRoutes(httplib::Server& server) {
     server.Get("/api/v1/workflows", [this](const httplib::Request& req, httplib::Response& res) {
         middleware_.requireAuth(req, res, [this](auto& req, auto& res, auto& ctx) {
             listWorkflows(req, res, ctx);
         });
     });
 
     server.Get(R"(/api/v1/workflows/([^/]+))", [this](const httplib::Request& req, httplib::Response& res) {
         middleware_.requireAuth(req, res, [this](auto& req, auto& res, auto& ctx) {
             getWorkflow(req, res, ctx);
@@ -165,20 +191,22 @@ void WorkflowController::createWorkflow(const httplib::Request& req, httplib::Re
         // Set owner - metadata fields (_id, _createdAt, _updatedAt, _version) are handled by database
         body["ownerId"] = ctx.user_id;
         body["active"] = body.value("active", false);
 
         // Validate required fields
         if (!body.contains("name") || body["name"].get<std::string>().empty()) {
             sendError(res, "Name is required", 400);
             return;
         }
 
+        materializeNodeConfigDefaults(body);
+
         auto result = storage_.insert("workflows", body, id);
         if (result.failed()) {
             sendError(res, result.error().message(), 500);
             return;
         }
 
         // Get the created workflow with metadata
         auto workflow = storage_.get("workflows", id);
         if (workflow.failed()) {
             sendError(res, "Failed to retrieve created workflow", 500);
@@ -215,20 +243,22 @@ void WorkflowController::updateWorkflow(const httplib::Request& req, httplib::Re
         // Note: updatedAt is managed by database automatically
 
         // Remember the current run state so the reconciliation below can tell
         // whether this update actually flipped it.
         bool was_active = false;
         auto before = storage_.get("workflows", id);
         if (before.ok()) {
             was_active = before.value().value("active", false);
         }
 
+        materializeNodeConfigDefaults(body);
+
         auto result = storage_.update("workflows", id, body, 0, true);
         if (result.failed()) {
             sendError(res, result.error().message(), 404);
             return;
         }
 
         // Get updated workflow
         auto workflow = storage_.get("workflows", id);
         if (workflow.ok()) {
             // Reconcile the scheduler in both directions. Registering on active
@@ -402,22 +432,24 @@ void WorkflowController::updateScheduledTriggers(const std::string& workflow_id,
         }
 
         auto& node_def = node_result.value();
 
         // Check if node has scheduling capability (is_scheduled flag or pollInterval config)
         bool is_scheduled = node_def.is_scheduled;
         if (!is_scheduled) {
             continue;
         }
 
-        // Get interval from node config
-        auto config = node.value("config", nlohmann::json::object());
+        // Get interval from node config, filling in any schema defaults the
+        // stored config is missing.
+        auto config = common::applyConfigDefaults(
+            node.value("config", nlohmann::json::object()), node_def.config_schema);
         int interval = config.value("pollInterval", 0);
 
         if (interval > 0) {
             scheduler_.registerWorkflow(
                 workflow_id,
                 workflow_name,
                 node_id,
                 node_type,
                 interval
             );
@@ -430,21 +462,22 @@ void WorkflowController::updateScheduledTriggers(const std::string& workflow_id,
 int WorkflowController::getScheduledInterval(const nlohmann::json& workflow) {
     auto nodes = workflow.value("nodes", nlohmann::json::array());
     for (const auto& node : nodes) {
         std::string node_type = node.value("type", "");
 
         auto node_result = node_store_.get(node_type);
         if (node_result.failed() || !node_result.value().is_trigger || !node_result.value().is_scheduled) {
             continue;
         }
 
-        auto config = node.value("config", nlohmann::json::object());
+        auto config = common::applyConfigDefaults(
+            node.value("config", nlohmann::json::object()), node_result.value().config_schema);
         return config.value("pollInterval", 0);
     }
     return 0;
 }
 
 void WorkflowController::sendJson(httplib::Response& res, const nlohmann::json& data, int status) {
     res.status = status;
     res.set_content(data.dump(), "application/json");
 }
 
diff --git a/src/webserver/api/workflow_controller.hpp b/src/webserver/api/workflow_controller.hpp
index 2447ceb..a748ae1 100644
--- a/src/webserver/api/workflow_controller.hpp
+++ b/src/webserver/api/workflow_controller.hpp
@@ -53,13 +53,17 @@ private:
     auth::AuthMiddleware& middleware_;
     runners::RunnerRegistry& registry_;
     runners::LoadBalancer& load_balancer_;
     WebSocketServer& ws_server_;
     WorkflowScheduler& scheduler_;
     nodes::NodeStore& node_store_;
 
     // Helper to check for scheduled triggers and register/unregister with scheduler
     void updateScheduledTriggers(const std::string& workflow_id, bool activate);
     int getScheduledInterval(const nlohmann::json& workflow);
+
+    // Fills in each node's stored config with any missing configSchema
+    // defaults, in place, so a saved workflow document is self-describing.
+    void materializeNodeConfigDefaults(nlohmann::json& body);
 };
 
 } // namespace smartbotic::webserver::api
diff --git a/src/webserver/webserver_service.cpp b/src/webserver/webserver_service.cpp
index a7d7f13..124419a 100644
--- a/src/webserver/webserver_service.cpp
+++ b/src/webserver/webserver_service.cpp
@@ -10,20 +10,21 @@
 #include "api/database_controller.hpp"
 #include "api/file_controller.hpp"
 #include "api/proxy_controller.hpp"
 #include "api/credential_controller.hpp"
 #include "nodes/node_store.hpp"
 #include "grpc/node_sync_service.hpp"
 #include "grpc/credential_service.hpp"
 #include "credentials/credential_store.hpp"
 #include "scheduler/workflow_scheduler.hpp"
 #include "common/time_utils.hpp"
+#include "common/config_defaults.hpp"
 #include "logging/logger.hpp"
 #include <grpcpp/grpcpp.h>
 #include "proto/runner.grpc.pb.h"
 
 namespace smartbotic::webserver {
 
 WebServerService::WebServerService(const WebServerServiceConfig& config)
     : config_(config) {
 
     // Initialize storage client
@@ -306,22 +307,24 @@ void WebServerService::loadScheduledWorkflows() {
             auto node_result = node_store_->get(node_type);
             if (node_result.failed()) {
                 continue;
             }
 
             const auto& node_def = node_result.value();
             if (!node_def.is_trigger || !node_def.is_scheduled) {
                 continue;
             }
 
-            // Get interval from node config
-            auto config = node.value("config", nlohmann::json::object());
+            // Get interval from node config, filling in any schema defaults
+            // the stored config is missing (e.g. an untouched form field).
+            auto config = smartbotic::common::applyConfigDefaults(
+                node.value("config", nlohmann::json::object()), node_def.config_schema);
             int interval = config.value("pollInterval", 0);
 
             if (interval > 0) {
                 auto policy = overlapPolicyFromString(
                     config.value("overlapPolicy", std::string("skip")));
 
                 scheduler_->registerWorkflow(
                     workflow_id,
                     workflow_name,
                     node_id,
diff --git a/tests/nodes/config-defaults-fill.json b/tests/nodes/config-defaults-fill.json
new file mode 100644
index 0000000..ec715f0
--- /dev/null
+++ b/tests/nodes/config-defaults-fill.json
@@ -0,0 +1,14 @@
+{
+  "name": "verify-config-defaults-fill",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "n2", "name": "NoConfig", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "n2", "targetInput": "data"}
+  ],
+  "expect": {
+    "n2": {"status": "completed", "output": {"result": {"processed": true}}}
+  }
+}
diff --git a/tests/nodes/config-defaults-preserve-falsy.json b/tests/nodes/config-defaults-preserve-falsy.json
new file mode 100644
index 0000000..6b84e8c
--- /dev/null
+++ b/tests/nodes/config-defaults-preserve-falsy.json
@@ -0,0 +1,14 @@
+{
+  "name": "verify-config-defaults-preserve-falsy",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "n2", "name": "EmptyCode", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": ""}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "n2", "targetInput": "data"}
+  ],
+  "expect": {
+    "n2": {"status": "failed", "errorContains": "No code provided"}
+  }
+}