Răsfoiți Sursa

feat: subtask auto-approve — tasks.auto_approve toggle above the subtasks table covers descendants transitively; covered review blocks auto-answered ('auto' recorded, timeline entry, delivered like a human verdict) while questions/RFCs and the parent's own review stay human-gated; all-terminal descendants resume with the push-first final-review instruction

clanker 1 lună în urmă
părinte
comite
e561f584c3

+ 1 - 0
meson.build

@@ -113,6 +113,7 @@ app_sources = files(
     'src/Data/Migrations/M0005_RawLog.vala',
     'src/Data/Migrations/M0006_Capabilities.vala',
     'src/Data/Migrations/M0007_SuspendedAt.vala',
+    'src/Data/Migrations/M0008_AutoApprove.vala',
     'src/Services/ProjectService.vala',
     'src/Services/TaskService.vala',
     'src/Services/SaddleService.vala',

+ 2 - 1
src/Api/SaddleApi.vala

@@ -129,8 +129,9 @@ namespace ClankerSaddle.Api {
 
                 case "review":
                     string review_summary = get_string_or(data, "summary", "");
-                    yield tasks.request_review(task_id, review_summary);
+                    var review_block = yield tasks.request_review(task_id, review_summary);
                     yield tasks.log_session_entry(task_id, "review", "saddle review", review_summary);
+                    yield tasks.auto_answer_if_covered(review_block);
                     break;
 
                 case "done":

+ 5 - 1
src/Data/Entities.vala

@@ -72,6 +72,7 @@ namespace ClankerSaddle.Data {
         public DateTime? started_at { get; set; default = null; }
         public DateTime? completed_at { get; set; default = null; }
         public DateTime? suspended_at { get; set; default = null; }
+        public int64 auto_approve { get; set; default = 0; }
 
         public static void entity_mapping(EntityMapperBuilder<TaskEntity> cfg) {
             cfg.table("tasks")
@@ -91,7 +92,8 @@ namespace ClankerSaddle.Data {
                 .column<DateTime?>("created_at", o => o.created_at, (o, v) => o.created_at = v)
                 .column<DateTime?>("started_at", o => o.started_at, (o, v) => o.started_at = v)
                 .column<DateTime?>("completed_at", o => o.completed_at, (o, v) => o.completed_at = v)
-                .column<DateTime?>("suspended_at", o => o.suspended_at, (o, v) => o.suspended_at = v);
+                .column<DateTime?>("suspended_at", o => o.suspended_at, (o, v) => o.suspended_at = v)
+                .column<int64?>("auto_approve", o => o.auto_approve, (o, v) => o.auto_approve = v ?? 0);
         }
     }
 
@@ -168,6 +170,7 @@ namespace ClankerSaddle.Data {
         public string? options { get; set; default = null; }
         public string? body { get; set; default = null; }
         public DateTime? answered_at { get; set; default = null; }
+        public string? answered_by { get; set; default = null; }
         public DateTime? created_at { get; set; default = null; }
 
         public static void entity_mapping(EntityMapperBuilder<TaskBlock> cfg) {
@@ -180,6 +183,7 @@ namespace ClankerSaddle.Data {
                 .column<string>("options", o => o.options, (o, v) => o.options = v)
                 .column<string>("body", o => o.body, (o, v) => o.body = v)
                 .column<DateTime?>("answered_at", o => o.answered_at, (o, v) => o.answered_at = v)
+                .column<string>("answered_by", o => o.answered_by, (o, v) => o.answered_by = v)
                 .column<DateTime?>("created_at", o => o.created_at, (o, v) => o.created_at = v);
         }
     }

+ 32 - 0
src/Data/Migrations/M0008_AutoApprove.vala

@@ -0,0 +1,32 @@
+using InvercargillSql.Migrations;
+
+namespace ClankerSaddle.Data.Migrations {
+
+    /**
+     * Web-stack auto-approve: `tasks.auto_approve` marks a parent whose
+     * descendants' review blocks the web answers automatically (the
+     * toggle covers descendants only — the parent's own final review is
+     * always a human gate). `task_blocks.answered_by` records who
+     * answered a block ('auto' for machine approvals; null for humans)
+     * so the timeline can tell them apart.
+     */
+    public class M0008_AutoApprove : Migration {
+
+        public override string name { get { return "Auto-approve: tasks.auto_approve, task_blocks.answered_by"; } }
+        public override uint64 serial { get { return 8; } }
+        public override string migration_namespace { get { return "clanker-saddle"; } }
+
+        public override void up(MigrationBuilder b) {
+            b.alter_table("tasks", (t) => {
+                t.add_column<int64?>("auto_approve");
+            });
+            b.alter_table("task_blocks", (t) => {
+                t.add_column<string>("answered_by");
+            });
+        }
+
+        public override void down(MigrationBuilder b) {
+            // Nullable additive columns; leaving them behind is harmless.
+        }
+    }
+}

+ 5 - 2
src/Services/SaddleService.vala

@@ -1210,8 +1210,10 @@ namespace ClankerSaddle.Services {
                         }
                         break;
                     case "review":
-                        yield tasks.request_review(task_id, output.length > 0 ? output
+                        var review_block = yield tasks.request_review(task_id,
+                            output.length > 0 ? output
                             : strip_quotes(strip_verb(command, "saddle review")));
+                        yield tasks.auto_answer_if_covered(review_block);
                         break;
                     case "done":
                         yield tasks.complete_task(task_id,
@@ -1251,10 +1253,11 @@ namespace ClankerSaddle.Services {
                     return;
                 }
             }
-            yield tasks.add_block(task_id, question,
+            var raised = yield tasks.add_block(task_id, question,
                 string_member(status, "block_type"),
                 string_array_member(status, "block_options"),
                 optional_string_member(status, "block_body"));
+            yield tasks.auto_answer_if_covered(raised);
         }
 
         /**

+ 79 - 1
src/Services/TaskService.vala

@@ -508,7 +508,8 @@ namespace ClankerSaddle.Services {
          * the paused workspace and hands the answer over; without one the
          * task flips straight back to 'working'.
          */
-        public async void answer_block(int64 block_id, string answer) throws Error {
+        public async void answer_block(int64 block_id, string answer,
+                string? answered_by = null) throws Error {
             var block = yield db.query<TaskBlock>()
                 .where(expr("id == $0", elem<int64?>(block_id)))
                 .first_async();
@@ -519,6 +520,7 @@ namespace ClankerSaddle.Services {
 
             ((!)block).answer = answer;
             ((!)block).answered_at = new DateTime.now_local();
+            ((!)block).answered_by = answered_by;
             yield db.update_async<TaskBlock>((!)block);
 
             var task = yield db.query<TaskEntity>()
@@ -633,6 +635,75 @@ namespace ClankerSaddle.Services {
             yield pulse.bump(task_id, "dependency added");
         }
 
+        /** Persists the auto-approve toggle for `task_id`. */
+        public async void save_auto_approve(int64 task_id, bool enabled) throws Error {
+            var task = yield db.query<TaskEntity>()
+                .where(expr("id == $0", elem<int64?>(task_id)))
+                .first_async();
+            if (task == null) {
+                return;
+            }
+            ((!)task).auto_approve = enabled ? 1 : 0;
+            yield db.update_async<TaskEntity>((!)task);
+            yield pulse.bump(task_id, "auto-approve toggled");
+        }
+
+        /**
+         * True when any STRICT ancestor of `task_id` carries the
+         * auto-approve toggle — the task's review blocks are then
+         * answered automatically (transitive to grandchildren; a
+         * top-level task has no ancestors and is never covered, so the
+         * parent's own final review stays a human gate).
+         */
+        public async bool is_auto_approve_covered(int64 task_id) throws Error {
+            int64 current = task_id;
+            while (current > 0) {
+                var row = yield db.query<TaskEntity>()
+                    .where(expr("id == $0", elem<int64?>(current)))
+                    .first_async();
+                if (row == null) {
+                    break;
+                }
+                int64 ancestor = ((!)row).parent_task_id;
+                if (ancestor <= 0) {
+                    break;
+                }
+                var ancestor_row = yield db.query<TaskEntity>()
+                    .where(expr("id == $0", elem<int64?>(ancestor)))
+                    .first_async();
+                if (ancestor_row == null) {
+                    break;
+                }
+                if (((!)ancestor_row).auto_approve != 0) {
+                    return true;
+                }
+                current = ancestor;
+            }
+            return false;
+        }
+
+        /**
+         * Auto-answers a just-raised review block when its task is
+         * covered by an ancestor's auto-approve toggle: answer text
+         * `Auto-approved (subtask auto-approve is on)`, answered-by
+         * 'auto', a timeline entry on the session — and the verdict
+         * reaches the daemon through the same delivery path as a human
+         * answer. Questions and RFC blocks are never touched; returns
+         * true when the block was auto-answered.
+         */
+        public async bool auto_answer_if_covered(TaskBlock block) throws Error {
+            if (block.block_type != BLOCK_REVIEW || block.answered_at != null) {
+                return false;
+            }
+            if (!(yield is_auto_approve_covered(block.task_id))) {
+                return false;
+            }
+            yield answer_block(block.id, "Auto-approved (subtask auto-approve is on)", "auto");
+            yield log_session_entry(block.task_id, "auto-approve",
+                "review auto-approved", block.body ?? block.question);
+            return true;
+        }
+
         /** Creates a child task under `parent_task_id` (status 'queued'). */
         public async TaskEntity create_subtask(int64 parent_task_id, string title,
                 string? description) throws Error {
@@ -864,6 +935,7 @@ namespace ClankerSaddle.Services {
         public async string subtask_status_report(int64 parent_id) throws Error {
             var report = new StringBuilder();
             report.append("Subtask update — resuming your suspended session:\n\n");
+            bool all_terminal = true;
             foreach (var child in yield db.query<TaskEntity>()
                     .where(expr("parent_task_id == $0", elem<int64?>(parent_id)))
                     .order_by(expr("id"))
@@ -877,8 +949,14 @@ namespace ClankerSaddle.Services {
                         child.summary ?? "no summary reported");
                 } else {
                     report.append_printf("- #%lld %s — %s\n", child.id, child.title, child.status);
+                    all_terminal = false;
                 }
             }
+            if (all_terminal) {
+                report.append("\nAll subtasks are complete. Verify the integration, then request"
+                    + " ONE final review of the whole change with `saddle review`"
+                    + " (branch must be pushed first).\n");
+            }
             return report.str;
         }
 

+ 39 - 0
src/actions/TaskActions.vala

@@ -468,6 +468,45 @@ namespace ClankerSaddle {
         }
     }
 
+    /**
+     * Flips a task's subtask auto-approve toggle: when on, review blocks
+     * raised by this task's descendants are answered automatically —
+     * the task's own review stays a human gate.
+     */
+    public class ToggleAutoApproveAction : SealedTaskAction {
+
+        private TaskService task_service = 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");
+            }
+
+            try {
+                var task = yield task_service.update_task(task_id, null, null, null, null);
+                if (task == null) {
+                    return directives().navigate("/tasks");
+                }
+                ((!)task).auto_approve = ((!)task).auto_approve != 0 ? 0 : 1;
+                yield task_service.save_auto_approve(((!)task).id, ((!)task).auto_approve != 0);
+                string state = ((!)task).auto_approve != 0 ? "on" : "off";
+                return directives().notify("success",
+                        @"Subtask auto-approve is now $(state)")
+                    .navigate(@"/tasks/$(task_id)");
+            } catch (Error e) {
+                return directives().notify("error", @"Could not toggle auto-approve: $(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,

+ 6 - 0
src/entrypoints/TaskDetailEntrypoint.vala

@@ -45,6 +45,7 @@ namespace ClankerSaddle {
         public bool answered { get; set; }
         public bool is_plan_review { get; set; }
         public bool show_body { get; set; default = true; }
+        public string answered_by { get; set; default = ""; }
         public string created_display { get; set; default = ""; }
         public string answered_display { get; set; default = ""; }
         public Statum.Model.ActionDto? answer_action { get; set; }
@@ -110,6 +111,8 @@ namespace ClankerSaddle {
         public string review_summary { get; set; default = ""; }
         public string review_diff_url { get; set; default = ""; }
         public bool resume_available { get; set; }
+        public bool auto_approve { get; set; }
+        public Statum.Model.ActionDto? toggle_auto_approve { get; set; }
         public string display { get; set; default = ""; }
         public bool terminal { get; set; }
         public bool failed { get; set; }
@@ -202,6 +205,8 @@ namespace ClankerSaddle {
             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.auto_approve = task.auto_approve != 0;
+            pub.toggle_auto_approve = actions.author_private<ToggleAutoApproveAction, 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);
@@ -240,6 +245,7 @@ namespace ClankerSaddle {
                     body = block.body ?? "",
                     options = parse_options(block.options),
                     answered = block.answer != null,
+                    answered_by = block.answered_by ?? "",
                     is_plan_review = block.block_type == BLOCK_RFC
                         && block.question == "Plan submitted for review",
                     created_display = format_time(block.created_at),

+ 2 - 0
src/main.vala

@@ -33,6 +33,7 @@ int main(string[] args) {
         db.add_migration<ClankerSaddle.Data.Migrations.M0005_RawLog>();
         db.add_migration<ClankerSaddle.Data.Migrations.M0006_Capabilities>();
         db.add_migration<ClankerSaddle.Data.Migrations.M0007_SuspendedAt>();
+        db.add_migration<ClankerSaddle.Data.Migrations.M0008_AutoApprove>();
         db.add_entity<Project>(Project.entity_mapping);
         db.add_entity<ProjectLink>(ProjectLink.entity_mapping);
         db.add_entity<TaskEntity>(TaskEntity.entity_mapping);
@@ -98,6 +99,7 @@ int main(string[] args) {
         statum.action<CancelTaskAction>();
         statum.action<RetryTaskAction>();
         statum.action<ResumeTaskAction>();
+        statum.action<ToggleAutoApproveAction>();
         statum.action<RefreshTaskDataAction>();
         statum.action<RefreshDashboardAction>();
         statum.action<RegisterSaddleAction>();

+ 9 - 1
src/pages/task_detail.html

@@ -364,7 +364,8 @@
 
                 <div stm-if="block.answered">
                     <p>Answer: <strong stm-text="block.answer">answer</strong>
-                        <span class="muted" stm-text="block.answered_display">when</span></p>
+                        <span class="muted" stm-text="block.answered_display">when</span>
+                        <span class="badge" stm-if="block.answered_by == 'auto'">auto</span></p>
                 </div>
 
                 <form stm-if="!block.answered && block.block_type == 'question'" stm-action="block.answer_action">
@@ -422,6 +423,13 @@
             </dialog>
 
             <h3>Subtasks</h3>
+            <p class="inline">Auto-approve subtask reviews:
+                <strong stm-text="task.auto_approve ? 'on' : 'off'">off</strong>
+                <button type="button" stm-action="task.toggle_auto_approve"
+                        stm-text="task.auto_approve ? 'Turn off' : 'Turn on'">Turn on</button>
+                <span class="muted">— covered descendants' review blocks are answered
+                    automatically (questions and RFCs still block; this task's own review is
+                    never auto-approved).</span></p>
             <table>
                 <thead><tr><th>#</th><th>Title</th><th>Status</th></tr></thead>
                 <tbody>

+ 120 - 0
test/web_stack_test.py

@@ -1488,6 +1488,126 @@ def main():
         check(status == 400 and "cycle" in body.get("error", ""),
               "depending on an ancestor rejected as a cycle")
 
+
+        print("== R4 B2: subtask auto-approve ==")
+        # Settle earlier stragglers first: the round-3/B1 suspended parents
+        # resume (a fresh /start) once their last subtask completes — those
+        # pushes would consume the one-shot suspend flag below.
+        check(wait_task_done(auth, push_parent, timeout=90) is not None
+              and wait_task_done(auth, push_child_two, timeout=90) is not None
+              and wait_task_done(auth, cb_parent, timeout=90) is not None,
+              "earlier suspended parents settle before the auto-approve run")
+
+        # Parent parks; its toggle then covers all descendants. Children are
+        # created with an unmet 'gpu' requirement so they stay queued (and
+        # untouched) until the requirement is lifted for the final-resume leg.
+        fake.suspend_after_start = True
+        act(auth, slot_public(auth, "/tasks", "tasks")["create"], {
+            "project_id": "1",
+            "title": "Auto parent",
+            "priority": "2",
+        })
+        auto_parent = int([t["id"] for t in slot_public(auth, "/tasks", "tasks")["tasks"]
+                           if t["title"] == "Auto parent"][0])
+        check(wait_for(lambda: task_state(auth, auto_parent).get("status") == "suspended"
+                       or False, timeout=60, what="auto parent parks") is not None,
+              "auto parent parks")
+
+        detail = task_state(auth, auto_parent)
+        check(detail.get("auto_approve") is False,
+              "auto-approve defaults to off")
+        act(auth, detail["toggle_auto_approve"], {})
+        check(task_state(auth, auto_parent).get("auto_approve") is True,
+              "toggle turns auto-approve on")
+
+        gpu_option = int([o["id"] for o in slot_public(auth, "/tasks", "tasks")["capability_options"]
+                          if o["name"] == "gpu"][0])
+        status, body = saddle_api("POST", "/api/saddle/subtask", r4_token,
+                                  {"parent_task_id": auto_parent,
+                                   "title": "Auto child", "depends_on": []})
+        auto_child = int(body["task_id"])
+        status, body = saddle_api("POST", "/api/saddle/subtask", r4_token,
+                                  {"parent_task_id": auto_child, "title": "Auto grandchild"})
+        auto_grand = int(body["task_id"])
+        act(auth, task_state(auth, auto_child)["attach_capability"],
+            {"capability_id": str(gpu_option)})
+        act(auth, task_state(auth, auto_grand)["attach_capability"],
+            {"capability_id": str(gpu_option)})
+
+        # Transitive coverage: the GRANDCHILD's review is auto-answered
+        # (any ancestor set), answered-by 'auto', timeline entry recorded.
+        conn = db()
+        conn.execute("insert into sessions (task_id, saddle_id, started_at)"
+                     " values (?, ?, strftime('%s','now'))", (auto_grand, fake_id))
+        conn.commit()
+        grand_session = conn.execute("select max(id) from sessions").fetchone()[0]
+        conn.close()
+        saddle_api("POST", "/api/saddle/%d/update" % fake_id, fake_token,
+                   {"type": "review", "session_id": grand_session, "data": {
+                       "summary": "the flange is wired"}})
+        grand_detail = task_state(auth, auto_grand)
+        grand_review = [b for b in grand_detail["blocks"] if b["block_type"] == "review"][0]
+        check(grand_review.get("answered") is True
+              and grand_review.get("answer") == "Auto-approved (subtask auto-approve is on)"
+              and grand_review.get("answered_by") == "auto",
+              "covered grandchild review auto-answered as 'auto'")
+        conn = db()
+        timeline_auto = conn.execute(
+            "select count(*) from session_commands where session_id = ?"
+            " and type = 'auto-approve'", (grand_session,)).fetchone()[0]
+        conn.close()
+        check(timeline_auto >= 1, "auto action visible in the timeline")
+
+        # Questions and RFCs still block for a human.
+        saddle_api("POST", "/api/saddle/%d/update" % fake_id, fake_token,
+                   {"type": "block", "session_id": grand_session, "data": {
+                       "question": "Which torque spec?", "type": "question"}})
+        saddle_api("POST", "/api/saddle/%d/update" % fake_id, fake_token,
+                   {"type": "block", "session_id": grand_session, "data": {
+                       "question": "RFC: approve the design", "type": "rfc",
+                       "body": "# Design"}})
+        grand_detail = task_state(auth, auto_grand)
+        check(all(b.get("answered") is not True for b in grand_detail["blocks"]
+                  if b["block_type"] in ("question", "rfc")),
+              "questions and RFCs are never auto-answered")
+
+        # The toggle covers DESCENDANTS only: the parent's own review waits.
+        conn = db()
+        conn.execute("insert into sessions (task_id, saddle_id, started_at, ended_at,"
+                     " outcome) values (?, ?, strftime('%s','now'), strftime('%s','now'),"
+                     " 'done')", (auto_parent, fake_id))
+        conn.commit()
+        parent_session = conn.execute("select max(id) from sessions").fetchone()[0]
+        conn.close()
+        saddle_api("POST", "/api/saddle/%d/update" % fake_id, fake_token,
+                   {"type": "review", "session_id": parent_session, "data": {
+                       "summary": "the whole change"}})
+        parent_detail = task_state(auth, auto_parent)
+        parent_review = [b for b in parent_detail["blocks"] if b["block_type"] == "review"][0]
+        check(parent_review.get("answered") is not True,
+              "the top-level parent's own review is never auto-approved")
+
+        # All descendants terminal → the suspended parent resumes with the
+        # final-review instruction (lift the capability requirement first).
+        act(auth, task_state(auth, auto_child)["capabilities"][0]["detach"], {})
+        act(auth, task_state(auth, auto_grand)["capabilities"][0]["detach"], {})
+
+        def pushes_for(task_id):
+            return len([p for p in fake.start_payloads
+                        if int(p.get("task", {}).get("id") or 0) == task_id])
+
+        before_pushes = pushes_for(auto_parent)
+        check(wait_for(lambda: pushes_for(auto_parent) > before_pushes or False,
+                       timeout=90, what="auto parent resumes") is not None,
+              "terminal subtasks resume the suspended parent")
+        notice = ""
+        for p in fake.start_payloads:
+            if int(p.get("task", {}).get("id") or 0) == auto_parent:
+                notice = p.get("resume_notice") or ""
+        check("All subtasks are complete" in notice
+              and "ONE final review" in notice and "saddle review" in notice,
+              "resume notice ends with the final-review instruction")
+
     finally:
         # Exit-code diagnostic: distinguishes a crashed app (negative
         # returncode, e.g. -11 SIGSEGV) from a clean shutdown. When