Explorar o código

r2: release_task — the ONE owner of close-open-sessions + free/requeue-saddle (arch 13): TaskService.release_task(task_id, outcome, requeue) folds close_open_sessions/free_saddles_running (their guarded UPDATEs byte-identical, the ask-race comment kept), complete/cancel/fail/requeue/retry collapse onto it; mark_saddle_unreachable keeps its keep-parked gate + 4-family flip + comment and its own guarded OFFLINE write, then releases the held task's sessions through the owner (the idle-free no-ops against the offline row — current_task_id already cleared under the entry-seen guard); unregister releases its held task through the requeue path too (deviation: the old inline 4-family flip left suspended tasks parked and open sessions dangling on deleted rows — the plan's normal form routes both through the owner; unpinned before, pinned now). Tests: the release outcomes (stopped requeue / done terminal) + both unregister routes (dead-host offline release, session-less direct release) — 849 -> 856.

clanker hai 1 mes
pai
achega
8c55db2b38
Modificáronse 3 ficheiros con 189 adicións e 75 borrados
  1. 36 39
      src/Services/SaddleService.vala
  2. 55 36
      src/Services/TaskService.vala
  3. 98 0
      test/web_stack_test.py

+ 36 - 39
src/Services/SaddleService.vala

@@ -292,9 +292,11 @@ namespace ClankerSaddle.Services {
         }
 
         /**
-         * Removes a saddle registration; tasks it was running are
-         * re-queued. Revocation: the row (and its per-saddle token)
-         * dies here, and every legacy `api_tokens` row held for the
+         * Removes a saddle registration; the task it was running
+         * releases through the ONE path — back to the queue, its open
+         * sessions closed stopped (no orphan open session outlives the
+         * row). Revocation: the row (and its per-saddle token) dies
+         * here, and every legacy `api_tokens` row held for the
          * saddle is deleted — nothing authorises `/api/saddle/*` for
          * it afterwards.
          */
@@ -304,15 +306,7 @@ namespace ClankerSaddle.Services {
                 return;
             }
             if (((!)saddle).current_task_id > 0) {
-                var task = yield db.query<TaskEntity>()
-                    .where(expr("id == $0", elem<int64?>(((!)saddle).current_task_id)))
-                    .first_async();
-                if (task != null && (((!)task).status == STATUS_ASSIGNED
-                        || ((!)task).status == STATUS_WORKING || ((!)task).status == STATUS_BLOCKED
-                        || ((!)task).status == STATUS_REVIEW_REQUESTED)) {
-                    ((!)task).status = STATUS_QUEUED;
-                    yield db.update_async<TaskEntity>((!)task);
-                }
+                yield task_service.release_task(((!)saddle).current_task_id, "stopped", true);
             }
             foreach (var row in yield db.query<ApiToken>()
                     .where(expr("saddle_id == $0", elem<int64?>(id)))
@@ -571,8 +565,10 @@ namespace ClankerSaddle.Services {
          * pass they would wait forever for a daemon that will not answer.
          * Only `assigned`/`working` tasks move: parked tasks
          * (blocked/review/suspended) keep their sessions, which the
-         * resume machinery needs. Each move lands a agent comment so the
-         * timeline shows why the task restarted.
+         * resume machinery needs. Each move is {@link TaskService.requeue_task}
+         * — the release owner closes the session and frees the saddle —
+         * plus a agent comment so the timeline shows why the task
+         * restarted.
          */
         public async void requeue_tasks_on_offline_saddles(TaskService tasks) throws Error {
             foreach (var status in new string[] { STATUS_ASSIGNED, STATUS_WORKING }) {
@@ -633,11 +629,13 @@ namespace ClankerSaddle.Services {
         /**
          * Marks a saddle offline when its control server cannot be
          * reached, releasing the task it was holding back to 'queued'
-         * and closing that task's open session (outcome 'stopped'). The
-         * requeue lands as a agent 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.
+         * and closing that task's open session (outcome 'stopped' —
+         * {@link TaskService.release_task} owns that bookkeeping; the
+         * saddle row's own fate is the guarded offline write, never
+         * the idle free). The requeue lands as a agent 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, TaskService tasks,
                 string? reason = null) throws Error {
@@ -646,46 +644,35 @@ namespace ClankerSaddle.Services {
                 return;
             }
             int64 held_task_at_entry = ((!)saddle).current_task_id;
-            if (((!)saddle).current_task_id > 0) {
-                int64 task_id = ((!)saddle).current_task_id;
+            int64 held_task = 0;
+            bool release_held = false;
+            if (held_task_at_entry > 0) {
+                held_task = held_task_at_entry;
                 bool keep_parked = false;
                 var task = yield db.query<TaskEntity>()
-                    .where(expr("id == $0", elem<int64?>(task_id)))
+                    .where(expr("id == $0", elem<int64?>(held_task)))
                     .first_async();
                 if (task != null && (((!)task).status == STATUS_ASSIGNED
                         || ((!)task).status == STATUS_WORKING || ((!)task).status == STATUS_BLOCKED
                         || ((!)task).status == STATUS_REVIEW_REQUESTED)) {
-                    keep_parked = yield tasks.newest_block_answered(task_id);
+                    keep_parked = yield tasks.newest_block_answered(held_task);
                     if (keep_parked) {
                         // Parked-with-answered: the verdict must survive
                         // for the resume (answer delivery, or the
                         // affinity allocation when the daemon asks
                         // again) — the session stays open and the task
                         // stays parked.
-                        print(@"[ClankerSaddle] Task $(task_id) keeps its parked session: its answered block awaits delivery\n");
+                        print(@"[ClankerSaddle] Task $(held_task) keeps its parked session: its answered block awaits delivery\n");
                     } else {
                         ((!)task).status = STATUS_QUEUED;
                         yield db.update_async<TaskEntity>((!)task);
                         string unreachable_text = reason != null && reason.length > 0
                             ? @"Saddle became unreachable ($(reason)); task requeued"
                             : "Saddle became unreachable; task requeued";
-                        yield tasks.add_system_comment(task_id, unreachable_text);
-                    }
-                }
-
-                if (!keep_parked) {
-                    var sessions = yield db.query<ClankerSaddle.Data.Session>()
-                        .where(expr("task_id == $0", elem<int64?>(task_id)))
-                        .materialise_async();
-                    foreach (var session in sessions) {
-                        if (session.ended_at != null) {
-                            continue;
-                        }
-                        session.ended_at = new DateTime.now_local();
-                        session.outcome = "stopped";
-                        yield db.update_async<ClankerSaddle.Data.Session>(session);
+                        yield tasks.add_system_comment(held_task, unreachable_text);
                     }
                 }
+                release_held = !keep_parked;
             }
             var offline_write = connection.create_command(
                 "UPDATE saddles SET status = 'offline', current_task_id = 0, allocated_at = NULL"
@@ -697,6 +684,16 @@ namespace ClankerSaddle.Services {
             // An offline saddle is neither sleeping nor wake-awaiting;
             // a restarted daemon re-establishes both by asking.
             poll_state.clear_ask(id);
+            if (release_held) {
+                // The held task's session close runs through the ONE
+                // release owner. Its idle-free no-ops against THIS row
+                // (the guarded offline write above already cleared
+                // current_task_id under the entry-seen guard); the
+                // close applies whatever the task's state was —
+                // flipped-and-requeued above, parked-suspended left
+                // alone, or a row that no longer exists.
+                yield tasks.release_task(held_task, "stopped", false);
+            }
             yield pulse.bump(0, "saddle unreachable");
         }
 

+ 55 - 36
src/Services/TaskService.vala

@@ -1158,8 +1158,7 @@ namespace ClankerSaddle.Services {
 
             yield mark_terminal((!)task, STATUS_DONE, summary, working_branch);
 
-            yield close_open_sessions(task_id, outcome);
-            yield free_saddles_running(task_id);
+            yield release_task(task_id, outcome, false);
             yield pulse.bump(task_id, "task completed");
             // Web Push (plan 11 task E): completion notifies the users
             // holding a "Notify me" pref for this task.
@@ -1196,8 +1195,7 @@ namespace ClankerSaddle.Services {
 
             yield mark_terminal((!)task, STATUS_FAILED, null, null, "Cancelled by user");
 
-            yield close_open_sessions(task_id, "stopped");
-            yield free_saddles_running(task_id);
+            yield release_task(task_id, "stopped", false);
             yield pulse.bump(task_id, "task cancelled");
         }
 
@@ -1229,8 +1227,7 @@ namespace ClankerSaddle.Services {
             yield mark_terminal((!)task, STATUS_FAILED, summary, null,
                 "Saddle reported a failure");
 
-            yield close_open_sessions(task_id, "failed");
-            yield free_saddles_running(task_id);
+            yield release_task(task_id, "failed", false);
             yield pulse.bump(task_id, "task failed");
             // Web Push (plan 11 task E): an agent-reported failure
             // notifies the opted-in users (a human cancellation does
@@ -1364,27 +1361,15 @@ namespace ClankerSaddle.Services {
         /**
          * Returns a non-terminal task to the queue: closes its open
          * sessions (outcome 'stopped') and frees the saddle that was
-         * running it. Plan 14 B1: a parked (suspended) task requeues
-         * too — the goal page's requeue-to-resume reaches a goal
-         * agent parked on a dead saddle; the suspension stamp clears
-         * so the next dispatch starts a fresh parking window.
+         * running it — {@link release_task} with the requeue, the
+         * single owner of the bookkeeping. Plan 14 B1: a parked
+         * (suspended) task requeues too — the goal page's
+         * requeue-to-resume reaches a goal agent parked on a dead
+         * saddle; the suspension stamp clears so the next dispatch
+         * starts a fresh parking window.
          */
         public async void requeue_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 && (((!)task).status == STATUS_ASSIGNED
-                    || ((!)task).status == STATUS_WORKING || ((!)task).status == STATUS_BLOCKED
-                    || ((!)task).status == STATUS_REVIEW_REQUESTED
-                    || ((!)task).status == STATUS_SUSPENDED)) {
-                ((!)task).status = STATUS_QUEUED;
-                if (((!)task).suspended_at != null) {
-                    ((!)task).suspended_at = null;
-                }
-                yield db.update_async<TaskEntity>((!)task);
-            }
-            yield close_open_sessions(task_id, "stopped");
-            yield free_saddles_running(task_id);
+            yield release_task(task_id, "stopped", true);
             yield pulse.bump(task_id, "task requeued");
         }
 
@@ -1396,8 +1381,9 @@ namespace ClankerSaddle.Services {
          * its card; the next attempt writes a fresh one), so no
          * summary→comment duplicate joins the thread. `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".
+         * the same bookkeeping as {@link requeue_task} (retry's flip
+         * is unconditional: the attempt resets whatever it held),
+         * plus the audit comment "Task retried by user".
          */
         public async void retry_task(int64 task_id) throws Error {
             var task = yield db.query<TaskEntity>()
@@ -1413,8 +1399,7 @@ namespace ClankerSaddle.Services {
             ((!)task).completed_at = null;
             yield db.update_async<TaskEntity>((!)task);
 
-            yield close_open_sessions(task_id, "stopped");
-            yield free_saddles_running(task_id);
+            yield release_task(task_id, "stopped", false);
             yield add_system_comment(task_id, "Task retried by user");
             yield pulse.bump(task_id, "task retried");
         }
@@ -1540,7 +1525,47 @@ namespace ClankerSaddle.Services {
             return null;
         }
 
-        private async void close_open_sessions(int64 task_id, string outcome) throws Error {
+        /**
+         * The ONE release path (plan 16 R2, arch item 13): every path
+         * that takes a task off its saddle runs through here. Closes
+         * the task's open sessions with `outcome` and frees whatever
+         * saddle row still runs it — one conditional UPDATE, atomic
+         * against a concurrent ask allocation claiming a new task for
+         * the saddle between a read and a full-row write. `requeue`
+         * first returns a non-terminal task to the queue
+         * (assigned/working/blocked/review_requested/suspended — the
+         * suspension stamp withdraws so the next dispatch opens a
+         * fresh parking window).
+         *
+         * The terminal callers (complete/cancel/fail) pass their own
+         * outcome with the status already stamped; requeue_task and
+         * the unregister path ride the requeue; retry_task does its
+         * own unconditional flip (summary and completed_at die with
+         * the attempt) and releases without one. The offline variant
+         * ({@link SaddleService.mark_saddle_unreachable}) keeps its
+         * own guarded OFFLINE write for the saddle row and calls this
+         * after it — the idle-free below no-ops against that row (its
+         * current_task_id already cleared under the entry-seen guard)
+         * while the session close applies; its keep-parked gate (an
+         * answered block awaiting delivery) skips the release
+         * entirely.
+         */
+        public async void release_task(int64 task_id, string outcome, bool requeue) throws Error {
+            if (requeue) {
+                var task = yield db.query<TaskEntity>()
+                    .where(expr("id == $0", elem<int64?>(task_id)))
+                    .first_async();
+                if (task != null && (((!)task).status == STATUS_ASSIGNED
+                        || ((!)task).status == STATUS_WORKING || ((!)task).status == STATUS_BLOCKED
+                        || ((!)task).status == STATUS_REVIEW_REQUESTED
+                        || ((!)task).status == STATUS_SUSPENDED)) {
+                    ((!)task).status = STATUS_QUEUED;
+                    if (((!)task).suspended_at != null) {
+                        ((!)task).suspended_at = null;
+                    }
+                    yield db.update_async<TaskEntity>((!)task);
+                }
+            }
             var sessions = yield db.query<Session>()
                 .where(expr("task_id == $0", elem<int64?>(task_id)))
                 .materialise_async();
@@ -1552,12 +1577,6 @@ namespace ClankerSaddle.Services {
                 session.outcome = outcome;
                 yield db.update_async<Session>(session);
             }
-        }
-
-        private async void free_saddles_running(int64 task_id) throws Error {
-            // One conditional UPDATE, atomic against a concurrent ask
-            // allocation claiming a new task for the saddle between a
-            // read and a full-row write.
             var command = connection.create_command(
                 "UPDATE saddles SET status = 'idle', current_task_id = 0, allocated_at = NULL"
                 + " WHERE current_task_id = :task");

+ 98 - 0
test/web_stack_test.py

@@ -4081,6 +4081,24 @@ def bundle_lifecycle(ctx):
     fake.hold_state = False  # release: the round ends, the daemon re-asks
     check(wait_task_done(auth, offline_task, timeout=60) is not None,
           "requeued task completes again on the healthy daemon")
+    # R2 (arch 13): the release owner's outcomes — the requeue closed
+    # the held attempt's session 'stopped'; the held daemon's terminal
+    # push then completes the task against that closed session (the
+    # outcome stands — a closed row never reopens). A task that
+    # completes a LIVE round closes its session 'done' (draft_dep
+    # above ran one open session to its terminal push).
+    conn = db()
+    outcome_rows = conn.execute(
+        "select outcome from sessions where task_id = ? order by id",
+        (offline_task,)).fetchall()
+    dep_outcome = conn.execute(
+        "select outcome from sessions where task_id = ? order by id desc",
+        (draft_dep,)).fetchone()
+    conn.close()
+    check(outcome_rows and outcome_rows[0][0] == "stopped",
+          "the one release path closes the requeued attempt 'stopped'")
+    check(dep_outcome is not None and dep_outcome[0] == "done",
+          "the terminal push closes a live round's session 'done'")
 
     print("== R6 F1: a hanging request cannot freeze the tick ==")
     # One wedged /task-update (never responds) blocks the comment/
@@ -4435,6 +4453,86 @@ def bundle_lifecycle(ctx):
     check(wait_task_done(auth, live_task, timeout=60) is not None,
           "live-block task completes after answering")
 
+    print("== R2: the one release path behind unregister ==")
+    # Arch 13: removing a saddle releases its held task through the
+    # same owner as every requeue — never an orphan open session on a
+    # deleted row. Route 1: a DEAD host — the abort delivery fails
+    # fast, the offline bookkeeping releases (queued + comment +
+    # session 'stopped'), the unregister then deletes the row.
+    saddles_state = slot_public(auth, "/saddles", "saddles")
+    act(auth, saddles_state["register"], {"name": "dead-host",
+                                          "host": "127.0.0.1:1"})
+    dead_card = [s for s in slot_public(auth, "/saddles", "saddles")["saddles"]
+                 if s["name"] == "dead-host"][0]
+    dead_id = int(dead_card["id"])
+    task_id_tmp = create_queued(auth, {
+        "project_id": "1",
+        "title": "Dead-host unregister task",
+        "priority": "2",
+    })
+    dead_task = task_id_tmp
+    conn = db()
+    conn.execute("update tasks set status = 'working' where id = ?",
+                 (dead_task,))
+    conn.execute("insert into sessions (task_id, saddle_id, started_at)"
+                 " values (?, ?, strftime('%s','now'))", (dead_task, dead_id))
+    conn.execute("update saddles set current_task_id = ? where id = ?",
+                 (dead_task, dead_id))
+    conn.commit()
+    dead_session = conn.execute(
+        "select id from sessions where task_id = ? and ended_at is null",
+        (dead_task,)).fetchone()[0]
+    conn.close()
+    act(auth, dead_card["unregister"], {})
+    conn = db()
+    dead_row = conn.execute("select status from tasks where id = ?",
+                            (dead_task,)).fetchone()
+    dead_session_row = conn.execute(
+        "select outcome, ended_at is not null from sessions where id = ?",
+        (dead_session,)).fetchone()
+    dead_saddle_count = conn.execute(
+        "select count(*) from saddles where id = ?", (dead_id,)).fetchone()[0]
+    conn.close()
+    check(dead_row is not None and dead_row[0] == "queued",
+          "unregister of a dead-host saddle releases the held task to the queue")
+    check(dead_session_row is not None and dead_session_row[0] == "stopped"
+          and dead_session_row[1],
+          "the offline release closes the held task's session 'stopped'")
+    check(dead_saddle_count == 0, "the dead-host saddle row is gone")
+
+    # Route 2: no open session on the saddle — the abort is a no-op
+    # and the unregister itself carries the release (the requeue flip
+    # through the one owner, the row deleted after).
+    saddles_state = slot_public(auth, "/saddles", "saddles")
+    act(auth, saddles_state["register"], {"name": "quiet-host",
+                                          "host": "127.0.0.1:%d" % fake.port})
+    quiet_card = [s for s in slot_public(auth, "/saddles", "saddles")["saddles"]
+                  if s["name"] == "quiet-host"][0]
+    quiet_id = int(quiet_card["id"])
+    task_id_tmp = create_queued(auth, {
+        "project_id": "1",
+        "title": "Quiet-host unregister task",
+        "priority": "2",
+    })
+    quiet_task = task_id_tmp
+    conn = db()
+    conn.execute("update tasks set status = 'working' where id = ?",
+                 (quiet_task,))
+    conn.execute("update saddles set current_task_id = ? where id = ?",
+                 (quiet_task, quiet_id))
+    conn.commit()
+    conn.close()
+    act(auth, quiet_card["unregister"], {})
+    conn = db()
+    quiet_row = conn.execute("select status from tasks where id = ?",
+                             (quiet_task,)).fetchone()
+    quiet_saddle_count = conn.execute(
+        "select count(*) from saddles where id = ?", (quiet_id,)).fetchone()[0]
+    conn.close()
+    check(quiet_row is not None and quiet_row[0] == "queued",
+          "unregister's own release returns the held task to the queue")
+    check(quiet_saddle_count == 0, "the quiet-host saddle row is gone")
+
 
 # =============================================================================
 # Bundle: answers — the answer-resume machinery in both daemon shapes