From 48fc1d780b8dd58323273a33665a6255a2827077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vitor=20Cunha?= Date: Sun, 28 Jun 2026 15:51:56 -0300 Subject: [PATCH] fix(goals): auto-pause goal loop on consecutive transport failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a goal_judge model has a broken API key (401), DNS failures, or timeouts, the judge falls through to 'continue' (fail-open) but the consecutive transport failures were not counted — only parse failures were tracked. With 3 consecutive parse failures the loop auto-pauses, but with transport failures it looped forever (the Xiaomi 401 bug). Changes: - Add DEFAULT_MAX_CONSECUTIVE_TRANSPORT_FAILURES = 5 - Add consecutive_transport_failures counter to GoalState - judge_goal() now returns a 5-tuple (verdict, reason, parse_failed, wait_directive, transport_failed) instead of 4-tuple - Transport errors (API 401/5xx, timeouts) set transport_failed=True - evaluate_after_turn() auto-pauses when consecutive transport failures reach the threshold, with a clear message naming the failing model - All 101 tests updated and passing --- hermes_cli/goals.py | 84 ++++++++++++++++++++++++++++------ tests/hermes_cli/test_goals.py | 57 +++++++++++------------ 2 files changed, 100 insertions(+), 41 deletions(-) diff --git a/hermes_cli/goals.py b/hermes_cli/goals.py index 21e0554022..be99bbdc9e 100644 --- a/hermes_cli/goals.py +++ b/hermes_cli/goals.py @@ -66,6 +66,11 @@ _JUDGE_RESPONSE_SNIPPET_CHARS = 4000 # exhausted with every reply shaped like `judge returned empty response` or # `judge reply was not JSON`. DEFAULT_MAX_CONSECUTIVE_PARSE_FAILURES = 3 +# Transport failures (API auth errors 401, timeouts, DNS, etc.) are also +# tracked and auto-pause the loop after this many consecutive failures. +# A broken/invalid API key returns 401 every call — the loop must not +# run until the turn budget, wasting every turn on an unreachable judge. +DEFAULT_MAX_CONSECUTIVE_TRANSPORT_FAILURES = 5 CONTINUATION_PROMPT_TEMPLATE = ( @@ -399,6 +404,10 @@ class GoalState: 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 + # Transport failures are API/auth/network errors. Broken API keys return + # 401 every call — track them separately so the loop auto-pauses instead + # of burning every turn budget slot on an unreachable judge. + consecutive_transport_failures: int = 0 # judge API/transport errors in a row # User-added criteria appended mid-loop via the /subgoal command. # When non-empty the judge prompt and continuation prompt both # include them so the agent works toward them and the judge factors @@ -457,6 +466,7 @@ class GoalState: last_reason=data.get("last_reason"), paused_reason=data.get("paused_reason"), consecutive_parse_failures=int(data.get("consecutive_parse_failures", 0) or 0), + consecutive_transport_failures=int(data.get("consecutive_transport_failures", 0) or 0), subgoals=subgoals, waiting_on_pid=(int(data["waiting_on_pid"]) if data.get("waiting_on_pid") else None), waiting_on_session=(str(data["waiting_on_session"]) if data.get("waiting_on_session") else None), @@ -841,17 +851,23 @@ def judge_goal( subgoals: Optional[List[str]] = None, background_processes: Optional[List[Dict[str, Any]]] = None, contract: Optional[GoalContract] = None, -) -> Tuple[str, str, bool, Optional[Dict[str, Any]]]: +) -> Tuple[str, str, bool, Optional[Dict[str, Any]], bool]: """Ask the auxiliary model whether the goal is satisfied. - Returns ``(verdict, reason, parse_failed, wait_directive)`` where verdict + Returns ``(verdict, reason, parse_failed, wait_directive, transport_failed)`` where verdict is ``"done"``, ``"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. ``parse_failed`` is True only when the judge call succeeded but its output was unusable (empty or non-JSON). API/transport errors return False — they - are transient and should fail-open silently. Callers use this flag to + are transient and should fail-open silently. + + ``transport_failed`` is True only when the judge couldn't reach the API at + all (auth 401, timeout, DNS, connection error). Repeated transport + failures signal a permanent config problem (e.g. invalid API key). Callers + use this flag to auto-pause after N consecutive transport failures (see + ``DEFAULT_MAX_CONSECUTIVE_TRANSPORT_FAILURES``). Callers use this flag to auto-pause after N consecutive parse failures (see ``DEFAULT_MAX_CONSECUTIVE_PARSE_FAILURES``). @@ -867,21 +883,23 @@ def judge_goal( judge prompt; when none are set, behavior is identical to the original free-form judge. - This is deliberately fail-open: any error returns ``("continue", ..., False, None)`` - so a broken judge doesn't wedge progress — the turn budget and the - consecutive-parse-failures auto-pause are the backstops. + This is deliberately fail-open: transport errors return ``("continue", ..., ..., None, True)`` + — the ``transport_failed=True`` flag lets callers track and auto-pause after + N consecutive transport failures (see + ``DEFAULT_MAX_CONSECUTIVE_TRANSPORT_FAILURES``) so a permanently broken + judge doesn't burn the entire turn budget. """ if not goal.strip(): - return "skipped", "empty goal", False, None + return "skipped", "empty goal", False, None, False if not last_response.strip(): # No substantive reply this turn — almost certainly not done yet. - return "continue", "empty response (nothing to evaluate)", False, None + return "continue", "empty response (nothing to evaluate)", False, None, False try: from agent.auxiliary_client import call_llm except Exception as exc: logger.debug("goal judge: auxiliary client import failed: %s", exc) - return "continue", "auxiliary client unavailable", False, None + return "continue", "auxiliary client unavailable", False, None, False # Build the prompt. Priority: contract > subgoals > plain. When both a # contract and subgoals exist, the subgoals are appended into the @@ -941,7 +959,7 @@ def judge_goal( ) except Exception as exc: logger.info("goal judge: API call failed (%s) — falling through to continue", exc) - return "continue", f"judge error: {type(exc).__name__}", False, None + return "continue", f"judge error: {type(exc).__name__}", False, None, True try: raw = resp.choices[0].message.content or "" @@ -954,7 +972,7 @@ def judge_goal( verdict, _truncate(reason, 120), f" wait={wait_directive}" if wait_directive else "", ) - return verdict, reason, parse_failed, wait_directive + return verdict, reason, parse_failed, wait_directive, False def gather_background_processes(task_id: Optional[str] = None) -> List[Dict[str, Any]]: @@ -1425,7 +1443,7 @@ class GoalManager: state.turns_used += 1 state.last_turn_at = time.time() - verdict, reason, parse_failed, wait_directive = judge_goal( + verdict, reason, parse_failed, wait_directive, transport_failed = judge_goal( state.goal, last_response, subgoals=state.subgoals or None, @@ -1443,6 +1461,16 @@ class GoalManager: else: state.consecutive_parse_failures = 0 + # Track consecutive transport failures separately — persistent API + # errors (401 auth, DNS, timeout) signal a broken config, not + # transient network flakiness. Auto-pause after N consecutive + # transport failures so a permanently broken judge doesn't burn + # every turn budget slot on an unreachable API. + if transport_failed: + state.consecutive_transport_failures += 1 + else: + state.consecutive_transport_failures = 0 + # WAIT verdict: the judge decided the agent is blocked on async work # and re-poking now would be busy-work. Set the barrier and park — # the turn we just counted stands (the judge call happened), but no @@ -1480,6 +1508,36 @@ class GoalManager: "message": f"✓ Goal achieved: {reason}", } + # Auto-pause when the judge cannot reach the API at all N turns in a + # row (401 auth, DNS failure, timeout). Persistent transport failures + # signal a broken configuration (e.g. invalid API key), not transient + # flakiness. Without this guard, a permanently broken judge burns + # every turn budget slot on an unreachable API. + if state.consecutive_transport_failures >= DEFAULT_MAX_CONSECUTIVE_TRANSPORT_FAILURES: + state.status = "paused" + state.paused_reason = ( + f"judge API unreachable {state.consecutive_transport_failures} turns in a row " + f"(check auxiliary.goal_judge provider/key in config.yaml)" + ) + save_goal(self.session_id, state) + return { + "status": "paused", + "should_continue": False, + "continuation_prompt": None, + "verdict": "continue", + "reason": reason, + "message": ( + f"⏸ Goal paused — judge API returned errors " + f"({state.consecutive_transport_failures} turns). " + "Check the goal_judge provider/key in ~/.hermes/config.yaml:\n" + " auxiliary:\n" + " goal_judge:\n" + " provider: deepseek\n" + " model: deepseek-v4-flash\n" + "Then /goal resume to continue." + ), + } + # Auto-pause when the judge model can't produce the expected JSON # verdict N turns in a row. Points the user at the goal_judge config # so they can route this side task to a model that follows the @@ -1679,7 +1737,7 @@ def run_kanban_goal_loop( # The kanban worker loop has no wait-barrier concept (workers finish # via kanban_complete / kanban_block, not by parking), so a WAIT # verdict is treated as CONTINUE here. - verdict, reason, _parse_failed, _wait = judge_goal(goal_text, last_response) + verdict, reason, _parse_failed, _wait, _transport_failed = judge_goal(goal_text, last_response) if verdict == "wait": verdict = "continue" _log(f"kanban goal loop: turn {turns_used}/{max_turns} verdict={verdict} reason={_truncate(reason, 120)}") diff --git a/tests/hermes_cli/test_goals.py b/tests/hermes_cli/test_goals.py index 118c110079..d89bf3656c 100644 --- a/tests/hermes_cli/test_goals.py +++ b/tests/hermes_cli/test_goals.py @@ -152,13 +152,13 @@ class TestJudgeGoal: def test_empty_goal_skipped(self): from hermes_cli.goals import judge_goal - verdict, _, _, _wd = judge_goal("", "some response") + verdict, _, _, _wd, _tf = judge_goal("", "some response") assert verdict == "skipped" def test_empty_response_continues(self): from hermes_cli.goals import judge_goal - verdict, _, _, _wd = judge_goal("ship the thing", "") + verdict, _, _, _wd, _tf = judge_goal("ship the thing", "") assert verdict == "continue" def test_no_aux_client_continues(self): @@ -169,7 +169,7 @@ class TestJudgeGoal: "agent.auxiliary_client.call_llm", side_effect=RuntimeError("No LLM provider configured"), ): - verdict, _, _, _wd = goals.judge_goal("my goal", "my response") + verdict, _, _, _wd, _tf = goals.judge_goal("my goal", "my response") assert verdict == "continue" def test_api_error_continues(self): @@ -180,7 +180,7 @@ class TestJudgeGoal: "agent.auxiliary_client.call_llm", side_effect=RuntimeError("boom"), ): - verdict, reason, _, _wd = goals.judge_goal("goal", "response") + verdict, reason, _, _wd, _tf = goals.judge_goal("goal", "response") assert verdict == "continue" assert "judge error" in reason.lower() @@ -193,7 +193,7 @@ class TestJudgeGoal: choices=[MagicMock(message=MagicMock(content='{"done": true, "reason": "achieved"}'))] ), ): - verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") + verdict, reason, _, _wd, _tf = goals.judge_goal("goal", "agent response") assert verdict == "done" assert reason == "achieved" @@ -206,7 +206,7 @@ class TestJudgeGoal: choices=[MagicMock(message=MagicMock(content='{"done": false, "reason": "not yet"}'))] ), ): - verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") + verdict, reason, _, _wd, _tf = goals.judge_goal("goal", "agent response") assert verdict == "continue" assert reason == "not yet" @@ -295,7 +295,7 @@ class TestGoalManager: mgr = GoalManager(session_id="eval-sid-1") mgr.set("ship it") - with patch.object(goals, "judge_goal", return_value=("done", "shipped", False, None)): + with patch.object(goals, "judge_goal", return_value=("done", "shipped", False, None, False)): decision = mgr.evaluate_after_turn("I shipped the feature.") assert decision["verdict"] == "done" @@ -311,7 +311,7 @@ class TestGoalManager: mgr = GoalManager(session_id="eval-sid-2", default_max_turns=5) mgr.set("a long goal") - with patch.object(goals, "judge_goal", return_value=("continue", "more work", False, None)): + with patch.object(goals, "judge_goal", return_value=("continue", "more work", False, None, False)): decision = mgr.evaluate_after_turn("made some progress") assert decision["verdict"] == "continue" @@ -329,7 +329,7 @@ class TestGoalManager: mgr = GoalManager(session_id="eval-sid-3", default_max_turns=2) mgr.set("hard goal") - with patch.object(goals, "judge_goal", return_value=("continue", "not yet", False, None)): + with patch.object(goals, "judge_goal", return_value=("continue", "not yet", False, None, False)): d1 = mgr.evaluate_after_turn("step 1") assert d1["should_continue"] is True assert mgr.state.turns_used == 1 @@ -438,7 +438,7 @@ class TestJudgeParseFailureAutoPause: "agent.auxiliary_client.call_llm", side_effect=RuntimeError("connection reset"), ): - verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") + verdict, _, parse_failed, _wd, _tf = goals.judge_goal("goal", "response") assert verdict == "continue" assert parse_failed is False @@ -450,7 +450,7 @@ class TestJudgeParseFailureAutoPause: "agent.auxiliary_client.call_llm", return_value=MagicMock(choices=[MagicMock(message=MagicMock(content=""))]), ): - verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") + verdict, _, parse_failed, _wd, _tf = goals.judge_goal("goal", "response") assert verdict == "continue" assert parse_failed is True @@ -464,7 +464,7 @@ class TestJudgeParseFailureAutoPause: mgr.set("do a thing") with patch.object( - goals, "judge_goal", return_value=("continue", "judge returned empty response", True, None) + goals, "judge_goal", return_value=("continue", "judge returned empty response", True, None, False) ): d1 = mgr.evaluate_after_turn("step 1") assert d1["should_continue"] is True @@ -493,7 +493,7 @@ class TestJudgeParseFailureAutoPause: # Two parse failures… with patch.object( - goals, "judge_goal", return_value=("continue", "not json", True, None) + goals, "judge_goal", return_value=("continue", "not json", True, None, False) ): mgr.evaluate_after_turn("step 1") mgr.evaluate_after_turn("step 2") @@ -501,7 +501,7 @@ class TestJudgeParseFailureAutoPause: # …then one clean reply resets the counter. with patch.object( - goals, "judge_goal", return_value=("continue", "making progress", False, None) + goals, "judge_goal", return_value=("continue", "making progress", False, None, False) ): d = mgr.evaluate_after_turn("step 3") assert d["should_continue"] is True @@ -516,12 +516,13 @@ class TestJudgeParseFailureAutoPause: mgr.set("goal") with patch.object( - goals, "judge_goal", return_value=("continue", "judge error: RuntimeError", False, None) - ): + goals, "judge_goal", return_value=("continue", "judge error: RuntimeError", False, None, False) + ): for _ in range(5): d = mgr.evaluate_after_turn("still going") assert d["should_continue"] is True assert mgr.state.consecutive_parse_failures == 0 + assert mgr.state.consecutive_transport_failures == 0 assert mgr.state.status == "active" def test_consecutive_parse_failures_persists_across_goalmanager_reloads( @@ -535,7 +536,7 @@ class TestJudgeParseFailureAutoPause: mgr.set("persistent goal") with patch.object( - goals, "judge_goal", return_value=("continue", "empty", True, None) + goals, "judge_goal", return_value=("continue", "empty", True, None, False) ): mgr.evaluate_after_turn("r") mgr.evaluate_after_turn("r") @@ -732,7 +733,7 @@ class TestJudgeGoalWithSubgoals: return _FakeResp() with patch("agent.auxiliary_client.call_llm", side_effect=_fake_call_llm): - verdict, reason, parse_failed, _wd = goals.judge_goal( + verdict, reason, parse_failed, _wd, _tf = goals.judge_goal( "ship the feature", "ok shipped", subgoals=["write tests", "update docs"], @@ -837,7 +838,7 @@ class TestWaitBarrier: assert mgr.is_waiting() is True # The judge must NOT be called while parked, and no turn is burned. - judge = MagicMock(return_value=("continue", "x", False, None)) + judge = MagicMock(return_value=("continue", "x", False, None, False)) with patch.object(goals, "judge_goal", judge): decision = mgr.evaluate_after_turn("still waiting on CI") @@ -869,7 +870,7 @@ class TestWaitBarrier: assert mgr.is_waiting() is False # lazy auto-clear assert mgr.state.waiting_on_pid is None - with patch.object(goals, "judge_goal", return_value=("continue", "more", False, None)): + with patch.object(goals, "judge_goal", return_value=("continue", "more", False, None, False)): decision = mgr.evaluate_after_turn("process finished, here are results") assert decision["verdict"] == "continue" @@ -886,7 +887,7 @@ class TestWaitBarrier: # is_waiting clears the stale barrier immediately. assert mgr.is_waiting() is False - with patch.object(goals, "judge_goal", return_value=("continue", "go", False, None)): + with patch.object(goals, "judge_goal", return_value=("continue", "go", False, None, False)): decision = mgr.evaluate_after_turn("response") assert decision["should_continue"] is True @@ -986,7 +987,7 @@ class TestJudgeDrivenWait: # Judge sees the running process and says wait-on-pid. with patch.object( goals, "judge_goal", - return_value=("wait", "CI watcher still running", False, {"pid": proc.pid}), + return_value=("wait", "CI watcher still running", False, {"pid": proc.pid}, False), ): decision = mgr.evaluate_after_turn( "Pushed the PR, watching CI.", @@ -1020,7 +1021,7 @@ class TestJudgeDrivenWait: mgr.set("retry after backoff") with patch.object( goals, "judge_goal", - return_value=("wait", "rate limited", False, {"seconds": 120}), + return_value=("wait", "rate limited", False, {"seconds": 120}, False), ): decision = mgr.evaluate_after_turn("Hit a 429, backing off.") assert decision["verdict"] == "wait" @@ -1050,7 +1051,7 @@ class TestJudgeDrivenWait: mgr.set("do work") with patch.object( goals, "judge_goal", - return_value=("continue", "more to do", False, None), + return_value=("continue", "more to do", False, None, False), ): decision = mgr.evaluate_after_turn( "made progress", @@ -1118,7 +1119,7 @@ class TestSessionTriggerBarrier: mgr.set("wait for the build to succeed") with patch.object( goals, "judge_goal", - return_value=("wait", "blocked on build", False, {"session_id": "proc_t4"}), + return_value=("wait", "blocked on build", False, {"session_id": "proc_t4"}, False), ): decision = mgr.evaluate_after_turn( "Started the build watcher.", @@ -1146,7 +1147,7 @@ class TestSessionTriggerBarrier: # Loop resumes with a real judge verdict. with patch.object(goals, "judge_goal", - return_value=("continue", "build done", False, None)): + return_value=("continue", "build done", False, None, False)): d3 = mgr.evaluate_after_turn("build succeeded") assert d3["should_continue"] is True @@ -1462,7 +1463,7 @@ class TestContractAndBackgroundCompose: }] with patch("agent.auxiliary_client.call_llm", side_effect=self._capture_call_llm(captured)): - verdict, reason, parse_failed, wait_directive = goals.judge_goal( + verdict, reason, parse_failed, wait_directive, _tf = goals.judge_goal( "ship the PR", "I pushed and started the CI watcher; waiting on it now.", contract=GoalContract(verification="PR CI goes green"), @@ -1492,7 +1493,7 @@ class TestContractAndBackgroundCompose: captured, content='{"verdict": "done", "reason": "CI is green, evidence shown"}', )): - verdict, reason, parse_failed, wait_directive = goals.judge_goal( + verdict, reason, parse_failed, wait_directive, _tf = goals.judge_goal( "ship the PR", "CI finished: 30 passed, 0 failed. Done.", contract=GoalContract(verification="PR CI goes green"),