Explorar o código

qa: terminal states are atomic-sticky — set_status and add_block's park flip guard the transition in the UPDATE's WHERE (a late 'working' push interleaving a completion on the main loop used to resurrect the task as working AFTER its session closed done — the parked-resume race, plan-15 C deviation 4; the callers' terminal checks have already yielded by write time). Harness: the dispatch rules-order fixture attaches rules while DRAFT then enqueues (the 1s dispatch tick could beat the attaches under load), and the app-ready probe requires OUR process alive — a free_port() TOCTOU handed the port to another shard's app whose identical seed answered the probe while ours died at MHD bind

clanker hai 1 mes
pai
achega
4ad0c84cc0
Modificáronse 2 ficheiros con 61 adicións e 10 borrados
  1. 46 7
      src/Services/TaskService.vala
  2. 15 3
      test/web_stack_test.py

+ 46 - 7
src/Services/TaskService.vala

@@ -424,11 +424,40 @@ namespace ClankerSaddle.Services {
                 return null;
             }
             if (((!)task).status != status) {
-                ((!)task).status = status;
-                if (status == STATUS_DONE && ((!)task).completed_at == null) {
-                    ((!)task).completed_at = new DateTime.now_local();
+                if (status != STATUS_DONE && status != STATUS_FAILED) {
+                    // Terminal states are sticky (plan 13 B2's
+                    // invariant): a NON-terminal transition must never
+                    // downgrade a done/failed task. The entity writes
+                    // below interleave with completions on the main
+                    // loop (the terminal checks in the callers have
+                    // already yielded by then — a late 'working' push
+                    // racing a 'done' push used to resurrect the task
+                    // as working AFTER its session closed done), so
+                    // the guard lives in the UPDATE's WHERE: atomic.
+                    // The row is re-read afterwards so the caller sees
+                    // the truth (terminal included), never a stale
+                    // entity it might write back.
+                    var guard = connection.create_command(
+                        "UPDATE tasks SET status = :status WHERE id = :id"
+                        + " AND status NOT IN ('done', 'failed')");
+                    guard.with_parameter<Invercargill.Element>("status",
+                        elem(status));
+                    guard.with_parameter<Invercargill.Element>("id",
+                        elem<int64?>(id));
+                    yield guard.execute_non_query_async();
+                    task = yield db.query<TaskEntity>()
+                        .where(expr("id == $0", elem<int64?>(id)))
+                        .first_async();
+                    if (task == null) {
+                        return null;
+                    }
+                } else {
+                    ((!)task).status = status;
+                    if (status == STATUS_DONE && ((!)task).completed_at == null) {
+                        ((!)task).completed_at = new DateTime.now_local();
+                    }
+                    yield db.update_async<TaskEntity>((!)task);
                 }
-                yield db.update_async<TaskEntity>(task);
                 yield pulse.bump(id, @"task $(status)");
             }
             return task;
@@ -853,9 +882,19 @@ namespace ClankerSaddle.Services {
                 .first_async();
             if (task != null && (((!)task).status == STATUS_ASSIGNED || ((!)task).status == STATUS_WORKING
                     || ((!)task).status == STATUS_BLOCKED || ((!)task).status == STATUS_REVIEW_REQUESTED)) {
-                ((!)task).status = block.block_type == BLOCK_REVIEW
-                    ? STATUS_REVIEW_REQUESTED : STATUS_BLOCKED;
-                yield db.update_async<TaskEntity>((!)task);
+                // The park flip is a guarded update: a completion
+                // interleaving the raise keeps its terminal state (the
+                // read above already yielded — the entity is stale by
+                // write time under load; plan 13 B2's sticky rule).
+                var guard = connection.create_command(
+                    "UPDATE tasks SET status = :status WHERE id = :id"
+                    + " AND status IN ('assigned', 'working', 'blocked', 'review_requested')");
+                guard.with_parameter<Invercargill.Element>("status",
+                    elem(block.block_type == BLOCK_REVIEW
+                        ? STATUS_REVIEW_REQUESTED : STATUS_BLOCKED));
+                guard.with_parameter<Invercargill.Element>("id",
+                    elem<int64?>(task_id));
+                yield guard.execute_non_query_async();
             }
             yield pulse.bump(task_id, "block raised");
             // Web Push (plan 11 task E): every raised block notifies all

+ 15 - 3
test/web_stack_test.py

@@ -988,7 +988,13 @@ def start_app():
                    "-ex", "run", "-ex", "bt", "--args", APP, str(APP_PORT)]
     process = subprocess.Popen(command, cwd=WORK, env=env,
                                stdout=log, stderr=subprocess.STDOUT)
-    outcome = wait_for(lambda: (app_ready() and "ready")
+    # The ready probe must require OUR process alive: a free_port()
+    # TOCTOU can hand this port to another shard's app — ours then dies
+    # at bind (MHD error()) while the probe happily answers from the
+    # intruder (same admin seed), and every check would silently drive
+    # the WRONG app. poll() != None with a serving port is exactly that
+    # shape: take the exit branch, the start_bundle retry re-ports.
+    outcome = wait_for(lambda: (app_ready() and process.poll() is None and "ready")
                        or (process.poll() is not None and "app-exited-%d" % process.poll())
                        or False, timeout=30, what="app ready")
     if outcome is None:
@@ -2147,16 +2153,22 @@ def bundle_dispatch(ctx):
     act(auth, ref_project["attach_rule"], {"rule_id": str(rule_id("Reference rule"))})
 
     tasks_state = slot_public(auth, "/tasks", "tasks")
-    task_id_tmp = create_queued(auth, {
+    # Draft-first (deterministic rule attachment): the dispatcher claims
+    # a queued task on its 1s tick, which under load could beat the
+    # attach_rule calls below and ship an assignment without the task's
+    # own rules — attach while draft, enqueue last.
+    act(auth, tasks_state["create"], {
         "project_id": "1",
         "title": "Rules dispatch",
         "description": "Effective rule set rides the /start payload.",
         "priority": "2",
     })
-    rule_task = task_id_tmp
+    rule_task = int([t["id"] for t in slot_public(auth, "/tasks", "tasks")["tasks"]
+                     if t["title"] == "Rules dispatch"][0])
     detail = task_state(auth, rule_task)
     act(auth, detail["attach_rule"], {"rule_id": str(rule_id("Write Vala docs"))})
     act(auth, detail["attach_rule"], {"rule_id": str(rule_id("Use Invercargill"))})
+    enqueue_task(auth, rule_task)
     detail = task_state(auth, rule_task)
     check(sorted(r["name"] for r in detail["rules"]) == ["Use Invercargill", "Write Vala docs"],
           "task page lists its attached rules")