Преглед изворни кода

feat: retry + resume — failed-task retry (summary moved to a comment and cleared, requeue bookkeeping), unreachable saddles requeue with an interruption comment, manual requeue-to-resume for tasks stuck on offline saddles, daemon 'resumed' status surfaced as a progress entry

clanker пре 1 месец
родитељ
комит
2a17623f47

+ 9 - 6
src/Services/Dispatcher.vala

@@ -109,7 +109,8 @@ namespace ClankerSaddle.Services {
                             foreach (var capability in yield capabilities.list_for_task(task.id)) {
                                 required.add(capability.name);
                             }
-                            print(@"[ClankerSaddle] Task $(task.id) waits for a capable saddle (requires: $(string.joinv(", ", required.to_array())))\n");
+                            string required_text = string.joinv(", ", required.to_array());
+                            print(@"[ClankerSaddle] Task $(task.id) waits for a capable saddle (requires: $(required_text))\n");
                         }
                         continue;
                     }
@@ -135,7 +136,7 @@ namespace ClankerSaddle.Services {
                         print(@"[ClankerSaddle] Dispatched task $(task.id) to saddle '$(((!)saddle).name)' (session $(session.id))\n");
                     } catch (Error e) {
                         warning(@"[ClankerSaddle] Pushing task $(task.id) to saddle $(((!)saddle).id) failed: $(e.message)");
-                        yield abandon_saddle(((!)saddle).id, saddles);
+                        yield abandon_saddle(((!)saddle).id, saddles, tasks);
                     }
                 }
             }
@@ -152,7 +153,7 @@ namespace ClankerSaddle.Services {
                     record_strike(saddle.id, count);
                     warning(@"[ClankerSaddle] Polling saddle $(saddle.id) failed (attempt $(count)/3): $(e.message)");
                     if (count >= 3) {
-                        yield abandon_saddle(saddle.id, saddles);
+                        yield abandon_saddle(saddle.id, saddles, tasks);
                     }
                 }
             }
@@ -197,11 +198,13 @@ namespace ClankerSaddle.Services {
 
         /**
          * Declares a saddle unreachable once its failures accumulated:
-         * the task it held returns to the queue and its session closes.
+         * the task it held returns to the queue (with a bot comment) and
+         * its session closes.
          */
-        private async void abandon_saddle(int64 saddle_id, SaddleService saddles) throws Error {
+        private async void abandon_saddle(int64 saddle_id, SaddleService saddles,
+                TaskService tasks) throws Error {
             record_strike(saddle_id, 0);
-            yield saddles.mark_saddle_unreachable(saddle_id);
+            yield saddles.mark_saddle_unreachable(saddle_id, tasks);
         }
 
         /**

+ 15 - 0
src/Services/SaddlePollState.vala

@@ -22,6 +22,7 @@ namespace ClankerSaddle.Services {
         private HashSet<string> raw_polling_started = new HashSet<string>();
         private HashSet<string> delivered_answers = new HashSet<string>();
         private HashSet<string> delivered_comments = new HashSet<string>();
+        private HashSet<string> resumed_notified = new HashSet<string>();
         private Mutex state_lock = Mutex();
 
         /** The last output entry id stored for `session_id` (0 when none). */
@@ -80,6 +81,20 @@ namespace ClankerSaddle.Services {
             state_lock.unlock();
         }
 
+        /**
+         * Marks `session_id`'s daemon-side resume as notified; returns
+         * false when the progress entry already went out.
+         */
+        public bool mark_resumed(int64 session_id) {
+            state_lock.lock();
+            bool already = resumed_notified.has(session_id.to_string());
+            if (!already) {
+                resumed_notified.add(session_id.to_string());
+            }
+            state_lock.unlock();
+            return !already;
+        }
+
         /**
          * Marks `block_id`'s answer as delivered; returns false when it
          * had already been handed over.

+ 22 - 3
src/Services/SaddleService.vala

@@ -293,10 +293,14 @@ namespace ClankerSaddle.Services {
 
         /**
          * Marks a saddle offline when its control server cannot be
-         * reached, releasing any task it was holding back to 'queued' and
-         * closing that task's open session (outcome 'stopped').
+         * reached, releasing the task it was holding back to 'queued'
+         * and closing that task's open session (outcome 'stopped'). The
+         * requeue lands as a bot comment on the task, so the timeline
+         * shows the interruption; combined with the daemon's
+         * crash-resume, the next dispatch restores the workspace and
+         * reports `resumed` on /status.
          */
-        public async void mark_saddle_unreachable(int64 id) throws Error {
+        public async void mark_saddle_unreachable(int64 id, TaskService tasks) throws Error {
             var saddle = yield get_saddle(id);
             if (saddle == null) {
                 return;
@@ -311,6 +315,8 @@ namespace ClankerSaddle.Services {
                         || ((!)task).status == STATUS_REVIEW_REQUESTED)) {
                     ((!)task).status = STATUS_QUEUED;
                     yield db.update_async<TaskEntity>((!)task);
+                    yield tasks.add_comment(task_id, "bot",
+                        "Saddle became unreachable; task requeued");
                 }
 
                 var sessions = yield db.query<ClankerSaddle.Data.Session>()
@@ -616,6 +622,19 @@ namespace ClankerSaddle.Services {
 
             var status = yield request_saddle_json(((!)saddle).host, "GET", "/status", null);
             string state = status.has_member("state") ? status.get_string_member("state") : "";
+
+            // A crash-resumable daemon reports `resumed: true` on the
+            // first status after restoring a workspace: surface it once
+            // per session as a progress entry (the timeline's
+            // "interrupted → resumed" arc). Absent on older daemons.
+            if (status.has_member("resumed")
+                    && status.get_member("resumed").get_value_type() == typeof(bool)
+                    && status.get_boolean_member("resumed")
+                    && session != null && poll_state.mark_resumed(((!)session).id)) {
+                yield tasks.add_progress(task_id,
+                    "Saddle restored the session context; task resumed");
+            }
+
             switch (state) {
                 case "done":
                     yield tasks.complete_task(task_id,

+ 31 - 0
src/Services/TaskService.vala

@@ -547,6 +547,37 @@ namespace ClankerSaddle.Services {
             yield pulse.bump(task_id, "task requeued");
         }
 
+        /**
+         * Returns a failed task to the queue for another attempt. The
+         * failure summary moves into a bot comment (so the history keeps
+         * it) before being cleared from the task, `completed_at` is
+         * withdrawn, any open session closes and its saddle frees — the
+         * same bookkeeping as {@link requeue_task}, plus the audit
+         * comment "Task retried by user".
+         */
+        public async void retry_task(int64 task_id) throws Error {
+            var task = yield db.query<TaskEntity>()
+                .where(expr("id == $0", elem<int64?>(task_id)))
+                .first_async();
+            if (task == null) {
+                throw new Error(Quark.from_string("clanker-saddle"), 8,
+                    @"No task exists with id $(task_id)");
+            }
+
+            if (((!)task).summary != null && ((!)task).summary.length > 0) {
+                yield add_comment(task_id, "bot", (!)(((!)task).summary));
+            }
+            ((!)task).status = STATUS_QUEUED;
+            ((!)task).summary = null;
+            ((!)task).completed_at = null;
+            yield db.update_async<TaskEntity>((!)task);
+
+            yield close_open_sessions(task_id, "stopped");
+            yield free_saddles_running(task_id);
+            yield add_comment(task_id, "bot", "Task retried by user");
+            yield pulse.bump(task_id, "task retried");
+        }
+
         /**
          * Queued tasks whose dependencies are all done, ordered by priority
          * (1 first) then creation time — the dispatcher's candidate list.

+ 81 - 0
src/actions/TaskActions.vala

@@ -363,6 +363,87 @@ namespace ClankerSaddle {
         }
     }
 
+    /**
+     * Retries a failed task: the requeue path with the failure summary
+     * moved into a comment (and cleared), all history kept. The next
+     * dispatch resumes the daemon-side workspace when it still exists.
+     */
+    public class RetryTaskAction : SealedTaskAction {
+
+        private TaskService tasks = inject<TaskService>();
+        private UserService users = inject<UserService>();
+
+        public override async DirectiveBuilder handle() throws GLib.Error {
+            var guard = yield SpryAuth.require_login(directives(), request, users);
+            if (guard != null) {
+                return (!)guard;
+            }
+
+            int64 task_id = sealed_task_id();
+            if (task_id <= 0) {
+                return directives().navigate("/tasks");
+            }
+
+            var detail = yield tasks.get_task(task_id);
+            if (detail == null) {
+                return directives().navigate("/tasks");
+            }
+            if (((!)detail).task.status != STATUS_FAILED) {
+                return directives().notify("error", "Only failed tasks can be retried")
+                    .navigate(@"/tasks/$(task_id)");
+            }
+
+            try {
+                yield tasks.retry_task(task_id);
+                return directives().notify("success", "Task requeued for another attempt")
+                    .navigate(@"/tasks/$(task_id)");
+            } catch (Error e) {
+                return directives().notify("error", @"Could not retry the task: $(e.message)")
+                    .navigate(@"/tasks/$(task_id)");
+            }
+        }
+    }
+
+    /**
+     * Manually requeues a task stuck working/blocked on a saddle that
+     * went offline (the automatic three-strike path may never fire when
+     * the saddle row was refreshed to offline first). The same path as
+     * the dispatcher's abandon: requeue + bot comment + saddle offline.
+     */
+    public class ResumeTaskAction : SealedTaskAction {
+
+        private TaskService task_service = inject<TaskService>();
+        private SaddleService saddle_service = inject<SaddleService>();
+        private UserService users = inject<UserService>();
+
+        public override async DirectiveBuilder handle() throws GLib.Error {
+            var guard = yield SpryAuth.require_login(directives(), request, users);
+            if (guard != null) {
+                return (!)guard;
+            }
+
+            int64 task_id = sealed_task_id();
+            if (task_id <= 0) {
+                return directives().navigate("/tasks");
+            }
+
+            try {
+                var session = yield task_service.open_session_for_task(task_id);
+                if (session != null) {
+                    yield saddle_service.mark_saddle_unreachable(((!)session).saddle_id, task_service);
+                } else {
+                    yield task_service.requeue_task(task_id);
+                }
+                return directives().notify("success",
+                        "Task requeued; the daemon restores its context on the next dispatch")
+                    .navigate(@"/tasks/$(task_id)");
+            } catch (Error e) {
+                return directives().notify("error", @"Could not requeue the task: $(e.message)")
+                    .navigate(@"/tasks/$(task_id)");
+            }
+        }
+    }
+
     /**
      * The task detail page's polling action: rebuilds the `task` slot from
      * current data so the three-second client poll shows live comments,

+ 15 - 0
src/entrypoints/TaskDetailEntrypoint.vala

@@ -107,6 +107,7 @@ namespace ClankerSaddle {
         public bool review_pending { get; set; }
         public string review_summary { get; set; default = ""; }
         public string review_diff_url { get; set; default = ""; }
+        public bool resume_available { get; set; }
         public int64 project_id { get; set; }
         public string project_name { get; set; default = ""; }
         public string git_url { get; set; default = ""; }
@@ -121,6 +122,8 @@ namespace ClankerSaddle {
         public Statum.Model.ActionDto? dependency { get; set; }
         public Statum.Model.ActionDto? complete { get; set; }
         public Statum.Model.ActionDto? cancel { get; set; }
+        public Statum.Model.ActionDto? retry { get; set; }
+        public Statum.Model.ActionDto? resume { get; set; }
         public Statum.Model.ActionDto? refresh { get; set; }
         public Statum.Model.ActionDto? attach_rule { get; set; }
         public Statum.Model.ActionDto? attach_capability { get; set; }
@@ -183,6 +186,8 @@ namespace ClankerSaddle {
             pub.dependency = actions.author_private<AddDependencyAction, TaskActionPrivate>(sealed);
             pub.complete = actions.author_private<CompleteTaskAction, TaskActionPrivate>(sealed);
             pub.cancel = actions.author_private<CancelTaskAction, TaskActionPrivate>(sealed);
+            pub.retry = actions.author_private<RetryTaskAction, TaskActionPrivate>(sealed);
+            pub.resume = actions.author_private<ResumeTaskAction, TaskActionPrivate>(sealed);
             pub.refresh = actions.author_private<RefreshTaskDataAction, TaskActionPrivate>(sealed, "GET");
             pub.attach_rule = actions.author_private<AttachTaskRuleAction, TaskActionPrivate>(sealed);
             pub.attach_capability = actions.author_private<AttachTaskCapabilityAction, TaskActionPrivate>(sealed);
@@ -325,6 +330,16 @@ namespace ClankerSaddle {
                     outcome = session.outcome ?? "",
                     open = session.ended_at == null
                 });
+                // A task stuck working/blocked whose open session sits on
+                // an offline saddle offers the manual requeue-for-resume
+                // path (the automatic three-strike abandon only fires
+                // while the dispatcher still polls the saddle).
+                if (session.ended_at == null && saddle != null
+                        && ((!)saddle).status == "offline"
+                        && (task.status == STATUS_WORKING || task.status == STATUS_BLOCKED
+                            || task.status == STATUS_REVIEW_REQUESTED)) {
+                    pub.resume_available = true;
+                }
             }
 
             var props = (PropertyDictionary) GObjectMapping.to_properties(pub);

+ 2 - 0
src/main.vala

@@ -94,6 +94,8 @@ int main(string[] args) {
         statum.action<AddDependencyAction>();
         statum.action<CompleteTaskAction>();
         statum.action<CancelTaskAction>();
+        statum.action<RetryTaskAction>();
+        statum.action<ResumeTaskAction>();
         statum.action<RefreshTaskDataAction>();
         statum.action<RefreshDashboardAction>();
         statum.action<RegisterSaddleAction>();

+ 5 - 0
src/pages/task_detail.html

@@ -231,6 +231,11 @@
                     <p stm-if="task.working_branch.length > 0" class="muted">Working branch: <span stm-text="task.working_branch">branch</span></p>
                     <form class="inline">
                         <input type="hidden" name="task_id" stm-attribute.value="task.id">
+                        <button type="button" stm-if="task.status == 'failed'" class="primary"
+                                stm-action="task.retry">Retry task</button>
+                        <button type="button" stm-if="task.resume_available"
+                                stm-action="task.resume"
+                                stm-confirm="Requeue this task? The daemon restores its workspace context on the next dispatch.">Requeue to resume</button>
                         <button type="button" stm-action="task.cancel" stm-confirm="Cancel this task?">Cancel task</button>
                     </form>
                 </section>

+ 103 - 5
test/web_stack_test.py

@@ -201,6 +201,9 @@ class FakeSaddle:
         self.state = "idle"
         self.summary = ""
         self.status_polls_after_start = 0
+        self.fail_next = False          # next /start drives the task to failed
+        self.start_counts = {}          # task id -> /start count
+        self.resumed_pending = False    # crash-resume flag on the next /status
 
     def append_chunk(self, text):
         with self.lock:
@@ -239,10 +242,19 @@ class FakeSaddle:
                 except ValueError:
                     payload = {"raw": raw}
                 self.daemon.start_payloads.append(payload)
+                task_id = (payload.get("task") or {}).get("id")
+                count = self.daemon.start_counts.get(task_id, 0) + 1
+                self.daemon.start_counts[task_id] = count
+                if count > 1:
+                    # Crash-resume contract: re-pushing a task whose
+                    # workspace still exists restores it; the first
+                    # /status after the restore reports "resumed": true.
+                    self.daemon.resumed_pending = True
                 self.daemon.append_chunk(self.daemon.PROMPT)
                 self.daemon.append_chunk("echo hello\r\n")
                 self.daemon.append_chunk("hello\r\n")
-                self.daemon.state = "working"
+                self.daemon.state = "failing" if self.daemon.fail_next else "working"
+                self.daemon.fail_next = False
                 self.daemon.status_polls_after_start = 0
                 self._send(200, {})
             elif self.path == "/answer":
@@ -252,13 +264,19 @@ class FakeSaddle:
 
         def do_GET(self):
             if self.path == "/status":
-                if self.daemon.state == "working":
+                response = {"state": self.daemon.state, "summary": self.daemon.summary}
+                if self.daemon.resumed_pending:
+                    response["resumed"] = True
+                    self.daemon.resumed_pending = False
+                if self.daemon.state in ("working", "failing"):
                     self.daemon.status_polls_after_start += 1
-                    if self.daemon.status_polls_after_start >= 2:
+                    if self.daemon.state == "failing" and self.daemon.status_polls_after_start >= 1:
+                        self.daemon.state = "failed"
+                        self.daemon.summary = "bash -n rejected the script (simulated)"
+                    elif self.daemon.state == "working" and self.daemon.status_polls_after_start >= 2:
                         self.daemon.state = "done"
                         self.daemon.summary = "fake daemon finished"
-                self._send(200, {"state": self.daemon.state,
-                                 "summary": self.daemon.summary})
+                self._send(200, response)
             elif self.path.startswith("/output"):
                 self._send(200, [])
             elif self.path.startswith("/log"):
@@ -741,6 +759,86 @@ def main():
         check(all(c["name"] != "bigmem"
                   for c in saddle_card("fake-saddle")["capabilities"]),
               "deleting a capability drops it from saddles")
+
+        print("== retry + resume ==")
+        # A daemon-side failure fails the task with its explanation.
+        fake.fail_next = True
+        act(auth, slot_public(auth, "/tasks", "tasks")["create"], {
+            "project_id": "1",
+            "title": "Flaky task",
+            "priority": "2",
+        })
+        flaky = int([t["id"] for t in slot_public(auth, "/tasks", "tasks")["tasks"]
+                     if t["title"] == "Flaky task"][0])
+        check(wait_for(lambda: task_state(auth, flaky).get("status") == "failed" or False,
+                       timeout=60, what="flaky task fails") is not None,
+              "daemon failure fails the task")
+        detail = task_state(auth, flaky)
+        check("bash -n rejected" in (detail.get("summary") or ""),
+              "failure summary shows the daemon's explanation")
+
+        # Retry: summary moves into a comment and clears, task requeues,
+        # history stays, and the re-dispatch restores the workspace.
+        comments_before = len(detail.get("comments", []))
+        progress_before = len(detail.get("progress", []))
+        act(auth, detail["retry"], {})
+        detail = task_state(auth, flaky)
+        check(detail["status"] == "queued" and (detail.get("summary") or "") == "",
+              "retry requeues the task and clears the stale summary")
+        check(any("bash -n rejected" in c["body"] for c in detail["comments"]),
+              "failure explanation kept as a bot comment")
+        check(any(c["body"] == "Task retried by user" for c in detail["comments"]),
+              "retry lands its audit comment")
+        check(len(detail["comments"]) >= comments_before + 1,
+              "retry keeps the comment history")
+        check(wait_task_done(auth, flaky, timeout=60) is not None,
+              "retried task dispatches again and completes")
+        detail = task_state(auth, flaky)
+        check(any("restored the session context" in p["summary"]
+                  for p in detail.get("progress", [])),
+              "daemon resume reported on the first status after the restore")
+        check(len(detail["progress"]) > progress_before,
+              "resume note appended to the progress timeline")
+
+        # Manual resume: a task stuck working on an offline saddle.
+        act(auth, slot_public(auth, "/tasks", "tasks")["create"], {
+            "project_id": "1",
+            "title": "Stuck task",
+            "priority": "2",
+        })
+        stuck = int([t["id"] for t in slot_public(auth, "/tasks", "tasks")["tasks"]
+                     if t["title"] == "Stuck task"][0])
+        check(wait_for(lambda: task_state(auth, stuck).get("status") == "working" or False,
+                       timeout=60, what="stuck task starts working") is not None,
+              "stuck task reaches working")
+        conn = db()
+        conn.execute("update saddles set status = 'offline' where name = 'fake-saddle'")
+        conn.commit()
+        conn.close()
+        detail = task_state(auth, stuck)
+        check(detail.get("resume_available") is True,
+              "task page offers resume while its saddle is offline")
+        act(auth, detail["resume"], {})
+        detail = task_state(auth, stuck)
+        check(detail["status"] == "queued",
+              "manual resume requeues the stuck task")
+        check(any(c["body"] == "Saddle became unreachable; task requeued"
+                  for c in detail["comments"]),
+              "manual resume lands the interruption comment")
+        check(all(s["status"] == "offline"
+                  for s in slot_public(auth, "/saddles", "saddles")["saddles"]),
+              "saddle stays offline after the manual requeue")
+
+        # Bringing the saddle back lets the requeued task resume and finish.
+        fake.state = "idle"
+        fake_card = saddle_card("fake-saddle")
+        act(auth, fake_card["refresh"], {})
+        check(any(s["status"] == "idle"
+                  for s in slot_public(auth, "/saddles", "saddles")["saddles"]
+                  if s["name"] == "fake-saddle"),
+              "refreshed saddle returns idle")
+        check(wait_task_done(auth, stuck, timeout=60) is not None,
+              "requeued task resumes on the refreshed saddle and completes")
     finally:
         app.terminate()
         try: