feat: switch a workflow off after too many failures in a row

A scheduled workflow that starts failing keeps failing every five minutes
until somebody notices. Its settings now say how many failures in a row it
is allowed, and past that it is switched off.

The count lives on the workflow, not in memory, because a restart must not
forgive a workflow that has been failing for an hour - and the number is
worth being able to see. It is written on every failure whether or not
anything is switched off, since that is what someone reads when asking how
long this has been going wrong.

A success resets it. A streak that is never reset is not a streak, it is a
total, and a workflow that fails once a week would eventually be switched off
for it. That meant widening the failure callback to cover both outcomes.
Cancelled counts as neither: somebody pressed Stop, which says nothing about
whether the workflow works.

Being switched off is said out loud rather than left as a silent "Inactive",
which is indistinguishable from someone turning it off on purpose - the
difference that matters when something has stopped running. The workflow
records why and when, the list shows "Switched off" with the reason and the
last error, and the scheduler is told, or it would keep firing something that
reads as inactive. Turning it back on by hand clears the count and the note,
because leaving the count would switch it off again on the very next failure.

Off by default, at 0.

Measured: with a limit of 2, one failure leaves it running at 1; the second
switches it off with the reason recorded; a success anywhere in between puts
the count back to 0; and Activate clears both.

Also: two SD.cpp cases now declare that they need a model loaded, and
scripts/run-node-tests.sh reports an unmet precondition as a skip by name.
That server unloads when idle, and those two have twice produced a red suite
for a reason that had nothing to do with the code. A skip that is reported is
honest; a pass that did not run is not. 60 passed, 2 skipped.

docs/nodes.md

@@ -782,3 +782,27 @@ waiting is how a rate limit becomes a ban.
 It is off by default on purpose. A POST that timed out may already have been
 received at the far end, and repeating it would do the thing twice - so the
 caller opts in where repeating is safe.
+
+## Running the verification cases
+
+```bash
+./scripts/run-node-tests.sh          # all of them
+python3 scripts/verify-node.py tests/nodes/<case>.json   # one
+```
+
+Exit 0 means every assertion held, 1 means one did not, and 2 means the case
+declares a precondition this machine does not meet. The runner counts that last
+one as skipped and names it. A case that cannot run is never counted as passed:
+a green tally that includes tests which did not run is worse than a red one.
+
+A case declares what it needs with `requires`:
+
+```json
+"requires": { "http": "http://mulan:8077/health",
+              "expect": { "model_loaded": true } }
+```
+
+Two SD.cpp cases use this. They compare against whatever model the server has
+loaded, and that server unloads when idle - so without the gate they fail for a
+reason that has nothing to do with the code, and a failure everyone learns to
+ignore is worse than no test.

scripts/run-node-tests.sh

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+# Run every verification case. A case that declares a precondition this machine
+# does not meet is counted as skipped and named - it is never counted as passed,
+# because a green tally that includes tests which did not run is worse than a
+# red one.
+set -uo pipefail
+cd "$(dirname "$0")/.."
+
+pass=0; fail=0; skip=0
+failed=(); skipped=()
+
+for case in tests/nodes/*.json; do
+    output=$(python3 scripts/verify-node.py "$case" 2>&1)
+    case_status=$?
+    name=$(basename "$case" .json)
+    if [ $case_status -eq 0 ]; then
+        pass=$((pass + 1))
+    elif [ $case_status -eq 2 ]; then
+        skip=$((skip + 1)); skipped+=("$name: ${output#SKIP *: }")
+    else
+        fail=$((fail + 1)); failed+=("$name")
+        printf '\n--- %s\n%s\n' "$name" "$output"
+    fi
+done
+
+echo
+echo "passed $pass, failed $fail, skipped $skip"
+[ ${#skipped[@]} -gt 0 ] && printf 'skipped: %s\n' "${skipped[@]}"
+[ ${#failed[@]} -gt 0 ] && printf 'failed:  %s\n' "${failed[@]}"
+exit $(( fail > 0 ? 1 : 0 ))

scripts/verify-node.py

@@ -2,7 +2,8 @@
 """Run one node verification case against the live SmartBotic services.
 
 Usage: scripts/verify-node.py tests/nodes/<case>.json
-Exit 0 if every assertion holds, 1 otherwise.
+Exit 0 if every assertion holds, 1 if one does not, 2 if the case declares a
+precondition this machine does not meet - reported as a skip, never as a pass.
 """
 import json
 import sys
@@ -121,10 +122,47 @@ def run_resume_case(case, token, execution_id):
     print("resumed %s with token %s" % (execution_id, resume_token))
 
 
+def precondition_unmet(case):
+    """Why this case cannot run here, or None if it can.
+
+    A couple of cases need a model loaded on the SD.cpp server, which unloads
+    itself when idle. Without this they fail for a reason that has nothing to do
+    with the code, and a failure everyone learns to ignore is worse than no
+    test. Skipping is only honest because it is reported as a skip - never as a
+    pass.
+    """
+    needs = case.get("requires")
+    if not needs:
+        return None
+
+    probe = needs.get("http")
+    if probe:
+        try:
+            with urllib.request.urlopen(probe, timeout=10) as response:
+                body = json.loads(response.read().decode())
+        except Exception as exc:
+            return f"{probe} could not be read ({exc})"
+
+        for path, expected in (needs.get("expect") or {}).items():
+            actual = body
+            for part in path.split("."):
+                actual = (actual or {}).get(part) if isinstance(actual, dict) else None
+            if actual != expected:
+                return f"{probe} reports {path}={actual!r}, this case needs {expected!r}"
+
+    return None
+
+
 def main():
     if len(sys.argv) != 2:
         raise SystemExit("usage: verify-node.py <case.json>")
     case = json.load(open(sys.argv[1]))
+
+    unmet = precondition_unmet(case)
+    if unmet:
+        print(f"SKIP {case['name']}: {unmet}")
+        return 2
+
     token = login()
 
     call("POST", "/nodes/migrate", token, {"nodesPath": "./nodes"})

src/webserver/api/execution_controller.cpp

@@ -11,9 +11,9 @@ ExecutionController::ExecutionController(storage::StorageClient& storage,
                                          WebSocketServer& ws_server,
                                          WorkflowScheduler& scheduler,
                                          runners::LoadBalancer& load_balancer,
-                                         FailureHandler on_failure)
+                                         FinishedHandler on_finished)
     : storage_(storage), middleware_(middleware), ws_server_(ws_server),
-      scheduler_(scheduler), load_balancer_(load_balancer), on_failure_(std::move(on_failure)) {}
+      scheduler_(scheduler), load_balancer_(load_balancer), on_finished_(std::move(on_finished)) {}
 
 void ExecutionController::registerRoutes(httplib::Server& server) {
     server.Get("/api/v1/executions", [this](const httplib::Request& req, httplib::Response& res) {
@@ -383,8 +383,13 @@ void ExecutionController::receiveExecutionEvent(const httplib::Request& req, htt
             scheduler_.notifyExecutionFinished(workflow_id, execution_id);
         }
 
-        if (event_type == "execution.failed" && on_failure_ && !workflow_id.empty()) {
-            on_failure_(workflow_id, execution_id, data.value("error", std::string()));
+        // A run that ended one way or the other. Cancelled is deliberately not
+        // counted either way: someone pressed Stop, which says nothing about
+        // whether the workflow works.
+        if (on_finished_ && !workflow_id.empty() &&
+            (event_type == "execution.failed" || event_type == "execution.completed")) {
+            on_finished_(workflow_id, execution_id, event_type == "execution.failed",
+                         data.value("error", std::string()));
         }
 
         // Broadcast to WebSocket clients

src/webserver/api/execution_controller.hpp

@@ -14,17 +14,21 @@ namespace smartbotic::webserver::api {
 
 class ExecutionController {
 public:
-    // Invoked when an execution reports failure, so the workflow's nominated
-    // error workflow can be run. Kept as a callback to avoid the controller
-    // depending on the whole service.
-    using FailureHandler = std::function<void(const std::string& workflow_id,
-                                              const std::string& execution_id,
-                                              const std::string& error)>;
+    // Invoked when an execution finishes, so the workflow's nominated error
+    // workflow can be run and its run of failures counted. Success matters as
+    // much as failure here: a streak that is never reset is not a streak, it is
+    // a total, and a workflow that fails once a week would eventually be
+    // switched off for it. Kept as a callback to avoid the controller depending
+    // on the whole service.
+    using FinishedHandler = std::function<void(const std::string& workflow_id,
+                                               const std::string& execution_id,
+                                               bool failed,
+                                               const std::string& error)>;
 
     ExecutionController(storage::StorageClient& storage, auth::AuthMiddleware& middleware,
                         WebSocketServer& ws_server, WorkflowScheduler& scheduler,
                         runners::LoadBalancer& load_balancer,
-                        FailureHandler on_failure = nullptr);
+                        FinishedHandler on_finished = nullptr);
 
     void registerRoutes(httplib::Server& server);
 
@@ -51,7 +55,7 @@ private:
     WebSocketServer& ws_server_;
     WorkflowScheduler& scheduler_;
     runners::LoadBalancer& load_balancer_;
-    FailureHandler on_failure_;
+    FinishedHandler on_finished_;
 };
 
 } // namespace smartbotic::webserver::api

src/webserver/api/workflow_controller.cpp

@@ -381,7 +381,15 @@ void WorkflowController::activateWorkflow(const httplib::Request& req, httplib::
                                            const auth::AuthContext& ctx) {
     std::string id = req.matches[1];
 
-    auto result = storage_.update("workflows", id, {{"active", true}, {"updatedAt", TimeUtils::nowMs()}}, 0, true);
+    // Turning it back on by hand clears the run of failures and the note about
+    // why it was switched off. Leaving the count would switch it off again on
+    // the very next failure, which is not what someone reaching for Activate
+    // means.
+    auto result = storage_.update("workflows", id,
+                                  {{"active", true},
+                                   {"consecutiveFailures", 0},
+                                   {"deactivatedReason", ""},
+                                   {"updatedAt", TimeUtils::nowMs()}}, 0, true);
     if (result.failed()) {
         sendError(res, "Workflow not found", 404);
         return;

src/webserver/webserver_service.cpp

@@ -219,8 +219,8 @@ void WebServerService::setupRoutes() {
     execution_ctrl_ = std::make_unique<api::ExecutionController>(
         *storage_, *auth_middleware_, *ws_server_, *scheduler_, *load_balancer_,
         [this](const std::string& workflow_id, const std::string& execution_id,
-               const std::string& error) {
-            runErrorWorkflow(workflow_id, execution_id, error);
+               bool failed, const std::string& error) {
+            noteExecutionOutcome(workflow_id, execution_id, failed, error);
         });
     file_ctrl_ = std::make_unique<api::FileController>(*storage_, *auth_middleware_);
     execution_ctrl_->registerRoutes(server);
@@ -350,6 +350,68 @@ void WebServerService::loadScheduledWorkflows() {
     LOG_INFO("Loaded {} scheduled workflows", registered_count);
 }
 
+void WebServerService::noteExecutionOutcome(const std::string& workflow_id,
+                                            const std::string& execution_id,
+                                            bool failed,
+                                            const std::string& error) {
+    if (failed) {
+        runErrorWorkflow(workflow_id, execution_id, error);
+    }
+
+    auto stored = storage_->get("workflows", workflow_id);
+    if (stored.failed()) {
+        return;
+    }
+
+    const auto workflow = stored.value();
+    const auto settings = workflow.value("settings", nlohmann::json::object());
+    const int limit = settings.value("deactivateAfterFailures", 0);
+    const int streak = workflow.value("consecutiveFailures", 0);
+
+    if (!failed) {
+        // Only written when there is something to clear, so an ordinary run does
+        // not cost a write.
+        if (streak != 0) {
+            storage_->update("workflows", workflow_id,
+                             {{"consecutiveFailures", 0}}, 0, true);
+        }
+        return;
+    }
+
+    const int next = streak + 1;
+    nlohmann::json patch = {{"consecutiveFailures", next}};
+
+    // Counting is worth doing even when nothing is switched off - it is the
+    // number someone looks at when asking how long this has been going wrong.
+    if (limit <= 0 || next < limit || workflow.value("active", false) != true) {
+        storage_->update("workflows", workflow_id, patch, 0, true);
+        return;
+    }
+
+    // Switched off, and said out loud. A workflow that simply appeared
+    // "Inactive" one morning with no reason recorded is worse than one that
+    // kept failing, because nobody can tell which of the two happened.
+    patch["active"] = false;
+    patch["deactivatedReason"] = "Switched off after " + std::to_string(next) +
+                                 " failures in a row. The last one: " +
+                                 (error.empty() ? "no reason given" : error);
+    patch["deactivatedAt"] = common::TimeUtils::nowMs();
+    patch["updatedAt"] = common::TimeUtils::nowMs();
+    storage_->update("workflows", workflow_id, patch, 0, true);
+
+    // Off the schedule too, or it would keep firing while reading as inactive.
+    scheduler_->unregisterWorkflow(workflow_id);
+
+    LOG_WARN("Workflow {} deactivated after {} consecutive failures: {}",
+             workflow_id, next, error);
+
+    ws_server_->broadcast("workflows.deactivated", {
+        {"id", workflow_id},
+        {"reason", patch["deactivatedReason"]},
+        {"consecutiveFailures", next}
+    });
+}
+
 void WebServerService::runErrorWorkflow(const std::string& failed_workflow_id,
                                         const std::string& failed_execution_id,
                                         const std::string& error_message) {

src/webserver/webserver_service.hpp

@@ -93,6 +93,15 @@ private:
 
     // Runs the workflow a failing workflow nominates as its error handler, if
     // it names one. Called when an execution reports failure.
+    // Count how many times in a row a workflow has failed, and switch it off
+    // when that passes what its settings allow. The count lives on the workflow
+    // rather than in memory: a restart must not forgive a workflow that has
+    // been failing for an hour, and the number is worth being able to see.
+    void noteExecutionOutcome(const std::string& workflow_id,
+                              const std::string& execution_id,
+                              bool failed,
+                              const std::string& error);
+
     void runErrorWorkflow(const std::string& failed_workflow_id,
                           const std::string& failed_execution_id,
                           const std::string& error_message);

tests/nodes/sdcpp-load-options-mismatch.json

@@ -1,19 +1,69 @@
 {
-  "name": "diag-load-options-mismatch",
-  "nodes": [
-    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
-    {"id": "health", "name": "Health", "type": "sdcpp-health", "position": {"x": 0, "y": 100},
-     "config": {"serverUrl": "http://mulan:8077"}},
-    {"id": "load", "name": "Load", "type": "sdcpp-model-load", "position": {"x": 0, "y": 220},
-     "config": {"serverUrl": "http://mulan:8077",
-                "credentialId": "cred_62748b40-e9d1-4659-b175-6dbc691cc0b1",
-                "modelName": "{{$node[\"Health\"].modelName}}",
-                "streamLayers": false,
-                "whenDifferent": "fail"}}
-  ],
-  "connections": [
-    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "health", "targetInput": "data"},
-    {"sourceNodeId": "health", "sourceOutput": "main", "targetNodeId": "load", "targetInput": "data"}
-  ],
-  "expect": {"load": {"status": "failed", "errorContains": "not with the settings"}}
-}
+ "name": "diag-load-options-mismatch",
+ "requires": {
+  "http": "http://mulan:8077/health",
+  "expect": {
+   "model_loaded": true
+  }
+ },
+ "nodes": [
+  {
+   "id": "n1",
+   "name": "Trigger",
+   "type": "click-trigger",
+   "position": {
+    "x": 0,
+    "y": 0
+   },
+   "config": {}
+  },
+  {
+   "id": "health",
+   "name": "Health",
+   "type": "sdcpp-health",
+   "position": {
+    "x": 0,
+    "y": 100
+   },
+   "config": {
+    "serverUrl": "http://mulan:8077"
+   }
+  },
+  {
+   "id": "load",
+   "name": "Load",
+   "type": "sdcpp-model-load",
+   "position": {
+    "x": 0,
+    "y": 220
+   },
+   "config": {
+    "serverUrl": "http://mulan:8077",
+    "credentialId": "cred_62748b40-e9d1-4659-b175-6dbc691cc0b1",
+    "modelName": "{{$node[\"Health\"].modelName}}",
+    "streamLayers": false,
+    "whenDifferent": "fail"
+   }
+  }
+ ],
+ "connections": [
+  {
+   "sourceNodeId": "n1",
+   "sourceOutput": "main",
+   "targetNodeId": "health",
+   "targetInput": "data"
+  },
+  {
+   "sourceNodeId": "health",
+   "sourceOutput": "main",
+   "targetNodeId": "load",
+   "targetInput": "data"
+  }
+ ],
+ "expect": {
+  "load": {
+   "status": "failed",
+   "errorContains": "not with the settings"
+  }
+ }
+}

tests/nodes/sdcpp-model-load-idempotent.json

@@ -1,98 +1,104 @@
 {
-  "name": "verify-sdcpp-model-load-idempotent",
-  "nodes": [
-    {
-      "id": "n1",
-      "name": "Trigger",
-      "type": "click-trigger",
-      "position": {
-        "x": 0,
-        "y": 0
-      },
-      "config": {}
-    },
-    {
-      "id": "srv",
-      "name": "SD.cpp Server",
-      "type": "configurator",
-      "position": {
-        "x": -260,
-        "y": 100
-      },
-      "config": {
-        "label": "SD.cpp Server",
-        "settings": [
-          {
-            "name": "serverUrl",
-            "value": "http://mulan:8077"
-          },
-          {
-            "name": "credentialId",
-            "value": "cred_62748b40-e9d1-4659-b175-6dbc691cc0b1"
-          }
-        ]
-      }
-    },
-    {
-      "id": "health",
-      "name": "Health",
-      "type": "sdcpp-health",
-      "position": {
-        "x": 0,
-        "y": 200
-      },
-      "config": {}
-    },
-    {
-      "id": "load",
-      "name": "Load",
-      "type": "sdcpp-model-load",
-      "position": {
-        "x": 0,
-        "y": 320
-      },
-      "config": {
-        "modelName": "{{$node[\"Health\"].modelName}}",
-        "streamLayers": true,
-        "eagerLoad": true
-      }
-    }
-  ],
-  "connections": [
-    {
-      "sourceNodeId": "n1",
-      "sourceOutput": "main",
-      "targetNodeId": "health",
-      "targetInput": "data"
-    },
-    {
-      "sourceNodeId": "health",
-      "sourceOutput": "main",
-      "targetNodeId": "load",
-      "targetInput": "data"
-    },
-    {
-      "sourceNodeId": "srv",
-      "sourceOutput": "main",
-      "targetNodeId": "health",
-      "targetInput": "config"
-    },
-    {
-      "sourceNodeId": "srv",
-      "sourceOutput": "main",
-      "targetNodeId": "load",
-      "targetInput": "config"
-    }
-  ],
-  "expectStatus": "completed",
+ "name": "verify-sdcpp-model-load-idempotent",
+ "requires": {
+  "http": "http://mulan:8077/health",
   "expect": {
-    "load": {
-      "status": "completed",
-      "output": {
-        "alreadyLoaded": true,
-        "loaded": false,
-        "reloadedFor": []
-      }
-    }
+   "model_loaded": true
   }
+ },
+ "nodes": [
+  {
+   "id": "n1",
+   "name": "Trigger",
+   "type": "click-trigger",
+   "position": {
+    "x": 0,
+    "y": 0
+   },
+   "config": {}
+  },
+  {
+   "id": "srv",
+   "name": "SD.cpp Server",
+   "type": "configurator",
+   "position": {
+    "x": -260,
+    "y": 100
+   },
+   "config": {
+    "label": "SD.cpp Server",
+    "settings": [
+     {
+      "name": "serverUrl",
+      "value": "http://mulan:8077"
+     },
+     {
+      "name": "credentialId",
+      "value": "cred_62748b40-e9d1-4659-b175-6dbc691cc0b1"
+     }
+    ]
+   }
+  },
+  {
+   "id": "health",
+   "name": "Health",
+   "type": "sdcpp-health",
+   "position": {
+    "x": 0,
+    "y": 200
+   },
+   "config": {}
+  },
+  {
+   "id": "load",
+   "name": "Load",
+   "type": "sdcpp-model-load",
+   "position": {
+    "x": 0,
+    "y": 320
+   },
+   "config": {
+    "modelName": "{{$node[\"Health\"].modelName}}",
+    "streamLayers": true,
+    "eagerLoad": true
+   }
+  }
+ ],
+ "connections": [
+  {
+   "sourceNodeId": "n1",
+   "sourceOutput": "main",
+   "targetNodeId": "health",
+   "targetInput": "data"
+  },
+  {
+   "sourceNodeId": "health",
+   "sourceOutput": "main",
+   "targetNodeId": "load",
+   "targetInput": "data"
+  },
+  {
+   "sourceNodeId": "srv",
+   "sourceOutput": "main",
+   "targetNodeId": "health",
+   "targetInput": "config"
+  },
+  {
+   "sourceNodeId": "srv",
+   "sourceOutput": "main",
+   "targetNodeId": "load",
+   "targetInput": "config"
+  }
+ ],
+ "expectStatus": "completed",
+ "expect": {
+  "load": {
+   "status": "completed",
+   "output": {
+    "alreadyLoaded": true,
+    "loaded": false,
+    "reloadedFor": []
+   }
+  }
+ }
 }

webui/src/components/workflow/WorkflowSettingsModal.tsx

@@ -93,6 +93,32 @@ export function WorkflowSettingsModal({
               <p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
                 The chosen workflow receives the failed workflow's name, the execution id and the error message.
               </p>
+
+              <div className="mt-3">
+                <label className="block text-sm text-gray-600 dark:text-gray-400 mb-1">
+                  Switch it off after this many failures in a row
+                </label>
+                <input
+                  type="number"
+                  min={0}
+                  value={settings.deactivateAfterFailures ?? 0}
+                  onChange={(e) => {
+                    const raw = e.target.value
+                    onSettingsChange({
+                      ...settings,
+                      // An empty box means off, not zero-and-therefore-immediately.
+                      deactivateAfterFailures: raw === '' ? 0 : Math.max(0, Number(raw)),
+                    })
+                  }}
+                  className="w-32 px-3 py-2 text-sm border border-gray-300 dark:border-slate-600 rounded-lg bg-white dark:bg-slate-900 text-gray-900 dark:text-gray-100"
+                />
+                <p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
+                  0 leaves it running whatever happens. Otherwise a workflow that fails
+                  this many times without a success in between is switched off, with the
+                  reason recorded. One success resets the count, and turning it back on
+                  by hand clears it.
+                </p>
+              </div>
             </div>
 
             {/* Storage Permissions */}

webui/src/pages/WorkflowsPage.tsx

@@ -624,15 +624,31 @@ function WorkflowCard({
         )}
       </div>
 
+      {/* A workflow the system switched off says so. "Inactive" on its own is
+          indistinguishable from someone having turned it off on purpose, which
+          is the difference that matters when something stopped running. */}
+      {!workflow.active && (workflow as any).deactivatedReason && (
+        <div className="flex items-start gap-1.5 mb-2 text-[11px] text-amber-600 dark:text-amber-400">
+          <AlertTriangle className="w-3.5 h-3.5 shrink-0 mt-px" />
+          <span className="break-words">{(workflow as any).deactivatedReason}</span>
+        </div>
+      )}
+
       <div className="flex items-center justify-between text-sm">
         <span
           className={`px-2 py-0.5 rounded-full text-xs font-medium ${
             workflow.active
               ? 'bg-green-100 dark:bg-green-900/30 text-green-700 dark:text-green-400'
+              : (workflow as any).deactivatedReason
+              ? 'bg-amber-100 dark:bg-amber-900/30 text-amber-700 dark:text-amber-400'
               : 'bg-gray-100 dark:bg-slate-700 text-gray-600 dark:text-gray-400'
           }`}
         >
-          {workflow.active ? 'Active' : 'Inactive'}
+          {workflow.active
+            ? 'Active'
+            : (workflow as any).deactivatedReason
+            ? 'Switched off'
+            : 'Inactive'}
         </span>
         <span className="text-gray-400 dark:text-gray-500">
           {workflow.nodes?.length || 0} node{workflow.nodes?.length === 1 ? '' : 's'}