From b3c1b3b3f340f747d2a4a5363501256d04c56ccb Mon Sep 17 00:00:00 2001 From: beardthelion Date: Wed, 3 Jun 2026 13:45:14 -0500 Subject: [PATCH] fix(kanban): address review feedback on goal_mode judge gate Apply naqerl's review comments on PR #38388: - Hoist `from hermes_cli.goals import judge_goal` to module-level imports so an import failure surfaces at module init, not lazily on the first goal-mode completion (no circular import: hermes_cli package init is trivial and does not load tools.kanban_tools). - Narrow the fail-open `try` to wrap only the judge_goal() call. The verdict check and its rejection `return tool_error(...)` now live outside the handler, so a failure there can no longer be swallowed by the broad except. - Pass `exc_info=True` to the logger.warning call per CONTRIBUTING.md. Update the test mock target to tools.kanban_tools.judge_goal, since the hoisted import rebinds the name into this module's namespace. --- tests/tools/test_kanban_tools.py | 2 +- tools/kanban_tools.py | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/tools/test_kanban_tools.py b/tests/tools/test_kanban_tools.py index fdcbb568466..3630c8b49da 100644 --- a/tests/tools/test_kanban_tools.py +++ b/tests/tools/test_kanban_tools.py @@ -622,7 +622,7 @@ def test_complete_goal_mode_rejected_by_judge(monkeypatch, tmp_path): def mock_judge_goal(goal, last_response, *, timeout=30.0, subgoals=None): return "continue", "missing verification evidence", False - monkeypatch.setattr("hermes_cli.goals.judge_goal", mock_judge_goal) + monkeypatch.setattr("tools.kanban_tools.judge_goal", mock_judge_goal) # Attempt to complete should be rejected out = kt._handle_complete({"summary": "I did some stuff but not X"}) diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 96b49cc01f7..3b100cf2563 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -34,6 +34,7 @@ import os from typing import Any, Optional from agent.redact import redact_sensitive_text +from hermes_cli.goals import judge_goal from tools.registry import registry, tool_error from hermes_cli.config import cfg_get, load_config @@ -572,26 +573,28 @@ def _handle_complete(args: dict, **kw) -> str: # calling kanban_complete before acceptance criteria are met. task = kb.get_task(conn, tid) if task and task.goal_mode: + verdict = "done" + reason = "" try: - from hermes_cli.goals import judge_goal verdict, reason, _ = judge_goal( goal=f"{task.title}\n\n{task.body or ''}".strip(), last_response=(summary or result or "").strip(), ) - if verdict != "done": - return tool_error( - f"Goal completion rejected by judge: {reason}. " - f"To proceed, either: (1) provide explicit acceptance " - f"evidence in your summary matching the task's criteria, " - f"or (2) create continuation tasks with parent={tid} " - f"and keep this task alive." - ) except Exception as judge_exc: # Fail-open to avoid wedging the worker if the judge # is temporarily unavailable or misconfigured. logger.warning( "goal judge check failed, allowing completion: %s", judge_exc, + exc_info=True, + ) + if verdict != "done": + return tool_error( + f"Goal completion rejected by judge: {reason}. " + f"To proceed, either: (1) provide explicit acceptance " + f"evidence in your summary matching the task's criteria, " + f"or (2) create continuation tasks with parent={tid} " + f"and keep this task alive." ) try: