From 7c954969b70bb75a24a1d16bbe7faeb8705a6f42 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:39:17 -0700 Subject: [PATCH] fix(auxiliary): route direct-create aux callers through call_llm (#65029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auxiliary): route direct-create aux callers through call_llm (#35566) Five callers (kanban_decompose, kanban_specify, profile_describer, and goals.py's judge + draft-contract) built raw clients via get_text_auxiliary_client() and passed extra_body=get_auxiliary_extra_body() — which only returns Nous portal tags and ignores auxiliary..extra_body from config.yaml entirely. That was the remaining half of #35566 after the call_llm path was fixed. Routing them through call_llm(task=...) gives each caller the full auxiliary contract for free: task extra_body, the reasoning_effort shorthand, transient retries, provider-profile projection, and fallback chains. goal_judge gains a DEFAULT_CONFIG block (it had none — its provider/model overrides silently didn't exist as documented keys). get_auxiliary_extra_body() now has zero non-test callers; kept for plugin back-compat. Fixes #35566. * test: migrate kanban dashboard + CLI specify mocks to call_llm Two more consumers of specify_task mocked the old get_text_auxiliary_client symbol (missed in the first sibling sweep — they live outside tests/hermes_cli's kanban files): the dashboard plugin's /specify endpoint tests and the /kanban slash-command E2E. Same migration as the rest: mock call_llm at the source, no-provider now surfaces via the LLM-error branch. --- hermes_cli/config.py | 12 ++ hermes_cli/goals.py | 36 +--- hermes_cli/kanban_decompose.py | 23 +-- hermes_cli/kanban_specify.py | 21 +-- hermes_cli/profile_describer.py | 22 +-- tests/hermes_cli/test_goals.py | 176 ++++++------------ tests/hermes_cli/test_kanban_cli.py | 7 +- tests/hermes_cli/test_kanban_decompose.py | 24 ++- tests/hermes_cli/test_kanban_specify.py | 29 ++- tests/hermes_cli/test_profile_describer.py | 8 +- tests/plugins/test_kanban_dashboard_plugin.py | 24 ++- 11 files changed, 141 insertions(+), 241 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index a47ba24eaefa..45cee2071232 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1700,6 +1700,18 @@ DEFAULT_CONFIG = { "extra_body": {}, "reasoning_effort": "", # per-task thinking level: none|minimal|low|medium|high|xhigh|max|ultra (empty = provider default) }, + # Goal judge — evaluates whether a /goal run's latest response + # satisfies the goal/contract, and drafts goal contracts. Short + # structured-JSON calls; a fast cheap model is fine. + "goal_judge": { + "provider": "auto", + "model": "", + "base_url": "", + "api_key": "", + "timeout": 60, + "extra_body": {}, + "reasoning_effort": "", # per-task thinking level: none|minimal|low|medium|high|xhigh|max|ultra (empty = provider default) + }, # Curator — skill-usage review fork. Timeout is generous because the # review pass can take several minutes on reasoning models (umbrella # building over hundreds of candidate skills). "auto" = use main chat diff --git a/hermes_cli/goals.py b/hermes_cli/goals.py index 3a1e869308ab..21e055402293 100644 --- a/hermes_cli/goals.py +++ b/hermes_cli/goals.py @@ -878,20 +878,11 @@ def judge_goal( return "continue", "empty response (nothing to evaluate)", False, None try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + 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 - try: - client, model = get_text_auxiliary_client("goal_judge") - except Exception as exc: - logger.debug("goal judge: get_text_auxiliary_client failed: %s", exc) - return "continue", "auxiliary client unavailable", False, None - - if client is None or not model: - return "continue", "no auxiliary client configured", False, None - # Build the prompt. Priority: contract > subgoals > plain. When both a # contract and subgoals exist, the subgoals are appended into the # contract block as extra criteria so the judge sees a single source of @@ -935,8 +926,11 @@ def judge_goal( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.goal_judge.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="goal_judge", messages=[ {"role": "system", "content": JUDGE_SYSTEM_PROMPT}, {"role": "user", "content": prompt}, @@ -944,7 +938,6 @@ def judge_goal( temperature=0, max_tokens=_goal_judge_max_tokens(), timeout=timeout, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("goal judge: API call failed (%s) — falling through to continue", exc) @@ -999,23 +992,15 @@ def draft_contract(objective: str, *, timeout: float = DEFAULT_JUDGE_TIMEOUT) -> return None try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + from agent.auxiliary_client import call_llm except Exception as exc: logger.debug("goal draft: auxiliary client import failed: %s", exc) return None try: - client, model = get_text_auxiliary_client("goal_judge") - except Exception as exc: - logger.debug("goal draft: get_text_auxiliary_client failed: %s", exc) - return None - - if client is None or not model: - return None - - try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm — same #35566 fix as the judge call above. + resp = call_llm( + task="goal_judge", messages=[ {"role": "system", "content": DRAFT_CONTRACT_SYSTEM_PROMPT}, {"role": "user", "content": f"Objective:\n{_truncate(objective, 4000)}"}, @@ -1023,7 +1008,6 @@ def draft_contract(objective: str, *, timeout: float = DEFAULT_JUDGE_TIMEOUT) -> temperature=0, max_tokens=_goal_judge_max_tokens(), timeout=timeout, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("goal draft: API call failed (%s)", exc) diff --git a/hermes_cli/kanban_decompose.py b/hermes_cli/kanban_decompose.py index 8af2da9114fe..d6c248c658a6 100644 --- a/hermes_cli/kanban_decompose.py +++ b/hermes_cli/kanban_decompose.py @@ -298,23 +298,11 @@ def decompose_task( roster, valid_names = _build_roster() try: - from agent.auxiliary_client import ( # type: ignore - get_auxiliary_extra_body, - get_text_auxiliary_client, - ) + from agent.auxiliary_client import call_llm # type: ignore except Exception as exc: logger.debug("decompose: auxiliary client import failed: %s", exc) return DecomposeOutcome(task_id, False, "auxiliary client unavailable") - try: - client, model = get_text_auxiliary_client("kanban_decomposer") - except Exception as exc: - logger.debug("decompose: get_text_auxiliary_client failed: %s", exc) - return DecomposeOutcome(task_id, False, "auxiliary client unavailable") - - if client is None or not model: - return DecomposeOutcome(task_id, False, "no auxiliary client configured") - user_msg = _USER_TEMPLATE.format( task_id=task.id, title=_truncate(task.title or "", 400), @@ -324,8 +312,12 @@ def decompose_task( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.kanban_decomposer.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the previous direct client.chat.completions.create() + # path dropped auxiliary..extra_body entirely (#35566). + resp = call_llm( + task="kanban_decomposer", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -333,7 +325,6 @@ def decompose_task( temperature=0.3, max_tokens=4000, timeout=timeout or 180, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info( diff --git a/hermes_cli/kanban_specify.py b/hermes_cli/kanban_specify.py index 40812d835f06..e7aba34a3717 100644 --- a/hermes_cli/kanban_specify.py +++ b/hermes_cli/kanban_specify.py @@ -162,22 +162,11 @@ def specify_task( ) try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + from agent.auxiliary_client import call_llm except Exception as exc: # pragma: no cover — import smoke test logger.debug("specify: auxiliary client import failed: %s", exc) return SpecifyOutcome(task_id, False, "auxiliary client unavailable") - try: - client, model = get_text_auxiliary_client("triage_specifier") - except Exception as exc: - logger.debug("specify: get_text_auxiliary_client failed: %s", exc) - return SpecifyOutcome(task_id, False, "auxiliary client unavailable") - - if client is None or not model: - return SpecifyOutcome( - task_id, False, "no auxiliary client configured" - ) - user_msg = _USER_TEMPLATE.format( task_id=task.id, title=_truncate(task.title or "", 400), @@ -185,8 +174,11 @@ def specify_task( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.triage_specifier.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="triage_specifier", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -194,7 +186,6 @@ def specify_task( temperature=0.3, max_tokens=HERMES_KANBAN_SPECIFY_MAX_TOKENS, timeout=timeout or 120, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info( diff --git a/hermes_cli/profile_describer.py b/hermes_cli/profile_describer.py index f80d1f5451e3..c6af27ae227f 100644 --- a/hermes_cli/profile_describer.py +++ b/hermes_cli/profile_describer.py @@ -210,23 +210,11 @@ def describe_profile( model, provider = None, None try: - from agent.auxiliary_client import ( # type: ignore - get_auxiliary_extra_body, - get_text_auxiliary_client, - ) + from agent.auxiliary_client import call_llm # type: ignore except Exception as exc: logger.debug("describe: auxiliary client import failed: %s", exc) return DescribeOutcome(canon, False, "auxiliary client unavailable") - try: - client, aux_model = get_text_auxiliary_client("profile_describer") - except Exception as exc: - logger.debug("describe: get_text_auxiliary_client failed: %s", exc) - return DescribeOutcome(canon, False, "auxiliary client unavailable") - - if client is None or not aux_model: - return DescribeOutcome(canon, False, "no auxiliary client configured") - user_msg = _USER_TEMPLATE.format( name=canon, model=(model or "(unset)"), @@ -237,8 +225,11 @@ def describe_profile( ) try: - resp = client.chat.completions.create( - model=aux_model, + # Route through call_llm so auxiliary.profile_describer.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="profile_describer", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -246,7 +237,6 @@ def describe_profile( temperature=0.3, max_tokens=400, timeout=timeout or 60, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("describe: API call failed for %s (%s)", canon, exc) diff --git a/tests/hermes_cli/test_goals.py b/tests/hermes_cli/test_goals.py index b6ae1abcda53..118c110079cc 100644 --- a/tests/hermes_cli/test_goals.py +++ b/tests/hermes_cli/test_goals.py @@ -166,8 +166,8 @@ class TestJudgeGoal: from hermes_cli import goals with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, None), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): verdict, _, _, _wd = goals.judge_goal("my goal", "my response") assert verdict == "continue" @@ -176,11 +176,9 @@ class TestJudgeGoal: """Judge exception → fail-open continue (don't wedge progress on judge bugs).""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.side_effect = RuntimeError("boom") with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("boom"), ): verdict, reason, _, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -189,17 +187,11 @@ class TestJudgeGoal: def test_judge_says_done(self): from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[ - MagicMock( - message=MagicMock(content='{"done": true, "reason": "achieved"}') - ) - ] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock( + choices=[MagicMock(message=MagicMock(content='{"done": true, "reason": "achieved"}'))] + ), ): verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") assert verdict == "done" @@ -208,17 +200,11 @@ class TestJudgeGoal: def test_judge_says_continue(self): from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[ - MagicMock( - message=MagicMock(content='{"done": false, "reason": "not yet"}') - ) - ] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock( + choices=[MagicMock(message=MagicMock(content='{"done": false, "reason": "not yet"}'))] + ), ): verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") assert verdict == "continue" @@ -448,11 +434,9 @@ class TestJudgeParseFailureAutoPause: """Transient network/API errors must not trip the auto-pause guard.""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.side_effect = RuntimeError("connection reset") with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("connection reset"), ): verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -462,13 +446,9 @@ class TestJudgeParseFailureAutoPause: """End-to-end: judge returns empty content → parse_failed=True.""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[MagicMock(message=MagicMock(content=""))] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock(choices=[MagicMock(message=MagicMock(content=""))]), ): verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -747,22 +727,11 @@ class TestJudgeGoalWithSubgoals: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() + def _fake_call_llm(**kwargs): + captured.update(kwargs) + return _FakeResp() - with patch.object(goals, "get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model"), create=True), \ - patch.object(goals, "get_auxiliary_extra_body", - return_value=None, create=True), \ - patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", - return_value=None): + with patch("agent.auxiliary_client.call_llm", side_effect=_fake_call_llm): verdict, reason, parse_failed, _wd = goals.judge_goal( "ship the feature", "ok shipped", @@ -790,18 +759,11 @@ class TestJudgeGoalWithSubgoals: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() + def _fake_call_llm(**kwargs): + captured.update(kwargs) + return _FakeResp() - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", - return_value=None): + with patch("agent.auxiliary_client.call_llm", side_effect=_fake_call_llm): goals.judge_goal("ship it", "done", subgoals=None) sent_messages = captured.get("messages") or [] @@ -1363,7 +1325,8 @@ class TestGoalManagerContract: class TestJudgeWithContract: - def _fake_client(self, captured, content='{"done": false, "reason": "more"}'): + def _fake_call_llm(self, captured, content='{"done": false, "reason": "more"}'): + """judge_goal routes through call_llm (#35566) — capture its kwargs.""" class _FakeMsg: pass _FakeMsg.content = content @@ -1371,14 +1334,11 @@ class TestJudgeWithContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() - return _FakeClient + + def _fake(**kwargs): + captured.update(kwargs) + return _FakeResp() + return _fake def test_judge_uses_contract_template(self, hermes_home): from unittest.mock import patch @@ -1386,10 +1346,8 @@ class TestJudgeWithContract: from hermes_cli.goals import GoalContract captured = {} - client = self._fake_client(captured) - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._fake_call_llm(captured)): goals.judge_goal( "ship it", "I think it's done", contract=GoalContract(verification="pytest -q passes"), @@ -1407,10 +1365,8 @@ class TestJudgeWithContract: from hermes_cli.goals import GoalContract captured = {} - client = self._fake_client(captured) - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._fake_call_llm(captured)): goals.judge_goal( "ship it", "done", subgoals=["write changelog"], @@ -1438,16 +1394,8 @@ class TestDraftContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - return _FakeResp() - - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + return_value=_FakeResp()): contract = goals.draft_contract("Migrate auth to JWT") assert contract is not None assert contract.outcome == "auth on JWT" @@ -1464,24 +1412,16 @@ class TestDraftContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - return _FakeResp() - - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + return_value=_FakeResp()): assert goals.draft_contract("anything") is None def test_draft_returns_none_when_no_client(self, hermes_home): from unittest.mock import patch from hermes_cli import goals - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, None)): + with patch("agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured")): assert goals.draft_contract("anything") is None @@ -1495,7 +1435,8 @@ class TestContractAndBackgroundCompose: the contract block and the background-process list to the judge, so it can return either done (evidence met) or wait (parked on the poller).""" - def _capture_client(self, captured, content='{"verdict": "wait", "wait_on_pid": 4242, "reason": "CI still running"}'): + def _capture_call_llm(self, captured, content='{"verdict": "wait", "wait_on_pid": 4242, "reason": "CI still running"}'): + """judge_goal routes through call_llm (#35566) — capture its kwargs.""" class _FakeMsg: pass _FakeMsg.content = content @@ -1503,14 +1444,11 @@ class TestContractAndBackgroundCompose: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() - return _FakeClient + + def _fake(**kwargs): + captured.update(kwargs) + return _FakeResp() + return _fake def test_judge_prompt_carries_contract_and_background(self, hermes_home): from unittest.mock import patch @@ -1518,14 +1456,12 @@ class TestContractAndBackgroundCompose: from hermes_cli.goals import GoalContract captured = {} - client = self._capture_client(captured) bg = [{ "session_id": "ci-watch", "pid": 4242, "status": "running", "command": "wait_for_pr_green.sh 50501", "trigger": "exit", }] - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._capture_call_llm(captured)): verdict, reason, parse_failed, wait_directive = goals.judge_goal( "ship the PR", "I pushed and started the CI watcher; waiting on it now.", @@ -1550,14 +1486,12 @@ class TestContractAndBackgroundCompose: from hermes_cli.goals import GoalContract captured = {} - client = self._capture_client( - captured, - content='{"verdict": "done", "reason": "CI is green, evidence shown"}', - ) bg = [{"session_id": "ci", "pid": 4242, "status": "running", "command": "ci", "trigger": "exit"}] - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._capture_call_llm( + captured, + content='{"verdict": "done", "reason": "CI is green, evidence shown"}', + )): verdict, reason, parse_failed, wait_directive = goals.judge_goal( "ship the PR", "CI finished: 30 passed, 0 failed. Done.", diff --git a/tests/hermes_cli/test_kanban_cli.py b/tests/hermes_cli/test_kanban_cli.py index c59578c4b6ae..6433635c28c4 100644 --- a/tests/hermes_cli/test_kanban_cli.py +++ b/tests/hermes_cli/test_kanban_cli.py @@ -465,11 +465,10 @@ def test_run_slash_specify_end_to_end(kanban_home, monkeypatch): resp.choices[0].message.content = ( '{"title": "Spec: rough idea", "body": "**Goal**\\nShip it."}' ) - fake_client = MagicMock() - fake_client.chat.completions.create = MagicMock(return_value=resp) + # specify_task routes through call_llm now (#35566) — mock it directly. monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (fake_client, "test-model"), + "agent.auxiliary_client.call_llm", + MagicMock(return_value=resp), ) # Specify via slash. diff --git a/tests/hermes_cli/test_kanban_decompose.py b/tests/hermes_cli/test_kanban_decompose.py index 5ba17e58caef..7a872a4dac85 100644 --- a/tests/hermes_cli/test_kanban_decompose.py +++ b/tests/hermes_cli/test_kanban_decompose.py @@ -41,18 +41,19 @@ def _mock_client_returning(content: str): def _patch_aux_client(content: str, *, model: str = "test-model"): - client = _mock_client_returning(content) + # decompose_task now routes through call_llm (see #35566) — mock it at + # the source module so task config, extra_body, and retries stay out of + # unit-test scope. return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, model), + "agent.auxiliary_client.call_llm", + return_value=_fake_aux_response(content), ) def _patch_extra_body(): - return patch( - "agent.auxiliary_client.get_auxiliary_extra_body", - return_value={}, - ) + # No-op shim retained for call-site compatibility: extra_body plumbing + # now lives inside call_llm, which _patch_aux_client already mocks. + return patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value={}) def _patch_list_profiles(names: list[str]): @@ -334,9 +335,11 @@ def test_decompose_no_aux_client_configured(kanban_home): for p in patches: p.start() try: + # call_llm raises RuntimeError when no provider is configured; the + # decomposer must convert that into a failed outcome, not a crash. with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): outcome = decomp.decompose_task(tid, author="me") finally: @@ -344,4 +347,5 @@ def test_decompose_no_aux_client_configured(kanban_home): p.stop() assert outcome.ok is False - assert "no auxiliary client" in outcome.reason + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in outcome.reason diff --git a/tests/hermes_cli/test_kanban_specify.py b/tests/hermes_cli/test_kanban_specify.py index dd377001590a..8b9c9f5213e4 100644 --- a/tests/hermes_cli/test_kanban_specify.py +++ b/tests/hermes_cli/test_kanban_specify.py @@ -48,15 +48,12 @@ def _mock_client_returning(content: str): def _patch_aux_client(content: str, *, model: str = "test-model"): - """Patch get_text_auxiliary_client at its source + at the module that - imported it lazily inside specify_task. Both patches are needed - because kanban_specify imports the function inside the function body. + """Patch call_llm at its source module — specify_task now routes through + it (#35566) instead of building a raw client. Returns (patcher, mock) so + callers can still assert on the call. """ - client = _mock_client_returning(content) - return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, model), - ), client + mock_fn = MagicMock(return_value=_fake_aux_response(content)) + return patch("agent.auxiliary_client.call_llm", mock_fn), mock_fn # --------------------------------------------------------------------------- @@ -159,13 +156,14 @@ def test_specify_task_no_aux_client_configured(kanban_home): tid = kb.create_task(conn, title="rough", triage=True) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): outcome = spec.specify_task(tid) assert outcome.ok is False - assert "auxiliary client" in outcome.reason + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in outcome.reason # Task must stay in triage — we never touched it. with kb.connect() as conn: assert kb.get_task(conn, tid).status == "triage" @@ -176,10 +174,9 @@ def test_specify_task_llm_api_error_keeps_task_in_triage(kanban_home): tid = kb.create_task(conn, title="rough", triage=True) client = MagicMock() - client.chat.completions.create = MagicMock(side_effect=RuntimeError("429 rate limited")) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "test-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("429 rate limited"), ): outcome = spec.specify_task(tid) @@ -288,8 +285,8 @@ def test_cli_specify_all_returns_1_when_every_task_fails(kanban_home, capsys): kb.create_task(conn, title="b", triage=True) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), # no aux client → every task fails + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), # every task fails ): rc = _run_cli("specify", "--all") diff --git a/tests/hermes_cli/test_profile_describer.py b/tests/hermes_cli/test_profile_describer.py index 3fc5fa3a6be3..e1a55a648d0a 100644 --- a/tests/hermes_cli/test_profile_describer.py +++ b/tests/hermes_cli/test_profile_describer.py @@ -79,11 +79,11 @@ def _fake_aux_response(content: str): def _patch_aux_client(content: str): - client = MagicMock() - client.chat.completions.create = MagicMock(return_value=_fake_aux_response(content)) + # describe_profile now routes through call_llm (#35566) — mock it at the + # source module. return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "test-model"), + "agent.auxiliary_client.call_llm", + return_value=_fake_aux_response(content), ) diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index 4c7f466ce8f3..2cf207d7d91c 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -2182,13 +2182,10 @@ def _patch_specifier_response(monkeypatch, *, content, model="test-model"): resp = MagicMock() resp.choices = [MagicMock()] resp.choices[0].message.content = content - fake_client = MagicMock() - fake_client.chat.completions.create = MagicMock(return_value=resp) - monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (fake_client, model), - ) - return fake_client + # specify_task routes through call_llm now (#35566) — mock it directly. + fake_call = MagicMock(return_value=resp) + monkeypatch.setattr("agent.auxiliary_client.call_llm", fake_call) + return fake_call def test_specify_happy_path(client, monkeypatch): @@ -2250,11 +2247,11 @@ def test_specify_no_aux_client_surfaces_reason(client, monkeypatch): json={"title": "rough", "triage": True}, ).json()["task"] - # Simulate "no auxiliary client configured". - monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (None, ""), - ) + # Simulate "no auxiliary client configured" — call_llm raises when + # no provider resolves (#35566 routing). + def _no_provider(**kwargs): + raise RuntimeError("No LLM provider configured") + monkeypatch.setattr("agent.auxiliary_client.call_llm", _no_provider) r = client.post( f"/api/plugins/kanban/tasks/{t['id']}/specify", @@ -2263,7 +2260,8 @@ def test_specify_no_aux_client_surfaces_reason(client, monkeypatch): assert r.status_code == 200 body = r.json() assert body["ok"] is False - assert "auxiliary client" in body["reason"] + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in body["reason"] # Task must stay in triage — nothing was touched. detail = client.get(f"/api/plugins/kanban/tasks/{t['id']}").json()["task"]