mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-18 14:52:04 +00:00
fix(auxiliary): route direct-create aux callers through call_llm (#65029)
* 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.<task>.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.
This commit is contained in:
Teknium
GitHub
parent
306e2d2318
commit
7c954969b7
11 changed files with 141 additions and 241 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.<task>.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(
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.",
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue