Merge branch 'goal/approval-fix' of clanker/clanker-saddle into web-stack

src/Services/GoalService.vala

@@ -82,7 +82,6 @@ namespace ClankerSaddle.Services {
         private StatePulse pulse = inject<StatePulse>();
         private PushService push = inject<PushService>();
         private TaskService tasks = inject<TaskService>();
-        private SaddlePollState polls = inject<SaddlePollState>();
         private EventService events = inject<EventService>();
 
         /** Appends a goal-scoped event with a human-readable line. */
@@ -771,14 +770,17 @@ namespace ClankerSaddle.Services {
                 }
             } 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).
-                    yield refuse_unpushed_origin(((!)goal), (!)task);
                     // Plan 16 S1 (canonical-on-accept): the report
                     // lands on the goal ONLY here — the winning
                     // block's body becomes completion_report, so the
                     // terminal card reads a field that provably exists
-                    // only post-acceptance.
+                    // only post-acceptance. The push-before-review
+                    // rule is enforced where the human placed it — at
+                    // the agent's completion attempt (the daemon's
+                    // submit_report gate), never here: a goal agent
+                    // parked in this review has no open session, so
+                    // the web's probe flags are necessarily stale and
+                    // an accept-time check could only misfire.
                     if (((!)block).body != null && ((!)block).body.length > 0) {
                         ((!)goal).completion_report = ((!)block).body;
                     }
@@ -1459,61 +1461,6 @@ namespace ClankerSaddle.Services {
             return section.str;
         }
 
-        // ------------------------------------------------------------------
-        // Completion gate (plan 15 C4)
-        // ------------------------------------------------------------------
-
-        /**
-         * The hard human gate on completion acceptance (plan 15 C4,
-         * F9): the accept REFUSES while any daemon-reported goal
-         * branch is unpushed — push-after-each-merge is enforced at
-         * the operator's confirmation, read from the LIVE per-saddle
-         * probe flags (never a stored snapshot) through the ONE
-         * {@link parse_goal_branches} parser (the same rows the goal
-         * page's chips render). Unknown flags (an older daemon that
-         * never reported goal_branches) leave the daemon-side push
-         * rule as the enforcing layer.
-         */
-        private async void refuse_unpushed_origin(Goal goal, TaskEntity run) throws Error {
-            int64 saddle_id = 0;
-            foreach (var session in yield db.query<Session>()
-                    .where(expr("task_id == $0", elem<int64?>(run.id)))
-                    .order_by_desc(expr("id"))
-                    .materialise_async()) {
-                if (session.ended_at == null) {
-                    saddle_id = session.saddle_id;
-                    break;
-                }
-                if (saddle_id == 0) {
-                    saddle_id = session.saddle_id;
-                }
-            }
-            if (saddle_id == 0) {
-                return;
-            }
-            string raw = polls.goal_branches_for(saddle_id);
-            if (raw.length == 0) {
-                return;
-            }
-            var unpushed = new Vector<string>();
-            foreach (var row in parse_goal_branches(raw)) {
-                if (!row.pushed) {
-                    unpushed.add(row.label());
-                }
-            }
-            if (unpushed.length > 0) {
-                var labels = new StringBuilder();
-                foreach (var label in unpushed.to_array()) {
-                    if (labels.len > 0) {
-                        labels.append(", ");
-                    }
-                    labels.append(label);
-                }
-                throw new Error(Quark.from_string("clanker-saddle"), 88,
-                    @"Origin is behind — push the goal branch(es) before accepting ($(labels.str)). The accept refuses until the daemon reports them pushed.");
-            }
-        }
-
         // ------------------------------------------------------------------
         // Helpers
         // ------------------------------------------------------------------
@@ -1539,12 +1486,13 @@ namespace ClankerSaddle.Services {
         /**
          * The ONE parser of the daemon's goal_branches report (plan
          * 15 C4): an array of `{repo, branch, pushed}` rows, read by
-         * BOTH consumers — the completion-accept gate
-         * ({@link refuse_unpushed_origin}) and the goal page's push
-         * chips — so the two can never diverge on the same payload
-         * again. `pushed` is the daemon's boolean (an integer counts
-         * as non-zero; anything else reads unpushed). A row missing
-         * its branch labels by its repo alone; label-less rows drop.
+         * the goal page's push chips — an advisory display of the
+         * live per-saddle probe flags. The push rule itself is
+         * enforced at the agent's completion attempt (the daemon's
+         * submit_report gate), never on the web's accept path.
+         * `pushed` is the daemon's boolean (an integer counts as
+         * non-zero; anything else reads unpushed). A row missing its
+         * branch labels by its repo alone; label-less rows drop.
          * Unparseable input parses as no rows.
          */
         public static Vector<GoalBranchPush> parse_goal_branches(string raw) {

src/Services/SaddlePollState.vala

@@ -52,6 +52,9 @@ namespace ClankerSaddle.Services {
         // Plan 15 C4: the daemon-reported per-repo push flags of the
         // goal branches, as the LAST /status probe saw them — live
         // probe state only, NEVER a stored snapshot on the goal row.
+        // They feed only the goal page's advisory push chips (the
+        // push rule itself lives daemon-side, at the agent's
+        // completion attempt).
         private Dictionary<string, string> probe_goal_branches = new Dictionary<string, string>();
         private Mutex state_lock = Mutex();
 
@@ -74,8 +77,8 @@ namespace ClankerSaddle.Services {
         /**
          * Records the daemon-reported goal-branch push flags (plan 15
          * C4): raw JSON as the probed daemon sent it — the goal page
-         * and the completion-accept gate read it back live. A missing
-         * member clears the record (nothing to render or enforce).
+         * reads them back live to render its advisory push chips. A
+         * missing member clears the record (nothing to render).
          */
         public void record_goal_branches(int64 saddle_id, string raw_json) {
             state_lock.lock();

src/Services/SaddleService.vala

@@ -521,10 +521,10 @@ namespace ClankerSaddle.Services {
             // workspace stays while the saddle frees) a goal agent
             // exposes the per-repo push flags of the goal branches on
             // /status; they are recorded as LIVE probe state on the
-            // saddle (never a stored snapshot on the goal row — a
-            // pre-push snapshot is exactly the F9 bug), and the goal
-            // page + the completion-accept gate read them back at
-            // render/answer time.
+            // saddle (never a stored snapshot on the goal row) and
+            // the goal page reads them back at render time for its
+            // advisory push chips. The push rule itself is enforced
+            // daemon-side at the agent's completion attempt.
             if (status.has_member("goal_branches")) {
                 var goal_branches = status.get_member("goal_branches");
                 if (goal_branches.get_node_type() == Json.NodeType.ARRAY

src/entrypoints/GoalDetailEntrypoint.vala

@@ -946,9 +946,11 @@ namespace ClankerSaddle {
             if (open_session_saddle > 0 && ctx.polls != null) {
                 string flags = ((!)ctx.polls).goal_branches_for(open_session_saddle);
                 if (flags.length > 0) {
-                    // The SAME parser the completion-accept gate reads
-                    // (GoalService.parse_goal_branches) feeds the chips —
-                    // gate and display cannot diverge on one payload.
+                    // The parser (GoalService.parse_goal_branches)
+                    // feeds the chips — a purely advisory rendering
+                    // of the live probe flags; the push rule itself
+                    // is enforced daemon-side at the agent's
+                    // completion attempt.
                     foreach (var row in GoalService.parse_goal_branches(flags)) {
                         view.push_branches.add(new GoalPushBranchView() {
                             branch = row.label(),

src/pages/goal_detail.html

@@ -224,7 +224,9 @@
                     <!-- The pending completion review renders in the rich
                          review form (highlight-remarks queue shared with
                          the plan review; push chips + compare above the
-                         report; accept gate enforced server-side). -->
+                         report — chips are advisory, the push rule is
+                         enforced daemon-side at the agent's completion
+                         attempt). -->
                     <div stm-if="block.is_completion_review"
                          class="plan-review" data-plan-review
                          stm-attribute.data-block-id="block.id">

test/bundles/goals.py

@@ -787,48 +787,35 @@ def run(ctx):
     conn.close()
     check(rows == 1,
           "a redelivered review-kind push adopts the pending row (raise-side single-row invariant)")
-    # 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.
-    refusal = action_notify(act(auth, state["completion_pending_block"]["answer_action"],
-                                {"answer": "Review approved. Continue with the task."}))
-    check(goal_state(goal1)["status"] == "completion_review",
-          "the accept REFUSES while origin is behind (hard gate, live flags)")
-    # One parser, one label: the gate's refusal names the SAME repo-
-    # qualified unpushed row the chips render — both surfaces read
-    # GoalService.parse_goal_branches (the gate-vs-chips divergence is
-    # structurally gone).
-    branches = {(b["branch"], b["pushed"]) for b in goal_state(goal1)["push_branches"]}
-    check("repob: goal/flange" in (refusal or "")
-          and ("repob: goal/flange", False) in branches
-          and ("repoa: goal/flange", True) in branches,
-          "the refusal and the chips read ONE parser (same labels, same flags)")
-    # Push the branch daemon-side and probe again — the flags flip
-    # live (no goal-row write), and the accept goes through.
+    # Goal 21 (accept-gate removal): the accept-time push gate is GONE.
+    # The push-before-review rule lives daemon-side, at the agent's
+    # completion attempt — a goal agent parked in this review has no
+    # open session, so the web's probe flags are necessarily stale and
+    # an accept-time check could only misfire (error 88 wedged the
+    # goal in completion_review forever). Regression: with the cached
+    # flags still reading repob UNPUSHED, the accept must go straight
+    # through — the flags feed only the goal page's advisory chips.
     fake.status_extra = {"goal_branches": [
         {"repo": "repoa", "branch": "goal/flange", "pushed": True},
-        {"repo": "repob", "branch": "goal/flange", "pushed": True}]}
+        {"repo": "repob", "branch": "goal/flange", "pushed": False}]}
     act(auth, fake_card["refresh"], {})
     check(wait_for(lambda: (goal_state(goal1).get("push_branches") or [])
-                   and all(b["pushed"] for b in goal_state(goal1)["push_branches"])
-                   or False, timeout=30, what="pushed flags land") is not None,
-          "the push chips derive from the LIVE probe flags")
+                   and any((b["branch"], b["pushed"]) == ("repob: goal/flange", False)
+                           for b in goal_state(goal1)["push_branches"])
+                   or False, timeout=30, what="unpushed flags land") is not None,
+          "the chips still render the live (unpushed) probe flags")
     conn = db()
     no_push_column = conn.execute(
         "select count(*) from pragma_table_info('goals')"
         " where name = 'branch_push_state'").fetchone()[0] == 0
     conn.close()
     check(no_push_column,
-          "no stored push snapshot exists (F9: the goal row never records it)")
-    state = goal_state(goal1)
-    # The passing accept posts the verdict MARKER (the goal page's
-    # accept form shape, plan 16 S2) — the server resolves the
-    # completion kind and stores the stable approval wording.
+          "no stored push snapshot exists (the goal row never records it)")
     act(auth, state["completion_pending_block"]["answer_action"],
         {"verdict": "accept"})
     check(wait_for(lambda: goal_state(goal1)["status"] == "complete" or False,
                    timeout=30, what="accepted completion completes") is not None,
-          "accepting the completion completes the goal once origin is current")
+          "the accept COMPLETES the goal while cached flags read unpushed (no gate)")
     state = goal_state(goal1)
     conn = db()
     accepted_report = conn.execute(

test/integration_test.py

@@ -28,10 +28,11 @@ call, questions ride `saddle block` through run_command):
 5. the terminal transition (the goals row's last_terminal_watermark)
    coalesces (~SADDLE_GOAL_RESUME_S) into the goal agent's batched
    resume; it merges, deletes the task branch, pushes the goal branch
-   (push-before-review) and submit_report enters completion review
-6. completion review accept — the live per-repo push flags gate it:
-   the accept REFUSES while origin is behind, passes once the branch
-   is re-pushed and the probe re-cached -> goal complete, run task done
+   (push-before-review, enforced daemon-side at the completion
+   attempt) and submit_report enters completion review
+6. completion review accept -> goal complete, run task done (no
+   web-side push gate: the daemon already refused the completion
+   attempt until every goal branch was on origin)
 7. the audit trail: the DERIVED goal narrative, the unified events
    ledger (turn cards with tool_call/tool_result, usage rows, the §7.2
    kind set, web-originated-only comment kinds), the event log page +
@@ -698,50 +699,17 @@ def main():
                                 capture_output=True).returncode == 0
         check(pushed, "the goal branch was pushed to origin (push-before-review)")
 
-        print("== accept refuses while origin is behind (the live push gate) ==")
-        # Rewind origin's goal branch to the pre-merge base and refresh
-        # the daemon probe: the live per-repo flags flip to unpushed and
-        # the completion accept must REFUSE (plan 15 C4 / F9) until the
-        # branch is pushed again — enforcing push-after-each-merge at
-        # the human gate, read from probe state (never a snapshot).
-        completion = state["completion_pending_block"]
-        subprocess.run(["git", "-C", goal_clone, "push", "-f", "origin",
-                        "main:refs/heads/goal/demo-run"], check=True, capture_output=True)
-        subprocess.run(["git", "-C", goal_clone, "fetch", "origin"],
-                       check=True, capture_output=True)
-        act(auth, card["refresh"], {})
-        check(wait_for(lambda: any(
-            "demo" in (b.get("branch") or "") and b.get("pushed") is False
-            for b in goal_state(auth, goal1).get("push_branches", [])) or False,
-            timeout=60, what="unpushed chip") is not None,
-              "the rewound origin flips the live push chip to unpushed")
-        refusal = act(auth, completion["answer_action"], {"verdict": "accept"})
-        check(any(d.get("type") == "notify" and d.get("kind") == "error"
-                  and "Origin is behind" in (d.get("message") or "")
-                  for d in refusal),
-              "the accept refuses with a clear message while origin is behind")
-        check(goal_state(auth, goal1)["status"] == "completion_review",
-              "the refused accept leaves the goal in completion review")
-        check(goal_state(auth, goal1)["completion_pending_block"]
-              not in (None, False),
-              "the refused accept leaves the completion block unanswered")
-        subprocess.run(["git", "-C", goal_clone, "push", "origin", "goal/demo-run"],
-                       check=True, capture_output=True)
-        act(auth, card["refresh"], {})
-        check(wait_for(lambda: any(
-            "demo" in (b.get("branch") or "") and b.get("pushed") is True
-            for b in goal_state(auth, goal1).get("push_branches", [])) or False,
-            timeout=60, what="pushed chip restored") is not None,
-              "re-pushing the branch restores the live push chip (after push)")
-        completion = goal_state(auth, goal1)["completion_pending_block"]
-
         print("== completion accept ==")
+        # Goal 21: the accept-time push gate is gone — the daemon already
+        # refused the agent's completion attempt until every goal branch
+        # was pushed (push-before-review, asserted above), so the branch
+        # is on origin here and the verdict path needs no second check.
         check("greeting.txt added via one worker task" in (completion or {}).get("body", ""),
               "the run task's completion block carries the report")
         winning_body = (completion or {}).get("body", "")
         accept = act(auth, completion["answer_action"], {"verdict": "accept"})
         check(not any(d.get("type") == "notify" and d.get("kind") == "error" for d in accept),
-              "the accept passes once the push flags read pushed")
+              "the accept passes (no web-side push gate on the verdict)")
         check(wait_goal_status(auth, goal1, "complete", timeout=30) is not None,
               "accepting the verdict marker completes the goal (server-owned wording)")
         check(wait_task_status(auth, run_task, "done", timeout=30) is not None,