diff --git a/agent/background_review.py b/agent/background_review.py index 3f4e5efcd37..bf78f679236 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -62,6 +62,11 @@ def _resolve_review_runtime(agent: Any) -> Dict[str, Any]: "api_key": parent_runtime.get("api_key") or None, "base_url": parent_runtime.get("base_url") or None, "api_mode": parent_api_mode, + "credential_pool": getattr(agent, "_credential_pool", None), + "request_overrides": dict(getattr(agent, "request_overrides", {}) or {}), + "max_tokens": getattr(agent, "max_tokens", None), + "command": getattr(agent, "acp_command", None), + "args": list(getattr(agent, "acp_args", []) or []), "routed": False, } try: @@ -89,10 +94,15 @@ def _resolve_review_runtime(agent: Any) -> Dict[str, Any]: ) return { "provider": rp.get("provider") or task_provider, - "model": task_model, + "model": rp.get("model") or task_model, "api_key": rp.get("api_key"), "base_url": rp.get("base_url"), "api_mode": rp.get("api_mode"), + "credential_pool": rp.get("credential_pool"), + "request_overrides": dict(rp.get("request_overrides") or {}), + "max_tokens": rp.get("max_output_tokens"), + "command": rp.get("command"), + "args": list(rp.get("args") or []), "routed": True, } except Exception as e: @@ -680,6 +690,12 @@ def _run_review_in_thread( # Match parent's toolset config so ``tools[]`` is byte-identical # in the request body — Anthropic's cache key includes it. # (The runtime whitelist below still restricts dispatch.) + _fork_kwargs: Dict[str, Any] = {} + if isinstance(_rt.get("max_tokens"), int): + _fork_kwargs["max_tokens"] = _rt["max_tokens"] + if isinstance(_rt.get("command"), str) and _rt["command"]: + _fork_kwargs["acp_command"] = _rt["command"] + _fork_kwargs["acp_args"] = _rt.get("args") or [] review_agent = AIAgent( model=_rt.get("model") or agent.model, max_iterations=16, @@ -689,11 +705,13 @@ def _run_review_in_thread( api_mode=_rt.get("api_mode"), base_url=_rt.get("base_url") or None, api_key=_rt.get("api_key") or None, - credential_pool=getattr(agent, "_credential_pool", None), + credential_pool=_rt.get("credential_pool"), + request_overrides=_rt.get("request_overrides") or {}, parent_session_id=agent.session_id, enabled_toolsets=getattr(agent, "enabled_toolsets", None), disabled_toolsets=getattr(agent, "disabled_toolsets", None), skip_memory=True, + **_fork_kwargs, ) review_agent._memory_write_origin = "background_review" review_agent._memory_write_context = "background_review" diff --git a/agent/curator.py b/agent/curator.py index df5c5c99fb0..ada93248e24 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -45,12 +45,26 @@ def _strip_aux_credential(value: Any) -> Optional[str]: class _ReviewRuntimeBinding(NamedTuple): - """Provider/model for the curator review fork plus optional per-slot overrides.""" + """Provider/model for the curator review fork plus per-slot overrides.""" provider: str model: str explicit_api_key: Optional[str] explicit_base_url: Optional[str] + request_overrides: Dict[str, Any] + + +def _merge_request_overrides( + runtime_overrides: Any, + slot_extra_body: Any, +) -> Dict[str, Any]: + """Merge resolver metadata with task-local request body fields.""" + merged = dict(runtime_overrides or {}) + if isinstance(slot_extra_body, dict) and slot_extra_body: + extra_body = dict(merged.get("extra_body") or {}) + extra_body.update(slot_extra_body) + merged["extra_body"] = extra_body + return merged DEFAULT_INTERVAL_HOURS = 24 * 7 # 7 days @@ -1764,6 +1778,7 @@ def _resolve_review_runtime(cfg: Dict[str, Any]) -> _ReviewRuntimeBinding: _task_model, _strip_aux_credential(_cur_task.get("api_key")), _strip_aux_credential(_cur_task.get("base_url")), + _merge_request_overrides({}, _cur_task.get("extra_body")), ) # 2. Legacy curator.auxiliary.{provider,model} (deprecated, pre-unification) @@ -1781,10 +1796,11 @@ def _resolve_review_runtime(cfg: Dict[str, Any]) -> _ReviewRuntimeBinding: str(_legacy_model), _strip_aux_credential(_legacy.get("api_key")), _strip_aux_credential(_legacy.get("base_url")), + _merge_request_overrides({}, _legacy.get("extra_body")), ) # 3. Fall through to the main chat model - return _ReviewRuntimeBinding(_main_provider, _main_model, None, None) + return _ReviewRuntimeBinding(_main_provider, _main_model, None, None, {}) def _resolve_review_model(cfg: Dict[str, Any]) -> tuple[str, str]: @@ -1851,7 +1867,10 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: _api_mode = None _resolved_provider = None _credential_pool = None - _request_overrides = None + _request_overrides: Dict[str, Any] = {} + _max_tokens = None + _acp_command = None + _acp_args = None _model_name = "" try: from hermes_cli.config import load_config @@ -1870,7 +1889,15 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: _api_mode = _rp.get("api_mode") _resolved_provider = _rp.get("provider") or _provider _credential_pool = _rp.get("credential_pool") - _request_overrides = _rp.get("request_overrides") + _request_overrides = _merge_request_overrides( + _rp.get("request_overrides"), + _binding.request_overrides.get("extra_body"), + ) + _max_tokens = _rp.get("max_output_tokens") + _acp_command = _rp.get("command") + _acp_args = list(_rp.get("args") or []) + if isinstance(_rp.get("model"), str) and _rp["model"].strip(): + _model_name = _rp["model"].strip() except Exception as e: logger.debug("Curator provider resolution failed: %s", e, exc_info=True) @@ -1879,6 +1906,12 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: review_agent = None try: + _agent_kwargs: Dict[str, Any] = {} + if isinstance(_max_tokens, int): + _agent_kwargs["max_tokens"] = _max_tokens + if isinstance(_acp_command, str) and _acp_command: + _agent_kwargs["acp_command"] = _acp_command + _agent_kwargs["acp_args"] = _acp_args or [] review_agent = AIAgent( model=_model_name, provider=_resolved_provider, @@ -1887,6 +1920,7 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: api_mode=_api_mode, credential_pool=_credential_pool, request_overrides=_request_overrides, + **_agent_kwargs, # Umbrella-building over a large skill collection is worth a # high iteration ceiling — the pass typically takes 50-100 # API calls against hundreds of candidate skills. The diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index 75688b6bdea..f0630d7850c 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -1074,6 +1074,25 @@ def test_review_runtime_strips_blank_aux_credentials(curator_env): assert binding.explicit_base_url is None +def test_review_runtime_carries_auxiliary_extra_body(curator_env): + curator = curator_env["curator"] + cfg = { + "auxiliary": { + "curator": { + "provider": "custom", + "model": "local-mini", + "extra_body": {"slot_flag": "slot-value"}, + }, + }, + } + + binding = curator._resolve_review_runtime(cfg) + + assert binding.request_overrides == { + "extra_body": {"slot_flag": "slot-value"} + } + + def test_review_runtime_ignores_auxiliary_credentials_when_using_main(curator_env): """Falling through to main model must not pick up stray auxiliary.curator secrets.""" curator = curator_env["curator"] @@ -1324,3 +1343,102 @@ def test_review_fork_forwards_runtime_pool_and_overrides(curator_env, monkeypatc assert meta.get("error") is None, meta.get("error") assert captured["kwargs"]["credential_pool"] is fake_pool assert captured["kwargs"]["request_overrides"] == fake_overrides + + +def test_review_fork_uses_runtime_model_and_output_cap(curator_env, monkeypatch): + curator = curator_env["curator"] + import importlib + importlib.reload(curator) + captured = {} + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"model": {"provider": "custom:gateway", "default": "gateway"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "provider": "custom", + "model": "real-model-id", + "api_key": "test-key", + "base_url": "https://gateway.example/v1", + "api_mode": "chat_completions", + "max_output_tokens": 1234, + }, + ) + + class _StubAgent: + def __init__(self, **kwargs): + captured.update(kwargs) + self._session_messages = [] + + def run_conversation(self, **_kwargs): + return {"final_response": "ok"} + + def close(self): + pass + + monkeypatch.setattr("run_agent.AIAgent", _StubAgent) + result = curator._run_llm_review("review") + + assert result["error"] is None + assert captured["model"] == "real-model-id" + assert captured["max_tokens"] == 1234 + + +def test_review_fork_merges_slot_extra_body_over_runtime(curator_env, monkeypatch): + curator = curator_env["curator"] + import importlib + importlib.reload(curator) + captured = {} + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: { + "auxiliary": { + "curator": { + "provider": "custom:gateway", + "model": "gateway", + "extra_body": {"shared": "slot", "slot_only": True}, + }, + }, + }, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "provider": "custom", + "api_key": "test-key", + "base_url": "https://gateway.example/v1", + "api_mode": "chat_completions", + "request_overrides": { + "extra_body": {"shared": "runtime", "runtime_only": True}, + "service_tier": "priority", + }, + }, + ) + + class _StubAgent: + def __init__(self, **kwargs): + captured.update(kwargs) + self._session_messages = [] + + def run_conversation(self, **_kwargs): + return {"final_response": "ok"} + + def close(self): + pass + + monkeypatch.setattr("run_agent.AIAgent", _StubAgent) + + result = curator._run_llm_review("review") + + assert result["error"] is None + assert captured["request_overrides"] == { + "extra_body": { + "shared": "slot", + "runtime_only": True, + "slot_only": True, + }, + "service_tier": "priority", + } diff --git a/tests/run_agent/test_background_review_cost_controls.py b/tests/run_agent/test_background_review_cost_controls.py index 5ca47b2a0f9..7e5da983faf 100644 --- a/tests/run_agent/test_background_review_cost_controls.py +++ b/tests/run_agent/test_background_review_cost_controls.py @@ -8,6 +8,7 @@ Covers the two behaviors this change adds: Pure-function / config-driven; no live model calls. """ +from typing import Any from unittest.mock import patch from agent import background_review as br @@ -28,6 +29,9 @@ class _FakeAgent: def __init__(self, provider="openai-codex", model="gpt-5.5"): self.provider = provider self.model = model + self._credential_pool: Any = None + self.request_overrides = {} + self.max_tokens: int | None = None def _current_main_runtime(self): return { @@ -56,6 +60,9 @@ def test_routing_to_different_model_marks_routed_and_resolves_credentials(): fake_rp = { "provider": "openrouter", "api_key": "or-key", "base_url": "https://openrouter.ai/api/v1", "api_mode": "chat_completions", + "credential_pool": "routed-pool", + "request_overrides": {"extra_body": {"store": False}}, + "max_output_tokens": 2048, } with patch("hermes_cli.config.load_config", return_value=cfg), \ patch("hermes_cli.runtime_provider.resolve_runtime_provider", return_value=fake_rp): @@ -64,6 +71,21 @@ def test_routing_to_different_model_marks_routed_and_resolves_credentials(): assert rt["provider"] == "openrouter" assert rt["model"] == "google/gemini-3-flash-preview" assert rt["api_key"] == "or-key" + assert rt["credential_pool"] == "routed-pool" + assert rt["request_overrides"] == {"extra_body": {"store": False}} + assert rt["max_tokens"] == 2048 + + +def test_unrouted_runtime_keeps_parent_pool_and_overrides(): + agent = _FakeAgent() + agent._credential_pool = "parent-pool" + agent.request_overrides = {"service_tier": "priority"} + agent.max_tokens = 4096 + with patch("hermes_cli.config.load_config", return_value={}): + rt = br._resolve_review_runtime(agent) + assert rt["credential_pool"] == "parent-pool" + assert rt["request_overrides"] == {"service_tier": "priority"} + assert rt["max_tokens"] == 4096 def test_routing_same_model_as_parent_is_not_routed():