Browse Source

replay: idempotent terminal + narrative projections — once per entry id, ever

Terminal transitions now stop at terminal state everywhere they can
re-enter: TaskService.complete/fail/cancel keep the first outcome (the
honest burst of distinct failure entry ids dies at the first),
GoalService.fail_goal and submit_completion refuse finished goals, and
both push projections (apply_entry_events, apply_result_state) consume
redeliveries silently once the task or its goal is terminal. Combined
with the deduped entry stream the 115-event/187-comment fan-out is
structurally impossible.
clanker 1 month ago
parent
commit
00c1a6dd2e
4 changed files with 215 additions and 29 deletions
  1. 18 2
      src/Services/GoalService.vala
  2. 44 24
      src/Services/SaddleService.vala
  3. 21 3
      src/Services/TaskService.vala
  4. 132 0
      test/web_stack_test.py

+ 18 - 2
src/Services/GoalService.vala

@@ -433,10 +433,19 @@ namespace ClankerSaddle.Services {
          * 'completion_review', the report is stored (goal row +
          * goal_blocks type 'completion') and the goal-run task parks
          * on the mirrored block. The daemon enforces the
-         * branch-push rule before letting `done` through.
+         * branch-push rule before letting `done` through. Idempotent
+         * on terminal goals (plan 13 B2): a redelivered done push
+         * must not resurrect a finished goal.
          */
         public async GoalBlock submit_completion(int64 goal_id, string report) throws Error {
             var goal = yield require_goal(goal_id);
+            if (((!)goal).status == GOAL_COMPLETE || ((!)goal).status == GOAL_FAILED
+                    || ((!)goal).status == GOAL_CANCELLED) {
+                // Terminal stands: the pending-report idempotency below
+                // cannot apply (the verdict already landed).
+                throw new Error(Quark.from_string("clanker-saddle"), 85,
+                    @"The goal is already $(((!)goal).status) — its completion review is closed");
+            }
             // Idempotent on the pending report (see submit_plan).
             foreach (var existing in yield list_blocks(goal_id)) {
                 if (existing.block_type == GOAL_BLOCK_COMPLETION
@@ -468,10 +477,17 @@ namespace ClankerSaddle.Services {
          * The goal agent failed the goal (`saddle failure` in goal
          * context): status 'failed', the report stored, the goal-run
          * task failed (its session closes and saddle frees) and every
-         * user notified.
+         * user notified. Idempotent on terminal goals (plan 13 B2):
+         * a replayed failure push — redelivered OR a fresh entry id
+         * from the honest burst — re-runs no bookkeeping, so exactly
+         * one goal-failed event, one notification, ever.
          */
         public async void fail_goal(int64 goal_id, string? report) throws Error {
             var goal = yield require_goal(goal_id);
+            if (((!)goal).status == GOAL_COMPLETE || ((!)goal).status == GOAL_FAILED
+                    || ((!)goal).status == GOAL_CANCELLED) {
+                return;
+            }
             ((!)goal).status = GOAL_FAILED;
             ((!)goal).completed_at = new DateTime.now_local();
             ((!)goal).completion_report = report ?? ((!)goal).completion_report;

+ 44 - 24
src/Services/SaddleService.vala

@@ -1258,10 +1258,26 @@ namespace ClankerSaddle.Services {
          * done submits the completion report (the goal-run task parks
          * instead of completing), a failure fails the goal — and
          * comments also land on the goal's thread.
+         *
+         * Plan 13 B2: the projections key on the DEDUPED entry stream
+         * ({@link store_pushed_entries} claims every id once per
+         * saddle, ever) and stop at terminal state — a task already
+         * done/failed, a goal already finished, projects nothing
+         * further. The live failure's 115-event/187-comment fan-out is
+         * structurally impossible: replays never reach this function,
+         * and the honest burst of DISTINCT failure ids dies at the
+         * first one.
          */
         public async bool apply_entry_events(int64 task_id, Vector<Json.Object> entries,
                 TaskService tasks) throws Error {
             var goal_id = yield goal_id_of_run(task_id);
+            bool terminal = yield task_is_terminal(task_id);
+            if (!terminal && goal_id > 0) {
+                terminal = yield goal_is_terminal(goal_id);
+            }
+            if (terminal) {
+                return false;
+            }
             foreach (var entry in entries) {
                 string command = string_member(entry, "command");
                 string output = string_member(entry, "output");
@@ -1568,20 +1584,27 @@ namespace ClankerSaddle.Services {
          * parks the same way — for a goal agent the resume is driven
          * by the goal's task-terminal events; and `working` flips the
          * task to working (never downgrading a pending review).
-         * Terminal states are ignored once the task is already
-         * terminal — an outbox redelivery after the entries finished
-         * the task must not re-run completion bookkeeping.
+         *
+         * Plan 13 B2: a push against a task that is already terminal —
+         * or a goal-context push against a finished goal — is an
+         * outbox redelivery and is consumed silently, whatever state
+         * it carries; terminal transitions and their narrative fire
+         * once, ever.
          */
         public async void apply_result_state(Saddle saddle, ClankerSaddle.Data.Session session,
                 string state, Json.Object? block, string? summary, TaskService tasks) throws Error {
             int64 task_id = session.task_id;
             int64 goal_id = yield goal_id_of_run(task_id);
+            bool terminal = yield task_is_terminal(task_id);
+            if (!terminal && goal_id > 0) {
+                terminal = yield goal_is_terminal(goal_id);
+            }
+            if (terminal) {
+                return;
+            }
 
             switch (state) {
                 case "done":
-                    if (yield task_is_terminal(task_id)) {
-                        return;
-                    }
                     if (goal_id > 0) {
                         // Goal context: `saddle done` submits the
                         // completion report and enters completion
@@ -1596,9 +1619,6 @@ namespace ClankerSaddle.Services {
                     yield tasks.complete_task(task_id, summary, null, "done");
                     break;
                 case "failed":
-                    if (yield task_is_terminal(task_id)) {
-                        return;
-                    }
                     string explanation = summary ?? "";
                     if (goal_id > 0) {
                         yield goal_service.fail_goal(goal_id,
@@ -1619,9 +1639,7 @@ namespace ClankerSaddle.Services {
                         // the payload-carrying transition already did,
                         // and a synthesised fallback row would dupe it.
                         if (block == null) {
-                            if (!(yield task_is_terminal(task_id))) {
-                                yield tasks.set_status(task_id, STATUS_BLOCKED);
-                            }
+                            yield tasks.set_status(task_id, STATUS_BLOCKED);
                             yield park_if_current(saddle.id, task_id);
                             break;
                         }
@@ -1639,14 +1657,12 @@ namespace ClankerSaddle.Services {
                                     (!)block.has_member("options") ? "options" : "block_options"),
                                 optional_string_member((!)block, "body"));
                         }
-                        if (!(yield task_is_terminal(task_id))) {
-                            yield tasks.set_status(task_id, STATUS_BLOCKED);
-                        }
+                        yield tasks.set_status(task_id, STATUS_BLOCKED);
                         yield park_if_current(saddle.id, task_id);
                         break;
                     }
                     yield ensure_block(tasks, task_id, block != null ? (!)block : new Json.Object());
-                    if (!(yield task_is_terminal(task_id))) {
+                    {
                         // A review-kind block parks as review_requested
                         // (the banner state); every other kind as
                         // blocked.
@@ -1671,14 +1687,9 @@ namespace ClankerSaddle.Services {
                     yield park_if_current(saddle.id, task_id);
                     break;
                 case "working":
-                    // Never downgrade a pending review: the agent keeps
-                    // working after `saddle review` (it does not block),
-                    // but the banner must stay up until the human answers.
-                    // A terminal task stays terminal — a redelivered
-                    // working push must not resurrect finished work.
-                    if (yield task_is_terminal(task_id)) {
-                        return;
-                    }
+                    // A pending review keeps its banner: started_at (and
+                    // the first-start push) only stamp for work that
+                    // never allocated.
                     var current = yield tasks.set_status(task_id, STATUS_WORKING);
                     if (current != null && ((!)current).status != STATUS_REVIEW_REQUESTED
                             && ((!)current).started_at == null) {
@@ -1706,6 +1717,15 @@ namespace ClankerSaddle.Services {
                 || ((!)task).status == STATUS_FAILED);
         }
 
+        /** True when the goal's status is complete, failed or cancelled. */
+        private async bool goal_is_terminal(int64 goal_id) throws Error {
+            var goal = yield db.query<Goal>()
+                .where(expr("id == $0", elem<int64?>(goal_id)))
+                .first_async();
+            return goal != null && (((!)goal).status == GOAL_COMPLETE
+                || ((!)goal).status == GOAL_FAILED || ((!)goal).status == GOAL_CANCELLED);
+        }
+
         /**
          * The running-liveness probe (plan 10 phase 2): a busy saddle's
          * session has seen neither chunks nor results for the staleness

+ 21 - 3
src/Services/TaskService.vala

@@ -1071,7 +1071,11 @@ namespace ClankerSaddle.Services {
         /**
          * Marks a task done: records `summary` and `working_branch`, stamps
          * `completed_at`, closes any open session (outcome `outcome`) and
-         * frees the saddle that was running it.
+         * frees the saddle that was running it. Terminal transitions
+         * project ONCE (plan 13 B2): a task already done or failed keeps
+         * its first outcome — a redelivered push or a second terminal
+         * entry with a fresh id re-runs no bookkeeping, no comment, no
+         * event (retry_task is the one path that un-terminals).
          */
         public async void complete_task(int64 task_id, string? summary,
                 string? working_branch, string outcome) throws Error {
@@ -1082,6 +1086,9 @@ namespace ClankerSaddle.Services {
                 throw new Error(Quark.from_string("clanker-saddle"), 8,
                     @"No task exists with id $(task_id)");
             }
+            if (((!)task).status == STATUS_DONE || ((!)task).status == STATUS_FAILED) {
+                return;
+            }
 
             ((!)task).status = STATUS_DONE;
             ((!)task).completed_at = new DateTime.now_local();
@@ -1111,7 +1118,8 @@ namespace ClankerSaddle.Services {
          * Cancels a task: status 'failed' with a cancellation summary. Any
          * open session is closed with outcome 'stopped' and its saddle
          * freed. Draft tasks are refused — they are deleted, not
-         * cancelled (nothing ever ran).
+         * cancelled (nothing ever ran). Idempotent on terminal tasks
+         * (plan 13 B2): the first outcome stands.
          */
         public async void cancel_task(int64 task_id) throws Error {
             var task = yield db.query<TaskEntity>()
@@ -1121,6 +1129,9 @@ namespace ClankerSaddle.Services {
                 throw new Error(Quark.from_string("clanker-saddle"), 9,
                     @"No task exists with id $(task_id)");
             }
+            if (((!)task).status == STATUS_DONE || ((!)task).status == STATUS_FAILED) {
+                return;
+            }
             if (((!)task).status == STATUS_DRAFT) {
                 throw new Error(Quark.from_string("clanker-saddle"), 55,
                     "Draft tasks are deleted, not cancelled");
@@ -1143,7 +1154,11 @@ namespace ClankerSaddle.Services {
          * Records a saddle-reported failure: status 'failed' with the
          * daemon's message as summary, the explanation stored again as a
          * agent comment (so the thread carries it), the open session
-         * closed with outcome 'failed' and its saddle freed.
+         * closed with outcome 'failed' and its saddle freed. Terminal
+         * transitions project ONCE (plan 13 B2): the live failure's
+         * honest burst — a dozen-plus distinct `saddle failure` entry
+         * ids in one session — still fails the task exactly once, with
+         * one comment and one goal event.
          */
         public async void fail_task(int64 task_id, string? summary) throws Error {
             var task = yield db.query<TaskEntity>()
@@ -1153,6 +1168,9 @@ namespace ClankerSaddle.Services {
                 throw new Error(Quark.from_string("clanker-saddle"), 25,
                     @"No task exists with id $(task_id)");
             }
+            if (((!)task).status == STATUS_DONE || ((!)task).status == STATUS_FAILED) {
+                return;
+            }
 
             ((!)task).status = STATUS_FAILED;
             ((!)task).completed_at = new DateTime.now_local();

+ 132 - 0
test/web_stack_test.py

@@ -5943,6 +5943,10 @@ def bundle_replay(ctx):
     auth = login("admin", "adminpass123")
     ctx.app = app  # teardown tracks the restarted process
     ctx.auth = auth
+    # The restart wiped the in-memory ask bookkeeping; the daemon
+    # re-establishes sleeping with a fresh ask (the real daemon's
+    # idle-awake ask / 15-minute safety self-ask).
+    fake.ask_now()
     status, _ = saddle_api("POST", "/api/saddle/results", fake_token,
                            {"session_id": churn_session, "state": "done",
                             "entries": burst + fresh})
@@ -5950,6 +5954,134 @@ def bundle_replay(ctx):
           and rows_for(src_session) == stored_once,
           "a post-restart redelivery still stores nothing (dedup is persistent)")
 
+    print("== plan 13 B2: duplicate failed entries project once (task) ==")
+    # The post-mortem's honest burst: the same failure entry redelivered
+    # under a churned session AND a second, DISTINCT failure entry id
+    # in the same session — one transition, one comment, ever.
+    def agent_comments(task_id, body=None):
+        conn = db()
+        try:
+            if body is None:
+                return conn.execute(
+                    "select count(*) from task_comments"
+                    " where task_id = ? and author = 'agent'",
+                    (task_id,)).fetchone()[0]
+            return conn.execute(
+                "select count(*) from task_comments"
+                " where task_id = ? and author = 'agent' and body = ?",
+                (task_id, body)).fetchone()[0]
+        finally:
+            conn.close()
+
+    conn = db()
+    conn.execute(
+        "insert into tasks (project_id, title, status, priority, created_at)"
+        " values (1, 'Burst failure task', 'working', 5, strftime('%s','now'))")
+    conn.commit()
+    burst_task = conn.execute(
+        "select max(id) from tasks where title = 'Burst failure task'").fetchone()[0]
+    conn.close()
+    burst_session = open_session_on_fake(burst_task)
+    first_fail = [{"id": 6001, "command": "saddle failure",
+                   "output": "bash -n rejected the script", "exit_code": 1,
+                   "type": "failed", "timestamp": "2026-09-11T00:01:00Z"}]
+    status, _ = saddle_api("POST", "/api/saddle/results", fake_token,
+                           {"session_id": burst_session, "state": "failed",
+                            "entries": first_fail,
+                            "summary": "bash -n rejected the script"})
+    check(status == 200
+          and task_state(auth, burst_task)["status"] == "failed",
+          "the failure entry + state pair fails the task")
+    check(agent_comments(burst_task, "bash -n rejected the script") == 1,
+          "the entry path and the state path together leave one failure comment")
+
+    # The burst: a DISTINCT failure entry id in the same session, plus
+    # both ids redelivered under a churned session — nothing re-runs.
+    second_fail = [{"id": 6002, "command": "saddle failure",
+                    "output": "bash -n rejected the script", "exit_code": 1,
+                    "type": "failed", "timestamp": "2026-09-11T00:01:01Z"}]
+    churned_burst = open_session_on_fake(burst_task)
+    for target in (burst_session, churned_burst):
+        status, _ = saddle_api("POST", "/api/saddle/results", fake_token,
+                               {"session_id": target, "state": "failed",
+                                "entries": first_fail + second_fail,
+                                "summary": "bash -n rejected the script"})
+        check(status == 200,
+              "the burst push under %s is consumed, not rejected" % (
+                  "its session" if target == burst_session else "a churned session"))
+    check(task_state(auth, burst_task)["status"] == "failed"
+          and agent_comments(burst_task) == 1,
+          "duplicate failed entries leave one transition and one comment")
+
+    print("== plan 13 B2: goal failure projects once ==")
+    # Goal context: `saddle failure` fails the goal ONCE — the 115
+    # goal-failed events of the live failure are structurally gone.
+    def goal_state(goal_id):
+        return slot_public(auth, "/goals/%d" % goal_id, "goal")
+
+    def goal_failed_events(goal_id):
+        conn = db()
+        try:
+            return conn.execute(
+                "select count(*) from goal_events"
+                " where goal_id = ? and kind = 'goal-failed'",
+                (goal_id,)).fetchone()[0]
+        finally:
+            conn.close()
+
+    act(auth, slot_public(auth, "/goals", "goals")["create"], {
+        "project_id": "1", "title": "Burst goal", "brief": "Fails honestly.",
+        "branch_name": "goal/burst"})
+    conn = db()
+    burst_goal = conn.execute(
+        "select id from goals where title = 'Burst goal'").fetchone()[0]
+    conn.close()
+    fake.goal_script = [("suspend",)]
+    act(auth, goal_state(burst_goal)["start"], {})
+    conn = db()
+    run_task = conn.execute(
+        "select id from tasks where goal_id = ? and kind = 'goal_run'",
+        (burst_goal,)).fetchone()[0]
+    conn.close()
+    check(wait_for(lambda: task_state(auth, run_task)["status"] == "suspended"
+                   or False, timeout=60, what="burst goal agent parks") is not None,
+          "the burst goal's agent parks first")
+    conn = db()
+    run_session = conn.execute(
+        "select id from sessions where task_id = ? and ended_at is null",
+        (run_task,)).fetchone()[0]
+    conn.close()
+
+    goal_fail = [{"id": 7001, "command": "saddle failure",
+                  "output": "the harness collapsed", "exit_code": 1,
+                  "type": "failed", "timestamp": "2026-09-11T00:02:00Z"},
+                 {"id": 7002, "command": "saddle failure",
+                  "output": "the harness collapsed", "exit_code": 1,
+                  "type": "failed", "timestamp": "2026-09-11T00:02:01Z"}]
+    status, _ = saddle_api("POST", "/api/saddle/results", fake_token,
+                           {"session_id": run_session, "state": "failed",
+                            "entries": goal_fail, "summary": "the harness collapsed"})
+    check(status == 200 and goal_state(burst_goal)["status"] == "failed",
+          "the goal agent's failure fails the goal")
+    check(goal_failed_events(burst_goal) == 1,
+          "a two-entry failure burst records exactly one goal-failed event")
+
+    # Redeliveries — same ids under the run's session AND under a
+    # churned session — plus the bare failed state again: still one.
+    churned_run = open_session_on_fake(run_task)
+    for target in (run_session, churned_run):
+        status, _ = saddle_api("POST", "/api/saddle/results", fake_token,
+                               {"session_id": target, "state": "failed",
+                                "entries": goal_fail,
+                                "summary": "the harness collapsed"})
+        check(status == 200, "the goal-failure redelivery under %s is consumed"
+              % ("its session" if target == run_session else "a churned session"))
+    check(goal_state(burst_goal)["status"] == "failed"
+          and goal_failed_events(burst_goal) == 1,
+          "redelivered failures never re-run the goal failure (one event, ever)")
+    check(task_state(auth, run_task)["status"] == "failed",
+          "the goal-run task stays failed after the replays")
+
     # Drain: nothing queued remains for the shard's shutdown.
     check(queued_count() == 0, "the replay shard drains its queue")