fix(kanban): judge unachievable goals as blocked, never done

2026-09-02 18:46:45 +10:00
parent d3e2ace1dd
commit 1bd9fce6cb
5 changed files with 195 additions and 33 deletions
@@ -153,12 +153,22 @@ JUDGE_SYSTEM_PROMPT = (
    "You are a strict judge evaluating whether an autonomous agent has "
    "achieved a user's stated goal. You receive the goal text, the agent's "
    "most recent response, and — when present — a list of background "
-    "processes the agent has running. Decide one of three verdicts.\n\n"
+    "processes the agent has running. Decide one of four verdicts.\n\n"
    "DONE — the goal is fully satisfied:\n"
    "- The response explicitly confirms the goal was completed, OR\n"
-    "- The response clearly shows the final deliverable was produced, OR\n"
-    "- The response explains the goal is unachievable / blocked / needs "
-    "user input (treat this as DONE with reason describing the block).\n\n"
+    "- The response clearly shows the final deliverable was produced.\n"
+    "DONE requires the deliverable to actually exist. If the response only "
+    "explains why the goal cannot be reached, the verdict is BLOCKED, not "
+    "DONE.\n\n"
+    "BLOCKED — the goal cannot be satisfied as stated:\n"
+    "- The response explains the goal is genuinely unachievable (impossible, "
+    "out of scope, no valid path to the deliverable), or refuses to "
+    "fabricate a deliverable that cannot exist, OR\n"
+    "- The response explains progress is blocked and the next step needs "
+    "user input to proceed.\n"
+    "Return BLOCKED with the reason describing what is blocking. BLOCKED is "
+    "a refusal, not a completion — never return BLOCKED for a goal that "
+    "was achieved.\n\n"
    "WAIT — the goal is NOT done, but the next step is to wait for async "
    "work to finish rather than act again. Choose this ONLY when the agent's "
    "progress is genuinely gated on something running on its own:\n"
@@ -180,6 +190,7 @@ JUDGE_SYSTEM_PROMPT = (
    "take right now. This is the default when in doubt.\n\n"
    "Reply ONLY with a single JSON object on one line. Shapes:\n"
    '{"verdict": "done", "reason": "<one sentence>"}\n'
+    '{"verdict": "blocked", "reason": "<one sentence>"}\n'
    '{"verdict": "continue", "reason": "<one sentence>"}\n'
    '{"verdict": "wait", "wait_on_session": "<id>", "reason": "<one sentence>"}\n'
    '{"verdict": "wait", "wait_on_pid": <int>, "reason": "<one sentence>"}\n'
@@ -203,7 +214,7 @@ JUDGE_USER_PROMPT_TEMPLATE = (
    "Agent's most recent response:\n{response}\n\n"
    "{background_block}"
    "Current time: {current_time}\n\n"
-    "Is the goal satisfied — done, continue, or wait?"
+    "Is the goal satisfied — done, blocked, continue, or wait?"
 )

 # Used when the user has added /subgoal criteria. The judge must
@@ -247,11 +258,11 @@ JUDGE_USER_PROMPT_WITH_CONTRACT_TEMPLATE = (
    "process to satisfy the Verification criterion (e.g. CI is the "
    "verification and it's still running), return WAIT on that process "
    "instead of re-poking — re-poking now would be pure busy-work.\n"
-    "- If the response explains the work is blocked / unachievable / needs "
-    "user input (e.g. the stated Stop condition was hit), treat it as DONE "
-    "with the reason describing the block.\n"
+    "- If the response explains the work is genuinely unachievable or hits "
+    "the stated Stop condition and needs user input, the goal is NOT done — "
+    "return BLOCKED with the reason describing the block.\n"
    "- Otherwise the goal is NOT done — CONTINUE.\n\n"
-    "Is the goal satisfied per its completion contract — done, continue, or wait?"
+    "Is the goal satisfied per its completion contract — done, blocked, continue, or wait?"
 )


@@ -553,7 +564,7 @@ class GoalState:
    max_turns: int = DEFAULT_MAX_TURNS
    created_at: float = 0.0
    last_turn_at: float = 0.0
-    last_verdict: Optional[str] = None        # "done" | "continue" | "skipped"
+    last_verdict: Optional[str] = None        # "done" | "blocked" | "continue" | "wait" | "skipped"
    last_reason: Optional[str] = None
    paused_reason: Optional[str] = None       # why we auto-paused (budget, etc.)
    consecutive_parse_failures: int = 0       # judge-output parse failures in a row
@@ -1027,7 +1038,7 @@ def _parse_judge_response(raw: str) -> Tuple[str, str, bool, Optional[Dict[str,
    """Parse the judge's reply. Fail-open on unusable output.

    Returns ``(verdict, reason, parse_failed, wait_directive)`` where:
-      - ``verdict`` is ``"done"``, ``"continue"``, or ``"wait"``.
+      - ``verdict`` is ``"done"``, ``"blocked"``, ``"continue"``, or ``"wait"``.
      - ``parse_failed`` is True when the judge returned output that couldn't
        be interpreted as the expected JSON verdict (empty body, prose,
        malformed JSON). Callers use it to auto-pause after N consecutive
@@ -1084,7 +1095,7 @@ def _parse_judge_response(raw: str) -> Tuple[str, str, bool, Optional[Dict[str,
            done = bool(done_val)
        verdict = "done" if done else "continue"

-    if verdict not in {"done", "continue", "wait"}:
+    if verdict not in {"done", "blocked", "continue", "wait"}:
        verdict = "continue"

    if verdict != "wait":
@@ -1178,7 +1189,7 @@ def judge_goal(
    """Ask the auxiliary model whether the goal is satisfied.

    Returns ``(verdict, reason, parse_failed, wait_directive, transport_failed)`` where verdict
-    is ``"done"``, ``"continue"``, ``"wait"``, or ``"skipped"`` (when the
+    is ``"done"``, ``"blocked"``, ``"continue"``, ``"wait"``, or ``"skipped"`` (when the
    judge couldn't be reached). ``wait_directive`` is set only for ``"wait"``
    (``{"pid": int}`` or ``{"seconds": int}``); ``None`` otherwise.

@@ -1882,7 +1893,7 @@ class GoalManager:
          - ``status``: current goal status after update
          - ``should_continue``: bool — caller should fire another turn
          - ``continuation_prompt``: str or None
-          - ``verdict``: "done" | "continue" | "wait" | "skipped" | "inactive"
+          - ``verdict``: "done" | "blocked" | "continue" | "wait" | "skipped" | "inactive"
          - ``reason``: str
          - ``message``: user-visible one-liner to print/send
        """
@@ -1999,6 +2010,28 @@ class GoalManager:
                "message": f"⏳ Goal parked (judge) — waiting on {tgt}: {reason}",
            }

+        # BLOCKED verdict: the judge ruled the goal genuinely cannot be
+        # satisfied as stated (impossible, out of scope, needs user input).
+        # This is NOT done — don't keep burning turns on an unachievable goal
+        # and don't wave it through as complete (#100954). Pause so the user
+        # sees the judge's reason and can re-scope (/goal set) or override
+        # (/goal resume).
+        if verdict == "blocked":
+            state.status = "paused"
+            state.paused_reason = f"judged unachievable: {reason}"
+            save_goal(self.session_id, state)
+            return {
+                "status": "paused",
+                "should_continue": False,
+                "continuation_prompt": None,
+                "verdict": "blocked",
+                "reason": reason,
+                "message": (
+                    f"🚫 Goal judged unachievable — paused: {reason} "
+                    "Re-scope with /goal set, or override with /goal resume."
+                ),
+            }
+
        if verdict == "done":
            state.status = "done"
            save_goal(self.session_id, state)
@@ -2202,7 +2235,7 @@ def run_kanban_goal_loop(
    Returns a decision dict: ``{"outcome", "turns_used", "reason"}`` where
    outcome is one of ``"completed_by_worker"``, ``"review_requested_by_worker"``,
    ``"changes_requested_by_reviewer"``, ``"blocked_budget"``,
-    ``"blocked_by_worker"``, or ``"stopped"``.
+    ``"blocked_unachievable"``, ``"blocked_by_worker"``, or ``"stopped"``.
    """

    def _log(msg: str) -> None:
@@ -2258,6 +2291,22 @@ def run_kanban_goal_loop(
            verdict = "continue"
        _log(f"kanban goal loop: turn {turns_used}/{max_turns} verdict={verdict} reason={_truncate(reason, 120)}")

+        if verdict == "blocked":
+            # The judge ruled the goal cannot be satisfied at all — this is
+            # NOT done (#100954). Block the card now with the judge's reason
+            # instead of spending the remaining turns re-poking an impossible
+            # goal, and never let it land in done.
+            _log(f"kanban goal loop: task {task_id} judged unachievable; blocking")
+            try:
+                block_fn(f"Goal-mode judge ruled the goal unachievable: {reason}")
+            except Exception as exc:
+                _log(f"kanban goal loop: block_fn failed ({exc})")
+            return {
+                "outcome": "blocked_unachievable",
+                "turns_used": turns_used,
+                "reason": f"judge verdict blocked: {reason}",
+            }
+
        if verdict == "done":
            if nudged_to_finalize:
                # Already asked once to call kanban_complete and it still
@@ -2311,18 +2311,23 @@ def _worker_run_id_for(task_id: str) -> Optional[int]:
        return None


-def _goal_mode_handoff_rejection(task: Optional[kb.Task], evidence: str) -> Optional[str]:
-    """Apply the goal judge to every terminal worker handoff, including review."""
+def _goal_mode_handoff_rejection(task: Optional[kb.Task], evidence: str):
+    """Apply the goal judge to every terminal worker handoff, including review.
+
+    Returns ``(verdict, reason_or_None)`` — ``"done"`` allows the handoff;
+    ``"blocked"`` means the judge ruled the goal unachievable (#100954);
+    ``"continue"``/``"wait"`` reject with the judge's reason.
+    """
    if task is None or not task.goal_mode:
-        return None
+        return ("done", None)
    try:
        from agent.auxiliary_client import get_text_auxiliary_client

        client, model = get_text_auxiliary_client("goal_judge")
    except Exception:
-        return None
+        return ("done", None)
    if client is None or not model:
-        return None
+        return ("done", None)

    from hermes_cli.goals import judge_goal

@@ -2341,7 +2346,7 @@ def _goal_mode_handoff_rejection(task: Optional[kb.Task], evidence: str) -> Opti
            judge_exc,
            exc_info=True,
        )
-    return reason if verdict != "done" else None
+    return (verdict, None if verdict == "done" else reason)


 def _cmd_complete(args: argparse.Namespace) -> int:
@@ -2379,11 +2384,21 @@ def _cmd_complete(args: argparse.Namespace) -> int:
            # to every terminal handoff so request-review cannot bypass the
            # acceptance contract that protects complete.
            task = kb.get_task(conn, tid)
-            rejection = _goal_mode_handoff_rejection(
+            gate_verdict, rejection = _goal_mode_handoff_rejection(
                task,
                (summary or args.result or "").strip(),
            )
-            if rejection is not None:
+            if gate_verdict == "blocked":
+                print(
+                    f"kanban: goal completion of {tid} rejected: judge ruled "
+                    f"the goal unachievable — {rejection}. Re-scope with "
+                    f"kanban edit, or record the block with kanban block "
+                    f"instead of completing.",
+                    file=sys.stderr,
+                )
+                failed.append(tid)
+                continue
+            if gate_verdict == "continue" or rejection is not None:
                print(
                    f"kanban: goal completion of {tid} rejected by judge: {rejection}. "
                    f"Provide evidence matching the task's acceptance criteria.",
@@ -2532,11 +2547,19 @@ def _cmd_request_review(args: argparse.Namespace) -> int:
            return 2
    reviewer = getattr(args, "reviewer", None)
    with kb.connect_closing() as conn:
-        rejection = _goal_mode_handoff_rejection(
+        gate_verdict, rejection = _goal_mode_handoff_rejection(
            kb.get_task(conn, tid),
            summary or "",
        )
-        if rejection is not None:
+        if gate_verdict == "blocked":
+            print(
+                f"kanban: goal review handoff of {tid} rejected: judge ruled "
+                f"the goal unachievable — {rejection}. Record the block with "
+                f"kanban block instead of requesting review.",
+                file=sys.stderr,
+            )
+            return 1
+        if gate_verdict == "continue" or rejection is not None:
            print(
                f"kanban: goal review handoff of {tid} rejected by judge: "
                f"{rejection}. Provide acceptance evidence matching the task.",
@@ -798,3 +798,55 @@ class TestContractAndBackgroundCompose:
        assert verdict == "wait"
        assert wait_directive and wait_directive.get("pid") == 4242

+
+class TestBlockedVerdict:
+    """#100954: a genuinely unachievable goal must be refused, not completed."""
+
+    def test_parse_judge_response_accepts_blocked(self):
+        from hermes_cli.goals import _parse_judge_response
+
+        verdict, reason, parse_failed, _wd = _parse_judge_response(
+            '{"verdict": "blocked", "reason": "the repo was deleted"}'
+        )
+        assert verdict == "blocked"
+        assert reason == "the repo was deleted"
+        assert parse_failed is False
+
+    def test_blocked_verdict_pauses_goal_instead_of_done(self, hermes_home):
+        from unittest.mock import patch
+        from hermes_cli.goals import GoalManager
+
+        mgr = GoalManager(session_id="blocked-sid")
+        mgr.set("delete a repository that does not exist")
+        with patch(
+            "hermes_cli.goals.judge_goal",
+            return_value=("blocked", "the repo does not exist", False, None, False),
+        ):
+            decision = mgr.evaluate_after_turn(
+                "The repo cannot be deleted: it does not exist."
+            )
+
+        assert decision["verdict"] == "blocked"
+        assert decision["status"] == "paused"
+        assert decision["should_continue"] is False
+        assert "unachievable" in decision["message"].lower()
+        assert mgr.state is not None
+        assert mgr.state.status == "paused"
+        assert "unachievable" in (mgr.state.paused_reason or "").lower()
+
+    def test_blocked_verdict_never_records_done(self, hermes_home):
+        from unittest.mock import patch
+        from hermes_cli.goals import GoalManager
+
+        mgr = GoalManager(session_id="blocked-sid-2")
+        mgr.set("square the circle")
+        with patch(
+            "hermes_cli.goals.judge_goal",
+            return_value=("blocked", "mathematically impossible", False, None, False),
+        ):
+            decision = mgr.evaluate_after_turn("This cannot be done.")
+        assert decision["status"] != "done"
+        assert mgr.state is not None
+        assert mgr.state.status != "done"
+        assert mgr.state.last_verdict == "blocked"
+
@@ -205,3 +205,21 @@ class TestCLIJudgeGate:
        rc, complete_calls = self._run(monkeypatch, goal_mode=False)
        assert rc == 0
        assert complete_calls == ["t1"]
+
+    def test_judge_blocked_verdict_rejects_completion(self, monkeypatch, capsys):
+        """#100954: an unachievable goal must not complete silently.
+
+        The judge's ``blocked`` verdict is a refusal, not a completion —
+        ``complete_task`` must never run and stderr must steer the user
+        toward re-scoping / recording the block.
+        """
+        rc, complete_calls = self._run(
+            monkeypatch,
+            verdict="blocked",
+            reason="the target repository does not exist",
+        )
+        err = capsys.readouterr().err
+        assert rc != 0, "blocked verdict must reject the completion"
+        assert complete_calls == [], "an unachievable goal must never reach complete_task"
+        assert "unachievable" in err.lower()
+        assert "kanban block" in err.lower()
@@ -251,10 +251,16 @@ def _goal_judge_available() -> bool:
    return client is not None and bool(model)


-def _goal_mode_handoff_rejection(task, evidence: str) -> Optional[str]:
-    """Return a rejection reason when a goal-mode terminal handoff is premature."""
+def _goal_mode_handoff_rejection(task, evidence: str):
+    """Return ``(verdict, reason_or_None)`` for a goal-mode terminal handoff.
+
+    ``{"done", None}`` means the judge allows the handoff; anything else is
+    a rejection whose verdict disambiguates the guidance the caller gives
+    the worker (``continue`` = not done yet, ``blocked`` = judged
+    unachievable — see #100954).
+    """
    if not task or not task.goal_mode or not _goal_judge_available():
-        return None
+        return ("done", None)
    verdict = "done"
    reason = ""
    try:
@@ -270,7 +276,7 @@ def _goal_mode_handoff_rejection(task, evidence: str) -> Optional[str]:
            judge_exc,
            exc_info=True,
        )
-    return reason if verdict != "done" else None
+    return (verdict, None if verdict == "done" else reason)


 # ---------------------------------------------------------------------------
@@ -752,11 +758,19 @@ def _handle_complete(args: dict, **kw) -> str:
            # Only enforce when a judge is actually reachable — see
            # _goal_judge_available for why an unavailable judge fails open.
            task = kb.get_task(conn, tid)
-            rejection = _goal_mode_handoff_rejection(
+            gate_verdict, rejection = _goal_mode_handoff_rejection(
                task,
                (summary or result or "").strip(),
            )
-            if rejection is not None:
+            if gate_verdict == "blocked":
+                return tool_error(
+                    f"Goal completion rejected: judge ruled the goal "
+                    f"unachievable — {rejection}. The task will NOT complete "
+                    f"silently. Either re-scope the task with kanban_edit, "
+                    f"or record the block with kanban_block and hand the "
+                    f"decision to a human / reviewer."
+                )
+            if gate_verdict == "continue" or rejection is not None:
                return tool_error(
                    f"Goal completion rejected by judge: {rejection}. "
                    f"To proceed, either: (1) provide explicit acceptance "
@@ -937,8 +951,14 @@ def _handle_request_review(args: dict, **kw) -> str:
        kb, conn = _connect(board=board)
        try:
            task = kb.get_task(conn, tid)
-            rejection = _goal_mode_handoff_rejection(task, summary)
-            if rejection is not None:
+            gate_verdict, rejection = _goal_mode_handoff_rejection(task, summary)
+            if gate_verdict == "blocked":
+                return tool_error(
+                    f"Goal review handoff rejected: judge ruled the goal "
+                    f"unachievable — {rejection}. Record the block with "
+                    f"kanban_block instead of requesting review."
+                )
+            if gate_verdict == "continue" or rejection is not None:
                return tool_error(
                    f"Goal review handoff rejected by judge: {rejection}. "
                    "Provide acceptance evidence matching the card before "