瀏覽代碼

fix: turning a node off survives the save, and does not stop the loop

Turning a node off and saving brought it back on. The flag was toggled on the
canvas and read back on load, but the save never sent it - so the next save
wrote the workflow without it and the node returned, which looks like the
editor refusing to do as it is told.

Fixing that exposed a second one underneath. A node turned off is transparent
everywhere else - the comment in the engine says so, and the flow carries on
without it - but the loop's back-edge check counted only nodes that had
completed. So turning off a notification that happens to sit on the way back
to the Loop quietly stopped the loop after one item. That is a very surprising
thing for an off switch to do, and it would have looked like the loop being
broken rather than the node being off.

Carrying on is precisely what reaching the loop means, so a disabled
pass-through now counts. A disabled node that decides a branch still does not:
it has no answer to give, so the path really does end there.

The check had to go where the disabled branch actually is - that path returns
early, before the one further down, which is why the first attempt at this
made no difference and the fixture caught it.

New fixture: three items, a disabled node on the back edge, all three
processed and the done branch reached. 66 passed.

35photo2anime's Nextcloud Talk node is left off, which is what was asked for
before it stopped working.
fszontagh 1 月之前
父節點
當前提交
6bc321dd91
共有 3 個文件被更改,包括 59 次插入 和 0 次删除
  1. 24 0
      src/runner/workflow_engine.cpp
  2. 30 0
      tests/nodes/loop-disabled-back-edge.json
  3. 5 0
      webui/src/pages/WorkflowEditorPage.tsx

+ 24 - 0
src/runner/workflow_engine.cpp

@@ -2506,6 +2506,20 @@ bool WorkflowEngine::executeLoopBody(
 
                 iteration_results[body_node_id] = disabled_result;
                 result.node_results[body_node_id + "_iter_" + std::to_string(i)] = disabled_result;
+
+                // This returns early, before the back-edge check further down,
+                // so the same rule has to be applied here. Everywhere else a
+                // disabled pass-through is transparent and the flow carries on
+                // without it - and carrying on is precisely what reaching the
+                // loop means. Without this, turning off a notification that
+                // happens to sit on the way back stops the loop after one item,
+                // which is a very surprising thing for an off switch to do.
+                // A disabled node that decides a branch is different: it has no
+                // answer to give, so the path really does end there.
+                if (!disabled_result.output.contains("_activeBranch") &&
+                    back_edge_sources.contains(body_node_id)) {
+                    reached_back_edge = true;
+                }
                 continue;
             }
 
@@ -2726,6 +2740,16 @@ bool WorkflowEngine::executeLoopBody(
             // Only a node that actually ran counts. A node on a branch that was
             // not taken is skipped, and a skipped loop-back edge is exactly the
             // case that should end the iteration.
+            //
+            // A node turned off is not that case. Everywhere else a disabled
+            // pass-through is transparent and the flow carries on without it,
+            // and carrying on is precisely what reaching the loop means - so
+            // turning off a notification that happens to sit on the way back
+            // must not quietly stop the loop after one item. A disabled node
+            // that decides a branch is different: it has no answer to give, so
+            // nothing after it runs and the path really does end there.
+            // A disabled node never reaches here - it is handled above, where
+            // the same rule about transparency is applied.
             if (body_result.status == NodeStatus::Completed &&
                 back_edge_sources.contains(body_node_id)) {
                 reached_back_edge = true;

+ 30 - 0
tests/nodes/loop-disabled-back-edge.json

@@ -0,0 +1,30 @@
+{
+  "name": "verify-loop-disabled-back-edge",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "items", "name": "Items", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { items: ['a', 'b', 'c'] };"}},
+    {"id": "loop", "name": "Loop", "type": "loop", "position": {"x": 0, "y": 200},
+     "config": {"inputField": "data.result.items", "continueOnError": true}},
+    {"id": "work", "name": "Work", "type": "set-fields", "position": {"x": 0, "y": 300},
+     "config": {"mode": "only-set", "fields": [{"name": "seen", "value": "{{data.loop.item}}"}]}},
+    {"id": "notify", "name": "Notify", "type": "set-fields", "position": {"x": 0, "y": 400},
+     "disabled": true,
+     "config": {"mode": "only-set", "fields": [{"name": "told", "value": "yes"}]}},
+    {"id": "after", "name": "After", "type": "code", "position": {"x": 0, "y": 520},
+     "config": {"code": "return { done: true };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "items", "targetInput": "data"},
+    {"sourceNodeId": "items", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "loop", "targetNodeId": "work", "targetInput": "data"},
+    {"sourceNodeId": "work", "sourceOutput": "main", "targetNodeId": "notify", "targetInput": "data"},
+    {"sourceNodeId": "notify", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "done", "targetNodeId": "after", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "after": {"status": "completed"},
+    "loop": {"status": "completed", "output": {"done": {"totalProcessed": 3}}}
+  }
+}

+ 5 - 0
webui/src/pages/WorkflowEditorPage.tsx

@@ -1437,6 +1437,11 @@ function WorkflowEditorInner() {
       name: node.data.label,
       config: node.data.config || {},
       position: node.position,
+      // Turning a node off has to survive the save. It was toggled on the
+      // canvas and read back on load, but never written - so a node switched
+      // off came back on at the next save, which looks like the editor
+      // refusing to do as it is told.
+      disabled: node.data.disabled === true,
     }))
 
     const workflowConnections = edges.map((edge) => ({