Kaynağa Gözat

fix: the completion review is one row — review-family cross-dedupe in raise_run_block (the daemon's done-park review block adopts the web's completion row instead of raising a twin that double-rendered the goal page review and pinned the dashboard); compare-changes link on the goal status head

clanker 1 ay önce
ebeveyn
işleme
89d58b1a8c

+ 32 - 5
src/Services/GoalService.vala

@@ -29,6 +29,18 @@ namespace ClankerSaddle.Services {
     public const string GOAL_BLOCK_PLAN = "plan";
     public const string GOAL_BLOCK_COMPLETION = "completion";
 
+    /**
+     * Whether a run-task block row is THE completion review. The review is
+     * raised from two paths — the web's completion raise (kind
+     * {@link GOAL_BLOCK_COMPLETION}) and the daemon's done-park (kind
+     * "review") — which adopt each other so only one row ever exists;
+     * every completion-verdict/narrative surface matches both kinds.
+     */
+    private static bool is_completion_row(TaskBlock block) {
+        return block.block_type == GOAL_BLOCK_COMPLETION
+            || block.block_type == "review";
+    }
+
     /** Task kinds (the `tasks.kind` wire values). */
     public const string TASK_KIND_TASK = "task";
     public const string TASK_KIND_GOAL_RUN = "goal_run";
@@ -419,7 +431,10 @@ namespace ClankerSaddle.Services {
                 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).
+            // Idempotent on the pending report (see submit_plan): the
+            // raise adopts an unanswered review-family row (the daemon's
+            // done-park "review" block IS this review) — one row,
+            // whichever path landed first.
             TaskBlock? raised = yield raise_run_block(((!)goal), "Goal completion report",
                 GOAL_BLOCK_COMPLETION, null, report, true);
             if (raised == null) {
@@ -548,7 +563,7 @@ namespace ClankerSaddle.Services {
                 } else {
                     ((!)goal).status = GOAL_PLANNING;
                 }
-            } else if (((!)block).block_type == GOAL_BLOCK_COMPLETION) {
+            } else if (is_completion_row(((!)block))) {
                 if (answer == NOTICE_REVIEW_APPROVED) {
                     // Plan 15 C4: the operator-confirmed hard gate —
                     // refuse while origin is behind (live flags only).
@@ -563,7 +578,7 @@ namespace ClankerSaddle.Services {
 
             yield tasks.answer_block(block_id, answer, answered_by);
 
-            if (((!)block).block_type == GOAL_BLOCK_COMPLETION
+            if (is_completion_row(((!)block))
                     && answer == NOTICE_REVIEW_APPROVED) {
                 // Approving the completion closes the goal agent's
                 // own task: the run is finished, its saddle frees.
@@ -601,7 +616,19 @@ namespace ClankerSaddle.Services {
                     goal.id.to_string(), question);
                 return null;
             }
+            // Review-family cross-dedupe: the completion review is raised
+            // from two paths (the web's completion kind, the daemon's
+            // done-park "review" kind, different question strings) — an
+            // unanswered row of EITHER kind IS the pending review; adopt
+            // it rather than raising a twin.
+            bool review_family = block_type == GOAL_BLOCK_COMPLETION
+                || block_type == "review";
             foreach (var existing in yield tasks.list_blocks(((!)run).id)) {
+                if (review_family && existing.answer == null
+                        && (existing.block_type == GOAL_BLOCK_COMPLETION
+                            || existing.block_type == "review")) {
+                    return existing;
+                }
                 bool same = dedupe_on_type
                     ? existing.block_type == block_type : existing.question == question;
                 if (same && existing.answer == null) {
@@ -742,7 +769,7 @@ namespace ClankerSaddle.Services {
                             rows.add(new NarrativeRow() {
                                 kind = "plan-submitted", at = block.created_at,
                                 text = @"Plan revision #$(block.id) submitted for review" });
-                        } else if (block.block_type == GOAL_BLOCK_COMPLETION) {
+                        } else if (is_completion_row(block)) {
                             rows.add(new NarrativeRow() {
                                 kind = "completion-submitted", at = block.created_at,
                                 text = "Completion report submitted for review" });
@@ -761,7 +788,7 @@ namespace ClankerSaddle.Services {
                                 text = approved
                                     ? @"Plan revision #$(block.id) approved — the goal is active"
                                     : @"Plan revision #$(block.id) rejected with feedback — the agent revises" });
-                        } else if (block.block_type == GOAL_BLOCK_COMPLETION) {
+                        } else if (is_completion_row(block)) {
                             bool approved = block.answer == NOTICE_REVIEW_APPROVED;
                             rows.add(new NarrativeRow() {
                                 kind = approved ? "goal-approved" : "completion-rejected",

+ 13 - 0
src/Services/SaddleService.vala

@@ -1406,7 +1406,20 @@ namespace ClankerSaddle.Services {
             if (question.length == 0) {
                 return;
             }
+            string incoming_type = block_string_or(block, "type", "block_type");
             foreach (var existing in yield tasks.list_blocks(task_id)) {
+                if (existing.answer == null || !poll_state.is_delivered(existing.id)) {
+                    // A pending review-family block of EITHER kind
+                    // (review/completion) swallows the other's push: the
+                    // daemon's done park and the web's completion raise
+                    // are the SAME review raised from two paths — one
+                    // row, whichever landed first.
+                    if ((incoming_type == "review" || incoming_type == "completion")
+                            && (existing.block_type == "review"
+                                || existing.block_type == "completion")) {
+                        return;
+                    }
+                }
                 if (existing.question != question) {
                     continue;
                 }

+ 6 - 2
src/entrypoints/GoalDetailEntrypoint.vala

@@ -425,8 +425,12 @@ namespace ClankerSaddle {
             foreach (var block in yield goals.list_blocks(goal.id)) {
                 bool pending_plan = block.block_type == GOAL_BLOCK_PLAN
                     && block.answer == null;
-                bool pending_completion = block.block_type == GOAL_BLOCK_COMPLETION
-                    && block.answer == null;
+                // The completion review may ride either kind: the web's
+                // completion raise or the daemon's done-park review block
+                // (adopted — one row, whichever landed first).
+                bool pending_completion = block.answer == null
+                    && (block.block_type == GOAL_BLOCK_COMPLETION
+                        || block.block_type == "review");
                 var view = new GoalBlockView() {
                     id = block.id,
                     question = block.question,

+ 2 - 0
src/pages/goal_detail.html

@@ -301,6 +301,8 @@
                 <p><span stm-attribute.class="'badge badge-' + goal.status" stm-text="goal.display">status</span>
                     · goal branch <code stm-text="goal.branch_name">branch</code>
                     <span class="muted">from</span> <code stm-text="goal.base_branch">base</code>
+                    <a stm-if="goal.compare_url.length > 0" stm-attribute.href="goal.compare_url"
+                       target="_blank" rel="noopener">· compare changes</a>
                     · priority <strong stm-text="goal.priority">3</strong>
                     <span stm-if="goal.agent_saddle.length > 0"> · agent saddle
                         <strong stm-text="goal.agent_saddle">saddle</strong></span></p>

+ 22 - 0
test/web_stack_test.py

@@ -5737,6 +5737,28 @@ def bundle_goals(ctx):
                    timeout=60, what="resubmitted completion") is not None,
           "the agent resubmits its completion after feedback")
     state = goal_state(goal1)
+    # Regression (plan-15 followup): the daemon's done-park ALSO pushes a
+    # review-kind block payload (question "Goal completion review", the
+    # real daemon's wording) after the done entry raised the completion
+    # row — the two paths must converge on ONE row, not twin reviews.
+    fake.push_results("blocked", block={
+        "question": "Goal completion review", "type": "review",
+        "body": "# Completion v2\n\nThe flange ships, with docs.",
+        "options": None})
+    conn = db()
+    rows = conn.execute(
+        "select count(*) from task_blocks where task_id = ?"
+        " and answer is null and type in ('completion','review')",
+        (run_task,)).fetchone()[0]
+    conn.close()
+    check(rows == 1,
+          "the daemon's review-kind completion push adopts the pending row (no twin)")
+    state = goal_state(goal1)
+    review_family_in_open = [b for b in state.get("open_blocks", [])
+                             if b["block_type"] in ("completion", "review")]
+    check(state["completion_pending"] is True and not review_family_in_open,
+          "the completion review renders once (dedicated card, not duplicated)")
+    fake.pending_block = None
     # Plan 15 C4: the completion-accept hard gate — the live probe
     # flags still report repob's branch unpushed, so the operator's
     # accept REFUSES with origin behind.