Browse Source

fix: rescue an execution stranded by a crash mid-resume

Answering a paused run claims its record first - status "resuming", pause
token cleared - and only then runs the second half. A crash in between left
the record somewhere nothing could reach: not "waiting", so the pending
listing no longer showed it and resume() refused it; no token, so the
approver's link was dead; and not "running", so reconcileOrphanedExecutions
skipped it. There was no way back short of editing the database by hand.

reconcileOrphanedExecutions now covers "resuming" alongside "running", and
says which of the two happened - a lost resume is a different loss, because
the approval was answered and the answer was never applied, and whoever
answered is entitled to know it did not take effect.

The claim also stamps the runner doing the resuming. It kept the runner id of
whoever ran the first half, and a paused run can be answered on a different
runner from the one that parked it, so the reconcile would have looked for the
claim under the wrong runner and never found it.

"resuming" now renders as a status in both execution lists rather than falling
through to the grey clock that means idle. It is not in the ExecutionStatus
enum and deliberately stays out: nothing converts the string back to the enum,
so a member would be dead code.

Verified by stranding a real paused run exactly as the claim write does, then
restarting the runner: resuming -> cancelled, carrying the message above, and
confirmed invisible to the pending listing while stranded.
fszontagh 1 month ago
parent
commit
20fc504a15

+ 7 - 0
src/runner/workflow_engine.cpp

@@ -1255,6 +1255,13 @@ common::Result<ExecutionResult> WorkflowEngine::resume(const std::string& execut
     claim["status"] = "resuming";
     claim["pauseToken"] = "";
     claim["pausedNodeId"] = "";
+    // Stamped with whoever is resuming, not whoever ran the first half. A
+    // paused run can be answered on a different runner from the one that
+    // parked it, and the record carried the original runner's id until this
+    // line - so if this process died during the window below, the reconcile
+    // that closes orphans would look for the claim under the wrong runner and
+    // never find it.
+    claim["runnerId"] = config_.runner_id;
 
     auto claim_result = storage_.update("executions", execution_id, claim, expected_version);
     if (claim_result.failed()) {

+ 20 - 2
src/webserver/webserver_service.cpp

@@ -728,7 +728,17 @@ void WebServerService::reconcileOrphanedExecutions(const std::string& runner_id,
 
     storage::QueryOptions options;
     options.filters.push_back({"runnerId", runner_id});
-    options.filters.push_back({"status", "running"});
+    // "resuming" belongs here as much as "running" does.
+    //
+    // Answering a paused run claims its record first - status "resuming", pause
+    // token cleared - and only then runs the second half. A crash in between
+    // left the record in a state nothing could reach: not "waiting", so
+    // GET /executions/pending no longer listed it and resume() refused it; no
+    // token, so the approver's link was dead; and not "running", so this
+    // reconcile skipped it. The run was stuck there permanently with no way
+    // back short of editing the database by hand.
+    options.filters.push_back({"status", nlohmann::json{{"op", "in"},
+                                                        {"value", nlohmann::json::array({"running", "resuming"})}}});
     options.page = 1;
     options.page_size = 500;
 
@@ -748,9 +758,17 @@ void WebServerService::reconcileOrphanedExecutions(const std::string& runner_id,
 
         // Deliberately not touching Waiting. That is a run parked on a person,
         // not on a runner, and it is meant to outlive one.
+        //
+        // A claimed resume says so, because the two are not the same loss: the
+        // approval was answered and the answer was never applied, and whoever
+        // gave it is entitled to know it did not take effect.
+        const bool was_resuming = record.value("status", "") == "resuming";
         const nlohmann::json patch = {
             {"status", "cancelled"},
-            {"error", "The runner restarted while this was running, so nothing was left to finish it"},
+            {"error", was_resuming
+                 ? "The runner restarted while this was being resumed, so the answer it was "
+                   "given was never applied. The run cannot be answered again - start it afresh."
+                 : "The runner restarted while this was running, so nothing was left to finish it"},
             {"finishedAt", common::TimeUtils::nowMs()}
         };
         if (storage_->update("executions", id, patch, 0, true).ok()) {

+ 3 - 1
webui/src/components/workflow/ExecutionListPanel.tsx

@@ -86,13 +86,15 @@ function StatusBadge({ status }: { status: string }) {
     running: { icon: Loader2, color: 'text-blue-600 dark:text-blue-400', bg: 'bg-blue-100 dark:bg-blue-900/30' },
     pending: { icon: Clock, color: 'text-gray-600 dark:text-gray-400', bg: 'bg-gray-100 dark:bg-gray-800' },
     cancelled: { icon: AlertCircle, color: 'text-orange-600 dark:text-orange-400', bg: 'bg-orange-100 dark:bg-orange-900/30' },
+    waiting: { icon: Clock, color: 'text-amber-600 dark:text-amber-400', bg: 'bg-amber-100 dark:bg-amber-900/30' },
+    resuming: { icon: Loader2, color: 'text-blue-600 dark:text-blue-400', bg: 'bg-blue-100 dark:bg-blue-900/30' },
   }
 
   const { icon: Icon, color, bg } = config[status] || config.pending
 
   return (
     <span className={`inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium ${color} ${bg}`}>
-      <Icon className={`w-3 h-3 ${status === 'running' ? 'animate-spin' : ''}`} />
+      <Icon className={`w-3 h-3 ${status === 'running' || status === 'resuming' ? 'animate-spin' : ''}`} />
       {status}
     </span>
   )

+ 7 - 0
webui/src/pages/ExecutionsPage.tsx

@@ -297,6 +297,11 @@ export default function ExecutionsPage() {
         // needs an answer, and stopping it is a different act from stopping
         // something mid-flight.
         return <Hourglass className="w-5 h-5 text-amber-500" />
+      case 'resuming':
+        // The brief window between an approval being answered and the second
+        // half of the run starting. It is working, not idle, so it reads like
+        // running rather than falling to the grey clock below.
+        return <RefreshCw className="w-5 h-5 text-blue-500 animate-spin" />
       default:
         return <Clock className="w-5 h-5 text-gray-400" />
     }
@@ -314,6 +319,8 @@ export default function ExecutionsPage() {
         return 'bg-gray-100 dark:bg-slate-700 text-gray-700 dark:text-gray-400'
       case 'waiting':
         return 'bg-amber-100 dark:bg-amber-900/30 text-amber-700 dark:text-amber-400'
+      case 'resuming':
+        return 'bg-blue-100 dark:bg-blue-900/30 text-blue-700 dark:text-blue-400'
       default:
         return 'bg-gray-100 dark:bg-slate-700 text-gray-600 dark:text-gray-400'
     }