diff --git a/acp_adapter/tools.py b/acp_adapter/tools.py index fce98cf54e6c..1f272ab252dc 100644 --- a/acp_adapter/tools.py +++ b/acp_adapter/tools.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import logging import uuid from typing import Any, Dict, List, Optional @@ -14,6 +15,8 @@ from acp.schema import ( ToolKind, ) +logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # Map hermes tool names -> ACP ToolKind # --------------------------------------------------------------------------- @@ -1026,7 +1029,37 @@ def build_tool_start( *, edit_diff: Any = None, ) -> ToolCallStart: - """Create a ToolCallStart event for the given hermes tool invocation.""" + """Create a ToolCallStart event for the given hermes tool invocation. + + A malformed tool argument (e.g. a non-string ``command``/``path`` from a + model that ignores the schema) must never abort the ACP tool-call render — + ``build_tool_start`` runs on the live tool-progress callback and during + session history replay. On any failure in the title/content/location + builders, fall back to a minimal, valid start event. Mirrors + ``get_cute_tool_message`` in ``agent/display.py``, wrapped for the same + reason on the CLI side. + """ + try: + return _build_tool_start( + tool_call_id, tool_name, arguments, edit_diff=edit_diff + ) + except Exception as exc: # noqa: BLE001 — a tool-call render must never abort the turn + logger.debug("ACP tool-start render failed for %r: %s", tool_name, exc) + safe_name = tool_name if isinstance(tool_name, str) and tool_name else "tool" + return acp.start_tool_call( + tool_call_id, safe_name, kind=get_tool_kind(safe_name), + content=None, locations=[], raw_input=None, + ) + + +def _build_tool_start( + tool_call_id: str, + tool_name: str, + arguments: Dict[str, Any], + *, + edit_diff: Any = None, +) -> ToolCallStart: + """Build the ToolCallStart event (unguarded; see ``build_tool_start``).""" kind = get_tool_kind(tool_name) title = build_tool_title(tool_name, arguments) locations = extract_locations(arguments) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index dbc946ce899a..6615dace60a8 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -534,25 +534,28 @@ def interruptible_api_call(agent, api_kwargs: dict): and _ttfb_disable_above > 0 and _est_tokens_for_codex_watchdog >= _ttfb_disable_above ): - _ttfb_enabled = False - logger.info( - "Disabling openai-codex no-byte TTFB watchdog for large request " - "(context=~%s tokens >= %.0f). Waiting for backend response instead. " - "Set HERMES_CODEX_TTFB_STRICT=1 to force early reconnects.", - f"{_est_tokens_for_codex_watchdog:,}", - _ttfb_disable_above, - ) - else: - _ttfb_cap = _env_float("HERMES_CODEX_TTFB_MAX_SECONDS", 120.0) - if _ttfb_cap > 0 and _ttfb_timeout > _ttfb_cap: + _large_request_ttfb_timeout = _codex_idle_timeout_default + if _ttfb_timeout < _large_request_ttfb_timeout: logger.info( - "Capping openai-codex no-byte TTFB timeout from %.0fs to %.0fs " - "(context=~%s tokens). Set HERMES_CODEX_TTFB_MAX_SECONDS to tune.", + "Scaling openai-codex no-byte TTFB watchdog from %.0fs to %.0fs " + "for large request (context=~%s tokens >= %.0f). " + "Set HERMES_CODEX_TTFB_STRICT=1 to keep the smaller cutoff.", _ttfb_timeout, - _ttfb_cap, + _large_request_ttfb_timeout, f"{_est_tokens_for_codex_watchdog:,}", + _ttfb_disable_above, ) - _ttfb_timeout = _ttfb_cap + _ttfb_timeout = _large_request_ttfb_timeout + _ttfb_cap = _env_float("HERMES_CODEX_TTFB_MAX_SECONDS", 120.0) + if _ttfb_cap > 0 and _ttfb_timeout > _ttfb_cap: + logger.info( + "Capping openai-codex no-byte TTFB timeout from %.0fs to %.0fs " + "(context=~%s tokens). Set HERMES_CODEX_TTFB_MAX_SECONDS to tune.", + _ttfb_timeout, + _ttfb_cap, + f"{_est_tokens_for_codex_watchdog:,}", + ) + _ttfb_timeout = _ttfb_cap _codex_idle_enabled = _codex_watchdog_enabled _codex_idle_timeout = _env_float( @@ -578,12 +581,23 @@ def interruptible_api_call(agent, api_kwargs: dict): t.join(timeout=0.3) _poll_count += 1 - # Touch activity every ~30s so the gateway's inactivity - # monitor knows we're alive while waiting for the response. + # Every ~30s: touch activity for the gateway inactivity monitor AND + # rewrite the live spinner/status line so CLI/TUI/Desktop users see + # what the agent is waiting on instead of an unexplained generic + # spinner (the "infinite thinking" complaint — the wait itself is + # usually a slow/overloaded provider, but the UI never said so). if _poll_count % 100 == 0: # 100 × 0.3s = 30s _elapsed = time.time() - _call_start - agent._touch_activity( - f"waiting for non-streaming response ({int(_elapsed)}s elapsed)" + _deadline = _stale_timeout + if ( + _ttfb_enabled + and getattr(agent, "_codex_stream_last_event_ts", None) is None + ): + _deadline = min(_deadline, _ttfb_timeout) + agent._emit_wait_notice( + f"⏳ waiting on {api_kwargs.get('model', 'the provider')} — " + f"{int(_elapsed)}s with no response yet (provider may be slow " + f"or overloaded; auto-reconnect at {int(_deadline)}s)" ) _elapsed = time.time() - _call_start @@ -628,6 +642,10 @@ def interruptible_api_call(agent, api_kwargs: dict): _close_request_client_once("codex_ttfb_kill") except Exception: pass + agent._emit_wait_notice( + f"⚠ no response from provider in {int(_elapsed)}s — " + f"reconnecting..." + ) agent._touch_activity( f"codex stream killed after {int(_elapsed)}s with no first byte" ) @@ -3134,9 +3152,29 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= if _hb_now - _last_heartbeat >= _HEARTBEAT_INTERVAL: _last_heartbeat = _hb_now _waiting_secs = int(_hb_now - last_chunk_time["t"]) - agent._touch_activity( - f"waiting for stream response ({_waiting_secs}s, no chunks yet)" - ) + if _waiting_secs >= _HEARTBEAT_INTERVAL: + # No chunks for 30s+ — rewrite the live spinner/status line + # so CLI/TUI/Desktop users see WHAT the wait is (slow or + # overloaded provider / long thinking pause) instead of an + # unexplained generic spinner, and WHEN recovery kicks in. + if ( + _stream_stale_timeout is not None + and _stream_stale_timeout != float("inf") + ): + _recovery = f"; auto-reconnect at {int(_stream_stale_timeout)}s" + else: + _recovery = "" + agent._emit_wait_notice( + f"⏳ waiting on {api_kwargs.get('model', 'the provider')} — " + f"{_waiting_secs}s with no output yet (provider may be " + f"slow or overloaded, or the model is thinking{_recovery})" + ) + else: + # Chunks are flowing — keep the activity tracker fresh but + # leave the live display alone. + agent._touch_activity( + f"waiting for stream response ({_waiting_secs}s, no chunks yet)" + ) # Detect stale streams: connections kept alive by SSE pings # but delivering no real chunks. Kill the client so the @@ -3179,6 +3217,10 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= # Reset the timer so we don't kill repeatedly while # the inner thread processes the closure. last_chunk_time["t"] = time.time() + agent._emit_wait_notice( + f"⚠ no output from provider for {int(_stale_elapsed)}s — " + f"reconnecting..." + ) agent._touch_activity( f"stale stream detected after {int(_stale_elapsed)}s, reconnecting" ) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 81c3131eace7..f3f5ec3da97f 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1360,6 +1360,45 @@ def _normalize_codex_response( # so the model keeps its chain-of-thought on the retry. final_text = "" + # ── Reasoning-channel answer salvage (xAI grok) ────────────── + # grok-4.x on the xAI /v1/responses surface sometimes emits its final + # answer inside the reasoning item instead of as a ``message`` output + # item, marking where the answer starts with grok's internal + # ```` delimiter. Without salvage, the reasoning-only rule + # below classifies the turn ``incomplete`` — and because reasoning + # items on this surface carry no ``encrypted_content``, the interim + # message replays as nothing, so every continuation request is + # byte-identical to the one that just failed. The turn burns its 3 + # retries and dies with "Codex response remained incomplete after 3 + # continuation attempts" even though the answer was produced on the + # first attempt. Observed live with grok-4.20 on xai-oauth + # (2026-07-13). Promote the delimited tail to assistant content and + # keep the untagged prefix as thinking text. + if ( + issuer_kind == "xai_responses" + and not final_text + and not tool_calls + and reasoning_parts + ): + joined_reasoning = "\n\n".join(reasoning_parts) + marker = joined_reasoning.rfind("") + if marker != -1: + salvaged = joined_reasoning[marker + len(""):] + closing = salvaged.find("") + if closing != -1: + salvaged = salvaged[:closing] + salvaged = salvaged.strip() + if salvaged: + logger.warning( + "xAI response delivered its final answer inside the " + "reasoning channel ( delimiter); promoting " + "%d chars to assistant content.", + len(salvaged), + ) + final_text = salvaged + reasoning_prefix = joined_reasoning[:marker].strip() + reasoning_parts = [reasoning_prefix] if reasoning_prefix else [] + assistant_message = SimpleNamespace( content=final_text, tool_calls=tool_calls, @@ -1380,12 +1419,28 @@ def _normalize_codex_response( finish_reason = "incomplete" elif (reasoning_items_raw or reasoning_parts or saw_reasoning_item) and not final_text: # Response contains only reasoning (encrypted thinking state and/or - # human-readable summary) with no visible content or tool calls. The - # model is still thinking and needs another turn to produce the actual - # answer. Marking this as "stop" would send it into the empty-content - # retry loop which burns retries then fails — treat it as incomplete so - # the Codex continuation path handles it correctly. - finish_reason = "incomplete" + # human-readable summary) with no visible content or tool calls. + # + # For the specially-handled backends (Codex, xAI, GitHub/Copilot), + # reasoning-only with status="completed" means "the model is still + # thinking and needs another turn" — treat it as incomplete so the + # Codex continuation path retries instead of falling into the + # empty-content retry loop. + # + # For all other backends (other:, etc.), trust the provider's + # own response.status signal. When status == "completed" and no items + # are queued/in_progress/incomplete, reasoning alone is a valid final + # state — forcing "incomplete" causes multi-minute stalls as the + # continuation path re-issues calls (3 retries × up to 240s each). + # See https://github.com/NousResearch/hermes-agent/issues/64434 + if response_status == "completed" and issuer_kind not in ( + "codex_backend", + "xai_responses", + "github_responses", + ): + finish_reason = "stop" + else: + finish_reason = "incomplete" else: finish_reason = "stop" return assistant_message, finish_reason diff --git a/agent/context_compressor.py b/agent/context_compressor.py index ec4314ab40b0..fb998a1dbe3d 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -586,6 +586,21 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An return result if changed else messages +def _str_arg(args: dict, key: str, default: str = "") -> str: + """Safely get a string argument from parsed tool args. + + LLMs sometimes return non-string parameter values (e.g. bool, int) for + tool calls. Calling ``len()`` / ``.count()`` / slicing on those causes + ``TypeError`` / ``AttributeError`` which crashes context compression. + This helper coerces any value to ``str`` so downstream code can assume + a string is always returned. + """ + val = args.get(key, default) + if isinstance(val, str): + return val + return str(val) if val is not None else default + + def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> str: """Create an informative 1-line summary of a tool call + result. @@ -598,18 +613,37 @@ def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> [terminal] ran `npm test` -> exit 0, 47 lines output [read_file] read config.py from line 1 (1,200 chars) [search_files] content search for 'compress' in agent/ -> 12 matches + + Never raises: models sometimes emit non-string argument values (bool, + int, None) and the args here come from persisted session history, so a + single malformed historical call must not crash compression — which + retries on the same history and would crash-loop. Individual branches + coerce the values they slice/measure (keeping summaries informative); + this wrapper is the backstop for anything they miss. """ + try: + return _summarize_tool_result_unguarded(tool_name, tool_args, tool_content) + except Exception as exc: # noqa: BLE001 — a summary must never crash compression + logger.debug("Tool-result summary failed for %s: %s", tool_name, exc) + _len = len(tool_content) if isinstance(tool_content, str) else 0 + return f"[{tool_name}] ({_len:,} chars result)" + + +def _summarize_tool_result_unguarded(tool_name: str, tool_args: str, tool_content: str) -> str: + """Build the summary line (unguarded; see ``_summarize_tool_result``).""" try: args = json.loads(tool_args) if tool_args else {} except (json.JSONDecodeError, TypeError): args = {} + if not isinstance(args, dict): + args = {} content = tool_content or "" content_len = len(content) line_count = content.count("\n") + 1 if content.strip() else 0 if tool_name == "terminal": - cmd = args.get("command", "") + cmd = _str_arg(args, "command") if len(cmd) > 80: cmd = cmd[:77] + "..." exit_match = re.search(r'"exit_code"\s*:\s*(-?\d+)', content) @@ -623,7 +657,7 @@ def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> if tool_name == "write_file": path = args.get("path", "?") - written_lines = args.get("content", "").count("\n") + 1 if args.get("content") else "?" + written_lines = _str_arg(args, "content").count("\n") + 1 if args.get("content") else "?" return f"[write_file] wrote to {path} ({written_lines} lines)" if tool_name == "search_files": @@ -658,14 +692,15 @@ def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> return f"[web_extract] {url_desc} ({content_len:,} chars)" if tool_name == "delegate_task": - goal = args.get("goal", "") + goal = _str_arg(args, "goal") if len(goal) > 60: goal = goal[:57] + "..." return f"[delegate_task] '{goal}' ({content_len:,} chars result)" if tool_name == "execute_code": - code_preview = (args.get("code") or "")[:60].replace("\n", " ") - if len(args.get("code", "")) > 60: + code_str = _str_arg(args, "code") + code_preview = code_str[:60].replace("\n", " ") + if len(code_str) > 60: code_preview += "..." return f"[execute_code] `{code_preview}` ({line_count} lines output)" @@ -674,7 +709,7 @@ def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> return f"[{tool_name}] name={name} ({content_len:,} chars)" if tool_name == "vision_analyze": - question = args.get("question", "")[:50] + question = _str_arg(args, "question")[:50] return f"[vision_analyze] '{question}' ({content_len:,} chars)" if tool_name == "memory": diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 5a81d2f7793e..a628093e1f7b 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -457,6 +457,21 @@ def _get_continuation_prompt(is_partial_stub: bool, dropped_tools: Optional[List ) +# Continuation nudge for Codex/Responses turns that came back with only +# internal reasoning (no visible content, no tool calls). When the interim +# assistant message also carries no encrypted reasoning items and no +# replayable message items, _chat_messages_to_responses_input emits nothing +# for it — a bare retry would be byte-identical to the request that just +# failed, so the model (observed: grok-4.20 on xai-oauth) deterministically +# repeats the reasoning-only response until the retry budget is exhausted. +_CODEX_INCOMPLETE_NUDGE = ( + "[System: Your previous response contained only internal reasoning and " + "never produced a visible answer or tool call. Do not keep thinking. " + "Produce your final answer as plain text now (or make the tool call " + "you were planning).]" +) + + # Shared recovery hint appended to every content-policy refusal message. Both # the HTTP-200 refusal path (``finish_reason=content_filter``) and the # exception path (a provider moderation error classified as @@ -4469,8 +4484,56 @@ def run_conversation( agent._emit_interim_assistant_message(interim_msg) if agent._codex_incomplete_retries < 3: + # When the interim message has nothing the Responses + # input converter will replay (no visible content, no + # encrypted reasoning items, no replayable message + # items — plain-text reasoning only), a bare retry is + # byte-identical to the request that just came back + # incomplete and fails the same way every time + # (observed with grok-4.20 on xai-oauth, whose + # reasoning items lack encrypted_content). Append a + # user-role nudge so the retry actually differs and + # explicitly asks for the final answer. + interim_replayable = ( + interim_has_content + or interim_has_codex_reasoning + or interim_has_codex_message_items + ) + if not interim_replayable: + _last_msg = messages[-1] if messages else None + _already_nudged = ( + isinstance(_last_msg, dict) + and _last_msg.get("role") == "user" + and _last_msg.get("content") == _CODEX_INCOMPLETE_NUDGE + ) + # Alternation guard: the nudge is a user-role message, + # so it may only follow an assistant message. When the + # interim was too empty to append (no content AND no + # reasoning), the last message is still the prior + # user/tool turn — appending the nudge there would + # create a user→user / tool→user sequence that strict + # providers reject. + _last_is_assistant = ( + isinstance(_last_msg, dict) + and _last_msg.get("role") == "assistant" + ) + if not _already_nudged and _last_is_assistant: + messages.append({ + "role": "user", + "content": _CODEX_INCOMPLETE_NUDGE, + }) if not agent.quiet_mode: agent._vprint(f"{agent.log_prefix}↻ Codex response incomplete; continuing turn ({agent._codex_incomplete_retries}/3)") + # Surface the continuation on the live spinner/status line + # (CLI/TUI/Desktop) and gateway heartbeat: each of these + # retries can spend minutes waiting on the provider, and + # without a distinct notice the user only sees a generic + # thinking spinner ("infinite thinking", #64434). + agent._emit_wait_notice( + f"↻ model returned reasoning with no final answer — " + f"asking it to continue " + f"({agent._codex_incomplete_retries}/3)" + ) agent._session_messages = messages continue diff --git a/agent/display.py b/agent/display.py index 66872c35d667..27aff5f9d955 100644 --- a/agent/display.py +++ b/agent/display.py @@ -462,13 +462,14 @@ def build_tool_preview(tool_name: str, args: dict, max_len: int | None = None) - sid = args.get("session_id", "") data = args.get("data", "") timeout_val = args.get("timeout") - parts = [action] + parts = [str(action) if action else ""] if sid: - parts.append(sid[:16]) + parts.append(str(sid)[:16]) if data: - parts.append(f'"{_oneline(data[:20])}"') + parts.append(f'"{_oneline(str(data)[:20])}"') if timeout_val and action == "wait": parts.append(f"{timeout_val}s") + parts = [p for p in parts if p] return " ".join(parts) if parts else None if tool_name == "todo": diff --git a/agent/moa_loop.py b/agent/moa_loop.py index 314aaa8b0295..43691fd7f997 100644 --- a/agent/moa_loop.py +++ b/agent/moa_loop.py @@ -137,6 +137,36 @@ def _slot_reasoning_config(slot: dict[str, Any]) -> dict[str, Any] | None: return None +def _aggregator_reasoning_config(aggregator: dict[str, Any]) -> dict[str, Any] | None: + """Resolve the aggregator's reasoning config: slot > per-model > global. + + The aggregator is MoA's ACTING model, so when its slot doesn't pin a + reasoning_effort it must resolve exactly like any other acting model: + through the shared chokepoint (``resolve_reasoning_config``), which + applies ``agent.reasoning_overrides`` for the slot's model first, then + the global ``agent.reasoning_effort``. Without this the main loop's + reasoning gates (keyed to the virtual ``moa://local`` identity) never + fire, so the aggregator silently ran at the backend default (#64187). + + Reference advisors intentionally do NOT get this fallback: they are side + calls (like auxiliary tasks), and inheriting a global ``xhigh`` into every + advisor fan-out would silently multiply cost. Their depth is slot-or- + provider-default only. + """ + cfg = _slot_reasoning_config(aggregator) + if cfg is not None: + return cfg + try: + from hermes_cli.config import load_config + from hermes_constants import resolve_reasoning_config + + return resolve_reasoning_config( + load_config() or {}, str(aggregator.get("model") or "") + ) + except Exception: # pragma: no cover - defensive; bad config must not break MoA + return None + + def _slot_runtime(slot: dict[str, Any]) -> dict[str, Any]: """Resolve a reference/aggregator slot to real runtime call kwargs. @@ -694,7 +724,7 @@ def aggregate_moa_context( messages=agg_messages, temperature=aggregator_temperature, max_tokens=max_tokens, - reasoning_config=_slot_reasoning_config(aggregator), + reasoning_config=_aggregator_reasoning_config(aggregator), **agg_runtime, ) synthesis = _extract_text(response) @@ -1089,7 +1119,7 @@ class MoAChatCompletions: max_tokens=agg_kwargs.get("max_tokens"), tools=agg_kwargs.get("tools"), extra_body=agg_kwargs.get("extra_body"), - reasoning_config=_slot_reasoning_config(aggregator), + reasoning_config=_aggregator_reasoning_config(aggregator), **stream_kwargs, **_slot_runtime(aggregator), ) diff --git a/hermes_cli/model_catalog.py b/hermes_cli/model_catalog.py index 40a3a5c00bd5..fec73234aa45 100644 --- a/hermes_cli/model_catalog.py +++ b/hermes_cli/model_catalog.py @@ -356,6 +356,42 @@ def get_curated_nous_models() -> list[str] | None: return out or None +def _default_model_from_block(block: dict[str, Any] | None) -> str | None: + """Return the id of the model entry labeled ``"default": true``, or None.""" + if not isinstance(block, dict): + return None + for m in block.get("models", []): + if isinstance(m, dict) and m.get("default"): + mid = str(m.get("id") or "").strip() + if mid: + return mid + return None + + +def get_default_model_from_cache(provider: str) -> str | None: + """Return the catalog's labeled default model for ``provider`` — cache only. + + The manifest marks exactly one model entry per provider with + ``"default": true``; that entry is the model Hermes silently lands on when + the user never picked one. This accessor reads ONLY the in-process copy or + the disk cache — it NEVER triggers a network fetch, so it is safe on hot + resolution paths (agent build, gateway session setup) that must stay + network-free. The cache is kept fresh by the picker/`hermes update` paths; + when no cached manifest exists (fresh install, offline), returns None and + the caller falls back to the in-repo constant. + """ + if _catalog_cache is not None: + block = _catalog_cache.get("providers", {}).get(provider) + found = _default_model_from_block(block) + if found: + return found + disk_data, _mtime = _read_disk_cache() + if disk_data is not None: + block = disk_data.get("providers", {}).get(provider) + return _default_model_from_block(block) + return None + + def seed_cache_from_checkout(project_root: "Path | str") -> bool: """Overwrite the disk cache with the catalog shipped in a local checkout. diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 9941760aad10..d9235a6d43e6 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -74,7 +74,7 @@ OPENROUTER_MODELS: list[tuple[str, str]] = [ # MiniMax ("minimax/minimax-m3", ""), # Z-AI - ("z-ai/glm-5.2", ""), + ("z-ai/glm-5.2", "default"), ("z-ai/glm-5.1", ""), # Xiaomi ("xiaomi/mimo-v2.5-pro", ""), @@ -1301,9 +1301,14 @@ _PROVIDER_ALIASES = { } -# The model Hermes silently lands on when the user never picked one and the -# provider's catalog carries it (GUI onboarding confirm card, empty -# ``model.default``, provider-set-but-model-missing resolution). Deliberately a +# In-repo fallback for the model Hermes silently lands on when the user never +# picked one (GUI onboarding confirm card, empty ``model.default``, +# provider-set-but-model-missing resolution). The AUTHORITATIVE source is the +# remote model catalog: the manifest labels exactly one entry per provider +# with ``"default": true`` (see get_default_model_from_cache in +# model_catalog.py), so maintainers can rotate the default without shipping a +# release. This constant is the offline/fresh-install fallback and MUST match +# the labeled entry in website/static/api/model-catalog.json. Deliberately a # capable low-cost model rather than the curated lists' entry [0]: aggregator # lists are ordered most-capable-first, so [0] is the priciest Anthropic # flagship (claude-fable-5 / opus) — silently billing the most expensive model @@ -1311,43 +1316,57 @@ _PROVIDER_ALIASES = { PREFERRED_SILENT_DEFAULT_MODEL = "z-ai/glm-5.2" -def pick_silent_default_model(model_ids: list[str]) -> str: +def get_preferred_silent_default_model(provider: str = "openrouter") -> str: + """Return the silent-default model id — catalog label first, constant second. + + Reads the ``"default": true`` label from the cached remote catalog + (never hits the network — safe on hot resolution paths), falling back to + :data:`PREFERRED_SILENT_DEFAULT_MODEL` when no cached manifest exists or + the provider block carries no label. + """ + try: + from hermes_cli.model_catalog import get_default_model_from_cache + labeled = get_default_model_from_cache(provider) + if labeled: + return labeled + except Exception: + pass + return PREFERRED_SILENT_DEFAULT_MODEL + + +def pick_silent_default_model(model_ids: list[str], provider: str = "openrouter") -> str: """Pick the silent default from an available-models list. - Returns :data:`PREFERRED_SILENT_DEFAULT_MODEL` when the list carries it, + Returns the catalog-labeled default (see + :func:`get_preferred_silent_default_model`) when the list carries it, else the first entry, else "". Used by every surface that must choose a model on the user's behalf without an interactive picker (GUI onboarding recommended-default, empty-model runtime fallback). """ - if PREFERRED_SILENT_DEFAULT_MODEL in model_ids: - return PREFERRED_SILENT_DEFAULT_MODEL + preferred = get_preferred_silent_default_model(provider) + if preferred in model_ids: + return preferred return model_ids[0] if model_ids else "" -# Cost-safe overrides for the *silent* auto-default -# (``get_default_model_for_provider``). Most providers' curated lists lead with a -# sensible default, but metered aggregators (Nous Portal, OpenRouter) order +# Providers whose *silent* auto-default must go through the cost-safe +# catalog-labeled default (``get_preferred_silent_default_model``) instead of +# curated-list entry [0]. Metered aggregators (Nous Portal, OpenRouter) order # their lists best-/most-capable-first — entry [0] is the priciest flagship # (``anthropic/claude-fable-5``). Using that as the non-interactive fallback # when a profile sets a provider with no model silently bills the most # expensive model for traffic the user never opted into (a missing default -# escalated to Opus and billed 863 requests before the user noticed). Pin the -# silent default to ``PREFERRED_SILENT_DEFAULT_MODEL`` instead so a missing -# model can never escalate to the flagship. +# escalated to Opus and billed 863 requests before the user noticed). The +# catalog manifest labels the default entry (``"default": true``) so it can +# rotate without a release; a missing model must never escalate to the +# flagship. # -# This is deliberately a fixed, side-effect-free default for the hot resolution -# path. The *interactive* default (GUI onboarding / ``hermes model``) uses the -# richer free/paid-tier-aware resolver — see ``get_recommended_default_model`` -# in hermes_cli/web_server.py and ``partition_nous_models_by_tier`` — which can -# hit the Portal; this fallback must stay cheap and network-free. -_PROVIDER_SILENT_DEFAULT_OVERRIDES: dict[str, str] = { - "nous": PREFERRED_SILENT_DEFAULT_MODEL, - # OpenRouter has no static ``_PROVIDER_MODELS`` entry (its picker list is - # fetched live), but the curated snapshot (``OPENROUTER_MODELS``) carries - # the preferred default — trust the override so provider-set-but-model- - # missing setups land on it instead of resolving to "". - "openrouter": PREFERRED_SILENT_DEFAULT_MODEL, -} +# This is deliberately a network-free lookup for the hot resolution path +# (cache-only catalog read). The *interactive* default (GUI onboarding / +# ``hermes model``) uses the richer free/paid-tier-aware resolver — see +# ``get_recommended_default_model`` in hermes_cli/web_server.py and +# ``partition_nous_models_by_tier`` — which can hit the Portal. +_SILENT_DEFAULT_PROVIDERS: frozenset[str] = frozenset({"nous", "openrouter"}) def get_default_model_for_provider(provider: str) -> str: @@ -1360,17 +1379,19 @@ def get_default_model_for_provider(provider: str) -> str: For most providers this is the first entry in ``_PROVIDER_MODELS`` — the same model the ``hermes model`` picker offers first. For metered aggregators whose curated list is ordered most-capable-first, that entry is also the - most EXPENSIVE one, so silently defaulting to it is a billing footgun. Such - providers carry an explicit override in - ``_PROVIDER_SILENT_DEFAULT_OVERRIDES``; a missing model must never - auto-escalate to the flagship. + most EXPENSIVE one, so silently defaulting to it is a billing footgun. + Those providers (``_SILENT_DEFAULT_PROVIDERS``) resolve through the + catalog-labeled default instead; a missing model must never auto-escalate + to the flagship. """ models = _PROVIDER_MODELS.get(provider, []) - override = _PROVIDER_SILENT_DEFAULT_OVERRIDES.get(provider) - # Trust the override when the provider has no static catalog (OpenRouter's - # picker list is fetched live; its curated snapshot carries the override). - if override and (override in models or not models): - return override + if provider in _SILENT_DEFAULT_PROVIDERS: + preferred = get_preferred_silent_default_model(provider) + # Trust the preferred default even when the provider has no static + # catalog (OpenRouter's picker list is fetched live; its curated + # snapshot carries the default). + if preferred and (preferred in models or not models): + return preferred return models[0] if models else "" @@ -1456,6 +1477,7 @@ def fetch_openrouter_models( live_by_id[mid] = item curated: list[tuple[str, str]] = [] + silent_default = get_preferred_silent_default_model("openrouter") for preferred_id in preferred_ids: live_item = live_by_id.get(preferred_id) if live_item is None: @@ -1465,14 +1487,20 @@ def fetch_openrouter_models( # when the user selects them. Ported from Kilo-Org/kilocode#9068. if not _openrouter_model_supports_tools(live_item): continue - desc = "free" if _openrouter_model_is_free(live_item.get("pricing")) else "" + if preferred_id == silent_default: + # Keep the silent-default badge through the live refresh so the + # picker shows which model Hermes lands on when none is selected. + desc = "default" + else: + desc = "free" if _openrouter_model_is_free(live_item.get("pricing")) else "" curated.append((preferred_id, desc)) if not curated: return list(_openrouter_catalog_cache or fallback) - first_id, _ = curated[0] - curated[0] = (first_id, "recommended") + first_id, first_desc = curated[0] + if not first_desc: + curated[0] = (first_id, "recommended") _openrouter_catalog_cache = curated return list(curated) @@ -1976,7 +2004,18 @@ def detect_static_provider_for_model( and default_models and resolved_provider not in current_keys ): - return (resolved_provider, default_models[0]) + # Route through the cost-safe default rather than picking + # ``default_models[0]`` directly. For metered aggregators whose + # curated list is ordered most-capable-first (e.g. Nous Portal), + # entry [0] is the priciest flagship, and typing ``/model nous`` + # would silently escalate to it — the exact billing footgun the + # catalog-labeled silent default (``_SILENT_DEFAULT_PROVIDERS``) + # exists to prevent. For providers outside that set this is + # unchanged (it returns ``models[0]``). + return ( + resolved_provider, + get_default_model_for_provider(resolved_provider) or default_models[0], + ) # Aggregators list other providers' models — never auto-switch TO them # If the model belongs to the current provider's catalog, don't suggest switching diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index f924fecdd7c9..175ab7550e81 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -5605,7 +5605,7 @@ def get_recommended_default_model(provider: str = ""): model_ids, pricing, portal_url ) - model = pick_silent_default_model(model_ids) + model = pick_silent_default_model(model_ids, provider="nous") return {"provider": "nous", "model": model, "free_tier": bool(free_tier)} except Exception: _log.exception("GET /api/model/recommended-default (nous) failed") @@ -5623,7 +5623,7 @@ def get_recommended_default_model(provider: str = ""): for row in payload.get("providers", []): if str(row.get("slug", "")).lower() == slug: models = [str(m) for m in (row.get("models") or [])] - return {"provider": slug, "model": pick_silent_default_model(models), "free_tier": None} + return {"provider": slug, "model": pick_silent_default_model(models, provider=slug), "free_tier": None} return {"provider": slug, "model": "", "free_tier": None} except Exception: _log.exception("GET /api/model/recommended-default failed") diff --git a/run_agent.py b/run_agent.py index 504402459c21..ecb6dce613f4 100644 --- a/run_agent.py +++ b/run_agent.py @@ -934,6 +934,30 @@ class AIAgent: except Exception: logger.debug("notice_clear_callback error in _emit_notice_clear", exc_info=True) + def _emit_wait_notice(self, text: str) -> None: + """Surface a live wait-state explanation on every driver. + + Long provider waits (slow/overloaded backend, no first byte, reasoning + model thinking for minutes) used to leave the user staring at a generic + "cogitating..." spinner with no hint of what the agent was waiting on. + This helper rewrites the live status line with an explanation: + + - CLI: ``thinking_callback`` updates the prompt_toolkit spinner text. + - TUI / Desktop: the same callback is bridged to the ``thinking.delta`` + event, which both render as the live spinner/status line. + - Gateway: ``_touch_activity`` stores the text as the activity + description, which the "⏳ Working — N min" heartbeat includes. + + Never raises — a wait notice must not break the API-call wait loop. + """ + self._touch_activity(text) + _thinking_cb = getattr(self, "thinking_callback", None) + if _thinking_cb: + try: + _thinking_cb(text) + except Exception: + logger.debug("thinking_callback error in _emit_wait_notice", exc_info=True) + # ── Buffered retry/fallback status ──────────────────────────────────── # Retry and fallback chains were flooding the CLI/gateway with status # noise that users found confusing: a single transient 429 could produce diff --git a/scripts/build_model_catalog.py b/scripts/build_model_catalog.py index 102ae2b05b0b..eeb3c51d214f 100755 --- a/scripts/build_model_catalog.py +++ b/scripts/build_model_catalog.py @@ -33,12 +33,31 @@ sys.path.insert(0, REPO_ROOT) # Ensure HERMES_HOME is set for imports that touch it at module level. os.environ.setdefault("HERMES_HOME", os.path.join(os.path.expanduser("~"), ".hermes")) -from hermes_cli.models import OPENROUTER_MODELS, _PROVIDER_MODELS # noqa: E402 +from hermes_cli.models import ( # noqa: E402 + OPENROUTER_MODELS, + PREFERRED_SILENT_DEFAULT_MODEL, + _PROVIDER_MODELS, +) OUTPUT_PATH = os.path.join(REPO_ROOT, "website", "static", "api", "model-catalog.json") CATALOG_VERSION = 1 +def _openrouter_entry(mid: str, desc: str) -> dict: + entry: dict = {"id": mid, "description": desc} + if mid == PREFERRED_SILENT_DEFAULT_MODEL: + entry["description"] = desc or "default" + entry["default"] = True + return entry + + +def _nous_entry(mid: str) -> dict: + entry: dict = {"id": mid} + if mid == PREFERRED_SILENT_DEFAULT_MODEL: + entry["default"] = True + return entry + + def build_catalog() -> dict: return { "version": CATALOG_VERSION, @@ -53,11 +72,13 @@ def build_catalog() -> dict: "display_name": "OpenRouter", "note": ( "Descriptions drive picker badges. Live /api/v1/models " - "filters curated ids by tool-calling support and free pricing." + "filters curated ids by tool-calling support and free pricing. " + 'The entry labeled "default": true is the model Hermes ' + "silently lands on when the user never picked one." ), }, "models": [ - {"id": mid, "description": desc} + _openrouter_entry(mid, desc) for mid, desc in OPENROUTER_MODELS ], }, @@ -66,11 +87,13 @@ def build_catalog() -> dict: "display_name": "Nous Portal", "note": ( "Free-tier gating is determined live via Portal pricing " - "(partition_nous_models_by_tier), not this manifest." + "(partition_nous_models_by_tier), not this manifest. " + 'The entry labeled "default": true is the model Hermes ' + "silently lands on when the user never picked one." ), }, "models": [ - {"id": mid} + _nous_entry(mid) for mid in _PROVIDER_MODELS.get("nous", []) ], }, diff --git a/scripts/release.py b/scripts/release.py index 80f9d69ca18f..4af52a6d13ed 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -104,6 +104,7 @@ AUTHOR_MAP = { "root@vmi3351581.contaboserver.net": "ostravajih", # PR #58374 salvage (poolside: coerce integer finish_reason and tool_call id to strings) "hello@sahil-shubham.in": "sahil-shubham", # PR #58448 salvage (whatsapp_cloud: honor documented WHATSAPP_CLOUD_ALLOWED_USERS / ALLOW_ALL_USERS in the DM intake gate) "ahmet.tunc@gmail.com": "Ahmett101", # PR #58445 salvage (profiles: allowlist default-export roots + preserve symlinks) + "ignaciopastorsan@gmail.com": "IpastorSan", # PR #63690 salvage (codex: rescue reasoning-only turns that die after 3 continuation attempts) "Ahmett101@users.noreply.github.com": "Ahmett101", # PR #59455 salvage (background-review: guard summarize against list-shaped tool responses; #59437) "wyuebei@gmail.com": "wyuebei-cloud", # PR #56640 salvage (hermes journey: replace GNU-only %-d strftime with dt.day for Windows) "yingwaizhiying@gmail.com": "msh01", # PR #58250 salvage (telegram: wall-clock init timeout via daemon-thread deadline + abandon the shielded initialize task on timeout so the retry ladder advances instead of hanging on attempt 1/8 under s6 supervision; #58236). Also covers PR #58276 salvage (compression: preserve a real user turn after compaction; #55677). @@ -333,6 +334,8 @@ AUTHOR_MAP = { "290859878+synapsesx@users.noreply.github.com": "synapsesx", "157689911+itsflownium@users.noreply.github.com": "itsflownium", "dirtyren@users.noreply.github.com": "dirtyren", + "liuwei666888@users.noreply.github.com": "liuwei666888", + "527711370@qq.com": "liuwei666888", "217401759+justinschille@users.noreply.github.com": "justinschille", "theoldwizard123@pm.me": "unsupportedpastels", "johnmlussier@gmail.com": "John-Lussier", diff --git a/tests/acp/test_tools.py b/tests/acp/test_tools.py index b291b36531a1..8ca3aafd2037 100644 --- a/tests/acp/test_tools.py +++ b/tests/acp/test_tools.py @@ -227,6 +227,26 @@ class TestBuildToolStart: assert result.content is None assert result.raw_input is None + def test_build_tool_start_survives_non_string_command(self): + """A malformed (non-string) terminal command previously raised + TypeError in build_tool_title (len(None)) and aborted the render.""" + result = build_tool_start("tc-bad-cmd", "terminal", {"command": None}) + assert isinstance(result, ToolCallStart) + assert result.kind == "execute" # tool identity preserved in the fallback + + def test_build_tool_start_survives_non_string_path(self): + """A non-string read_file path previously raised a ToolCallLocation + pydantic ValidationError in extract_locations and aborted the render.""" + result = build_tool_start("tc-bad-path", "read_file", {"path": {"p": "x"}}) + assert isinstance(result, ToolCallStart) + assert result.kind == "read" + + def test_build_tool_start_survives_non_string_goal(self): + """A non-string delegate_task goal previously raised TypeError + (len(123)) in build_tool_title and aborted the render.""" + result = build_tool_start("tc-bad-goal", "delegate_task", {"goal": 123}) + assert isinstance(result, ToolCallStart) + def test_build_tool_start_for_web_extract_is_compact(self): """web_extract start should stay compact; title identifies URLs.""" args = {"urls": ["https://example.com/docs"]} diff --git a/tests/agent/test_codex_responses_adapter.py b/tests/agent/test_codex_responses_adapter.py index 72b8787e02e4..4a14cadf4a3d 100644 --- a/tests/agent/test_codex_responses_adapter.py +++ b/tests/agent/test_codex_responses_adapter.py @@ -51,6 +51,12 @@ def test_normalize_codex_response_drops_transient_rs_tmp_reasoning_items(): def test_normalize_codex_response_treats_summary_only_reasoning_as_incomplete(): + """Summary-only reasoning keeps the continuation path for Codex backends. + + Since #64434, an unrecognized issuer with ``response.status="completed"`` + trusts the provider and returns ``stop`` — so this test pins the Codex + backend explicitly, where reasoning-only still means "still thinking". + """ response = SimpleNamespace( status="completed", output=[ @@ -63,7 +69,9 @@ def test_normalize_codex_response_treats_summary_only_reasoning_as_incomplete(): ], ) - assistant_message, finish_reason = _normalize_codex_response(response) + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="codex_backend" + ) assert finish_reason == "incomplete" assert assistant_message.content == "" @@ -429,3 +437,81 @@ def test_normalize_codex_response_failed_with_message_only(): ) with pytest.raises(RuntimeError, match=r"^model error$"): _normalize_codex_response(response) + + +# --------------------------------------------------------------------------- +# Reasoning-channel answer salvage (xAI grok) — grok-4.x on the xAI +# /v1/responses surface sometimes emits its final answer inside the +# reasoning item, delimited by grok's internal "" tag, with no +# ``message`` output item at all. Because those reasoning items carry no +# encrypted_content, the interim message replays as nothing and every +# continuation request is byte-identical — the turn burns 3 retries and +# fails even though the answer was produced. Observed live with grok-4.20 +# on xai-oauth (2026-07-13). +# --------------------------------------------------------------------------- + + +def _xai_reasoning_only_response(reasoning_text): + return SimpleNamespace( + status="completed", + output=[ + SimpleNamespace( + type="reasoning", + id="rs_1", + encrypted_content=None, + summary=[SimpleNamespace(text=reasoning_text)], + ) + ], + ) + + +def test_normalize_codex_response_salvages_xai_reasoning_channel_answer(): + response = _xai_reasoning_only_response( + "The process is still running.\n\nAll good, process running." + ) + + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="xai_responses" + ) + + assert finish_reason == "stop" + assert assistant_message.content == "All good, process running." + assert assistant_message.reasoning == "The process is still running." + + +def test_normalize_codex_response_salvage_strips_closing_tag(): + response = _xai_reasoning_only_response( + "Thinking.\nThe answer." + ) + + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="xai_responses" + ) + + assert finish_reason == "stop" + assert assistant_message.content == "The answer." + + +def test_normalize_codex_response_salvage_is_xai_scoped(): + """Non-xAI issuers keep the reasoning-only → incomplete classification; + the Codex backend replays encrypted reasoning, so its continuation + genuinely progresses and must not be short-circuited.""" + response = _xai_reasoning_only_response( + "Thinking.\nThe answer." + ) + + assistant_message, finish_reason = _normalize_codex_response(response) + + assert finish_reason == "incomplete" + assert assistant_message.content == "" + + +def test_normalize_codex_response_xai_reasoning_without_marker_stays_incomplete(): + response = _xai_reasoning_only_response("Still thinking, no answer yet.") + + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="xai_responses" + ) + + assert finish_reason == "incomplete" + assert assistant_message.content == "" diff --git a/tests/agent/test_codex_ttfb_watchdog.py b/tests/agent/test_codex_ttfb_watchdog.py index 983a4cbe4a03..283a6a8e9a8d 100644 --- a/tests/agent/test_codex_ttfb_watchdog.py +++ b/tests/agent/test_codex_ttfb_watchdog.py @@ -353,8 +353,8 @@ def test_ttfb_disabled_via_env_zero(tmp_path, monkeypatch): def test_large_codex_request_waits_instead_of_ttfb_reconnect(tmp_path, monkeypatch): """Large Codex inputs can legitimately take longer than the small-request - first-byte cutoff before the first SSE frame. Preserve the full input and - wait instead of killing/retrying at TTFB.""" + first-byte cutoff before the first SSE frame. Scale the TTFB timeout up + for those requests instead of killing/retrying at the small-request cutoff.""" from agent import chat_completion_helpers as h agent = _make_codex_agent(tmp_path, monkeypatch) @@ -386,6 +386,45 @@ def test_large_codex_request_waits_instead_of_ttfb_reconnect(tmp_path, monkeypat assert "codex_ttfb_kill" not in closes +def test_large_codex_request_can_still_ttfb_reconnect_when_capped(tmp_path, monkeypatch): + """Large Codex requests should keep a finite TTFB watchdog instead of + disabling it entirely. A low max cap should still force an early reconnect.""" + from agent import chat_completion_helpers as h + + agent = _make_codex_agent(tmp_path, monkeypatch) + monkeypatch.setenv("HERMES_CODEX_TTFB_TIMEOUT_SECONDS", "1") + monkeypatch.setenv("HERMES_CODEX_TTFB_MAX_SECONDS", "1") + + closes: list = [] + dummy_client = SimpleNamespace() + monkeypatch.setattr(agent, "_create_request_openai_client", lambda **k: dummy_client) + monkeypatch.setattr( + agent, "_abort_request_openai_client", lambda c, reason=None: closes.append(reason) + ) + monkeypatch.setattr( + agent, "_close_request_openai_client", lambda c, reason=None: closes.append(reason) + ) + + stop = {"flag": False} + + def fake_hang(api_kwargs, client=None, on_first_delta=None): + deadline = time.time() + 30 + while time.time() < deadline and not stop["flag"] and not agent._interrupt_requested: + time.sleep(0.02) + raise RuntimeError("connection closed") + + monkeypatch.setattr(agent, "_run_codex_stream", fake_hang) + + large_input = "x" * 44_000 # ~11k estimated tokens, above the large-request gate. + try: + with pytest.raises(TimeoutError) as excinfo: + h.interruptible_api_call(agent, {"model": "gpt-5.5", "input": large_input}) + assert "TTFB threshold: 1s" in str(excinfo.value) + assert "codex_ttfb_kill" in closes + finally: + stop["flag"] = True + + def test_large_codex_request_strict_ttfb_env_still_reconnects(tmp_path, monkeypatch): """Operators can force the old early-reconnect behavior for large inputs with HERMES_CODEX_TTFB_STRICT=1.""" diff --git a/tests/agent/test_moa_reasoning_effort.py b/tests/agent/test_moa_reasoning_effort.py index 0aa09f8b4cd3..e2ba25c4f404 100644 --- a/tests/agent/test_moa_reasoning_effort.py +++ b/tests/agent/test_moa_reasoning_effort.py @@ -70,3 +70,93 @@ def test_call_llm_builder_translates_reasoning_config_to_extra_body(): reasoning_config={"enabled": False}, ) assert off["extra_body"]["reasoning"] == {"enabled": False} + + +class TestAggregatorGlobalFallback: + """#64187: the aggregator (MoA's acting model) resolves like any acting + model when its slot has no reasoning_effort: per-model override + (agent.reasoning_overrides for the slot's model) > global + agent.reasoning_effort. Reference advisors do NOT get this fallback + (side calls — cost containment).""" + + def test_slot_value_wins_over_global(self, monkeypatch): + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"agent": {"reasoning_effort": "medium"}}, + ) + cfg = moa_loop._aggregator_reasoning_config({"reasoning_effort": "xhigh"}) + assert cfg == {"enabled": True, "effort": "xhigh"} + + def test_unset_slot_falls_back_to_global(self, monkeypatch): + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"agent": {"reasoning_effort": "high"}}, + ) + cfg = moa_loop._aggregator_reasoning_config({"provider": "openrouter", "model": "m"}) + assert cfg == {"enabled": True, "effort": "high"} + + def test_unset_slot_honors_per_model_override(self, monkeypatch): + """The aggregator's model gets its agent.reasoning_overrides entry — + same resolution as any acting model, not just the bare global.""" + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: { + "agent": { + "reasoning_effort": "medium", + "reasoning_overrides": {"claude-opus-4.8": "xhigh"}, + } + }, + ) + cfg = moa_loop._aggregator_reasoning_config( + {"provider": "openrouter", "model": "anthropic/claude-opus-4.8"} + ) + assert cfg == {"enabled": True, "effort": "xhigh"} + + def test_slot_value_beats_per_model_override(self, monkeypatch): + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: { + "agent": { + "reasoning_effort": "medium", + "reasoning_overrides": {"claude-opus-4.8": "xhigh"}, + } + }, + ) + cfg = moa_loop._aggregator_reasoning_config( + {"model": "anthropic/claude-opus-4.8", "reasoning_effort": "low"} + ) + assert cfg == {"enabled": True, "effort": "low"} + + def test_global_yaml_false_disables(self, monkeypatch): + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"agent": {"reasoning_effort": False}}, + ) + cfg = moa_loop._aggregator_reasoning_config({}) + assert cfg == {"enabled": False} + + def test_no_slot_no_global_returns_none(self, monkeypatch): + from agent import moa_loop + + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}) + assert moa_loop._aggregator_reasoning_config({}) is None + + def test_reference_slots_do_not_inherit_global(self, monkeypatch): + """Advisors stay slot-or-default: global effort must NOT leak in.""" + from agent import moa_loop + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"agent": {"reasoning_effort": "xhigh"}}, + ) + assert moa_loop._slot_reasoning_config({"provider": "p", "model": "m"}) is None diff --git a/tests/agent/test_summarize_tool_result_type_safety.py b/tests/agent/test_summarize_tool_result_type_safety.py new file mode 100644 index 000000000000..8cfa5abb6557 --- /dev/null +++ b/tests/agent/test_summarize_tool_result_type_safety.py @@ -0,0 +1,286 @@ +"""Type safety tests for _summarize_tool_result. + +When LLMs return non-string parameter values (e.g. bool, int, None) in tool +call arguments, _summarize_tool_result() must not crash with TypeError or +AttributeError. This caused an infinite TUI crash loop in production. +""" +import json +import pytest +from agent.context_compressor import _summarize_tool_result + + +class TestTypeSafety: + """Non-string tool arguments must not crash _summarize_tool_result.""" + + def test_terminal_command_bool(self): + """bool value for 'command' should not raise TypeError.""" + args = json.dumps({"command": True}) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "terminal" in result + assert "True" in result or "exit" in result + + def test_terminal_command_int(self): + """int value for 'command' should not raise TypeError.""" + args = json.dumps({"command": 42}) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "terminal" in result + assert "42" in result + + def test_terminal_command_none(self): + """None value for 'command' should not raise TypeError.""" + args = json.dumps({"command": None}) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "terminal" in result + + def test_write_file_content_bool(self): + """bool value for 'content' should not raise AttributeError.""" + args = json.dumps({"path": "test.txt", "content": False}) + result = _summarize_tool_result("write_file", args, "OK") + assert "write_file" in result + assert "test.txt" in result + + def test_write_file_content_int(self): + """int value for 'content' should not raise AttributeError.""" + args = json.dumps({"path": "test.txt", "content": 123}) + result = _summarize_tool_result("write_file", args, "OK") + assert "write_file" in result + + def test_delegate_task_goal_bool(self): + """bool value for 'goal' should not raise TypeError.""" + args = json.dumps({"goal": False}) + result = _summarize_tool_result("delegate_task", args, "result") + assert "delegate_task" in result + assert "False" in result + + def test_delegate_task_goal_int(self): + """int value for 'goal' should not raise TypeError.""" + args = json.dumps({"goal": 999}) + result = _summarize_tool_result("delegate_task", args, "result") + assert "delegate_task" in result + assert "999" in result + + def test_execute_code_code_bool(self): + """bool value for 'code' should not raise TypeError.""" + args = json.dumps({"code": True}) + result = _summarize_tool_result("execute_code", args, "output") + assert "execute_code" in result + assert "True" in result + + def test_execute_code_code_int(self): + """int value for 'code' should not raise TypeError.""" + args = json.dumps({"code": 0}) + result = _summarize_tool_result("execute_code", args, "output") + assert "execute_code" in result + + def test_vision_analyze_question_bool(self): + """bool value for 'question' should not raise TypeError.""" + args = json.dumps({"question": True}) + result = _summarize_tool_result("vision_analyze", args, "analysis") + assert "vision_analyze" in result + assert "True" in result + + def test_vision_analyze_question_int(self): + """int value for 'question' should not raise TypeError.""" + args = json.dumps({"question": 123}) + result = _summarize_tool_result("vision_analyze", args, "analysis") + assert "vision_analyze" in result + assert "123" in result + + def test_vision_analyze_question_list(self): + """list value for 'question' should not raise TypeError.""" + args = json.dumps({"question": ["a", "b"]}) + result = _summarize_tool_result("vision_analyze", args, "analysis") + assert "vision_analyze" in result + + def test_vision_analyze_question_dict(self): + """dict value for 'question' should not raise TypeError.""" + args = json.dumps({"question": {"key": "value"}}) + result = _summarize_tool_result("vision_analyze", args, "analysis") + assert "vision_analyze" in result + + +class TestNormalStringArguments: + """Normal string arguments should continue to work as before.""" + + def test_terminal_normal_command(self): + """Normal string command should be summarized correctly.""" + args = json.dumps({"command": "ls -la"}) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "terminal" in result + assert "ls -la" in result + assert "exit 0" in result + + def test_terminal_long_command_truncated(self): + """Long commands should be truncated.""" + long_cmd = "a" * 100 + args = json.dumps({"command": long_cmd}) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "..." in result + assert len(result) < 150 + + def test_write_file_normal_content(self): + """Normal string content should count lines correctly.""" + args = json.dumps({"path": "test.py", "content": "line1\nline2\nline3"}) + result = _summarize_tool_result("write_file", args, "OK") + assert "write_file" in result + assert "test.py" in result + assert "3 lines" in result + + def test_delegate_task_normal_goal(self): + """Normal string goal should be summarized correctly.""" + args = json.dumps({"goal": "Fix the bug"}) + result = _summarize_tool_result("delegate_task", args, "done") + assert "delegate_task" in result + assert "Fix the bug" in result + + def test_delegate_task_long_goal_truncated(self): + """Long goals should be truncated.""" + long_goal = "x" * 100 + args = json.dumps({"goal": long_goal}) + result = _summarize_tool_result("delegate_task", args, "done") + assert "..." in result + + def test_execute_code_normal_code(self): + """Normal code should be previewed correctly.""" + args = json.dumps({"code": "print('hello')"}) + result = _summarize_tool_result("execute_code", args, "hello") + assert "execute_code" in result + assert "print" in result + + def test_execute_code_long_code_truncated(self): + """Long code should be truncated.""" + long_code = "a = 1\n" * 20 + args = json.dumps({"code": long_code}) + result = _summarize_tool_result("execute_code", args, "output") + assert "..." in result + + def test_vision_analyze_normal_question(self): + """Normal question should be included.""" + args = json.dumps({"question": "What is this?"}) + result = _summarize_tool_result("vision_analyze", args, "It's a cat") + assert "vision_analyze" in result + assert "What is this?" in result + + def test_vision_analyze_long_question_truncated(self): + """Long questions should be truncated.""" + long_q = "?" * 100 + args = json.dumps({"question": long_q}) + result = _summarize_tool_result("vision_analyze", args, "answer") + assert len(result) < 150 + + +class TestEdgeCases: + """Edge cases and boundary conditions.""" + + def test_empty_args(self): + """Empty JSON object should not crash.""" + result = _summarize_tool_result("terminal", "{}", "output") + assert "terminal" in result + + def test_invalid_json(self): + """Invalid JSON should not crash.""" + result = _summarize_tool_result("terminal", "not json", "output") + assert "terminal" in result + + def test_null_args(self): + """None/null args should not crash.""" + result = _summarize_tool_result("terminal", None, "output") + assert "terminal" in result + + def test_non_dict_json_args(self): + """Args that parse to a non-dict (list/scalar) should not crash.""" + result = _summarize_tool_result("terminal", json.dumps([1, 2]), "output") + assert "terminal" in result + result = _summarize_tool_result("terminal", json.dumps("bare"), "output") + assert "terminal" in result + + def test_unknown_tool_name(self): + """Unknown tool name should return generic summary.""" + args = json.dumps({"foo": "bar"}) + result = _summarize_tool_result("unknown_tool", args, "output") + # Should return some fallback, not crash + assert isinstance(result, str) + + def test_mixed_types_in_args(self): + """Args with mixed types should not crash.""" + args = json.dumps({ + "command": "ls", + "background": True, + "timeout": 30, + "extra": None + }) + result = _summarize_tool_result("terminal", args, '{"exit_code": 0}') + assert "terminal" in result + assert "ls" in result + + +class TestBackstopWrapper: + """The outer guard: NO input shape may raise out of _summarize_tool_result. + + Compression retries on the same persisted history, so an escaping + exception here becomes a crash loop. The wrapper returns a minimal + '[tool] (N chars result)' summary when a branch fails. + """ + + def test_never_raises_matrix(self): + """Fuzz the per-tool branches with hostile value shapes.""" + hostile_values = [None, True, 42, 3.14, ["a"], {"k": "v"}] + tools = [ + "terminal", "read_file", "write_file", "search_files", "patch", + "browser_navigate", "web_search", "web_extract", "delegate_task", + "execute_code", "skill_view", "vision_analyze", "memory", + "cronjob", "process", "totally_unknown_tool", + ] + keys = ["command", "path", "content", "pattern", "url", "query", + "urls", "goal", "code", "name", "question", "action", + "target", "session_id", "mode", "offset", "ref"] + for tool in tools: + for value in hostile_values: + args = json.dumps({k: value for k in keys}) + result = _summarize_tool_result(tool, args, "x" * 250) + assert isinstance(result, str) and result, (tool, value) + + def test_backstop_fallback_shape(self): + """When a branch does fail, the fallback names the tool and size.""" + from unittest.mock import patch as _patch + with _patch( + "agent.context_compressor._summarize_tool_result_unguarded", + side_effect=TypeError("boom"), + ): + result = _summarize_tool_result("terminal", "{}", "y" * 300) + assert result == "[terminal] (300 chars result)" + + def test_backstop_handles_non_string_content(self): + from unittest.mock import patch as _patch + with _patch( + "agent.context_compressor._summarize_tool_result_unguarded", + side_effect=TypeError("boom"), + ): + result = _summarize_tool_result("terminal", "{}", None) + assert result == "[terminal] (0 chars result)" + + +class TestDisplayPreviewTypeSafety: + """Sibling site: agent/display.py previews run on the live + tool-progress callback and crashed on non-string process args.""" + + def test_process_preview_non_string_session_id(self): + from agent.display import build_tool_preview + assert build_tool_preview("process", {"action": "poll", "session_id": 123}) == "poll 123" + + def test_process_preview_non_string_data(self): + from agent.display import build_tool_preview + result = build_tool_preview( + "process", {"action": "submit", "session_id": "abc", "data": 42} + ) + assert result == 'submit abc "42"' + + def test_process_preview_none_action(self): + from agent.display import build_tool_preview + result = build_tool_preview("process", {"action": None, "session_id": "abc"}) + assert isinstance(result, str) + + def test_process_label_non_string_session_id(self): + from agent.display import build_tool_label + result = build_tool_label("process", {"action": "poll", "session_id": 123}) + assert isinstance(result, str) diff --git a/tests/hermes_cli/test_model_catalog.py b/tests/hermes_cli/test_model_catalog.py index b464fb046a9a..a156ffbaef4e 100644 --- a/tests/hermes_cli/test_model_catalog.py +++ b/tests/hermes_cli/test_model_catalog.py @@ -288,6 +288,68 @@ class TestCuratedAccessors: assert model_catalog.get_curated_nous_models() is None +class TestDefaultModelFromCache: + """get_default_model_from_cache reads the '"default": true' label without + ever hitting the network.""" + + def _manifest_with_default(self) -> dict: + m = _valid_manifest() + m["providers"]["openrouter"]["models"][1]["default"] = True # gpt-5.4 + m["providers"]["nous"]["models"][1]["default"] = True # kimi-k2.6 + return m + + def test_reads_label_from_disk_cache(self, isolated_home): + from hermes_cli import model_catalog + cache = isolated_home / "cache" + cache.mkdir() + (cache / "model_catalog.json").write_text( + json.dumps(self._manifest_with_default()) + ) + with patch.object(model_catalog, "_fetch_manifest") as fetch: + assert ( + model_catalog.get_default_model_from_cache("openrouter") + == "openai/gpt-5.4" + ) + assert ( + model_catalog.get_default_model_from_cache("nous") + == "moonshotai/kimi-k2.6" + ) + fetch.assert_not_called() + + def test_no_label_returns_none(self, isolated_home): + from hermes_cli import model_catalog + cache = isolated_home / "cache" + cache.mkdir() + (cache / "model_catalog.json").write_text(json.dumps(_valid_manifest())) + with patch.object(model_catalog, "_fetch_manifest") as fetch: + assert model_catalog.get_default_model_from_cache("openrouter") is None + fetch.assert_not_called() + + def test_no_cache_returns_none_without_network(self, isolated_home): + from hermes_cli import model_catalog + with patch.object(model_catalog, "_fetch_manifest") as fetch: + assert model_catalog.get_default_model_from_cache("openrouter") is None + fetch.assert_not_called() + + def test_shipped_manifest_labels_glm52_default(self, isolated_home): + """Contract with the in-repo manifest: both provider blocks label the + same default entry the code constant points at.""" + import hermes_cli.model_catalog as model_catalog + from hermes_cli.models import PREFERRED_SILENT_DEFAULT_MODEL + + repo_root = Path(model_catalog.__file__).resolve().parent.parent + manifest = json.loads( + (repo_root / "website" / "static" / "api" / "model-catalog.json").read_text() + ) + for provider in ("openrouter", "nous"): + block = manifest["providers"][provider] + labeled = [m["id"] for m in block["models"] if m.get("default")] + assert labeled == [PREFERRED_SILENT_DEFAULT_MODEL], ( + f"{provider}: exactly one entry must be labeled default and it " + f"must match PREFERRED_SILENT_DEFAULT_MODEL" + ) + + class TestDisabled: def test_disabled_config_short_circuits(self, isolated_home): from hermes_cli import model_catalog diff --git a/tests/hermes_cli/test_models.py b/tests/hermes_cli/test_models.py index bda4807d8094..ca5fb8219ea9 100644 --- a/tests/hermes_cli/test_models.py +++ b/tests/hermes_cli/test_models.py @@ -82,7 +82,10 @@ class TestFetchOpenRouterModels: def test_falls_back_to_static_snapshot_on_fetch_failure(self, monkeypatch): monkeypatch.setattr(_models_mod, "_openrouter_catalog_cache", None) - with patch("hermes_cli.models._urlopen_model_catalog_request", side_effect=OSError("boom")): + # Pin the remote manifest out too — otherwise the fallback silently + # depends on whatever the deployed catalog currently contains. + with patch("hermes_cli.model_catalog.get_curated_openrouter_models", return_value=None), \ + patch("hermes_cli.models._urlopen_model_catalog_request", side_effect=OSError("boom")): models = fetch_openrouter_models(force_refresh=True) assert models == OPENROUTER_MODELS diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 51115fc1d5e3..93823144a19b 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -2359,15 +2359,16 @@ def _codex_reasoning_only_response(*, encrypted_content="enc_abc123", summary_te def test_normalize_codex_response_marks_reasoning_only_as_incomplete(monkeypatch): - """A response with only reasoning items and no content should be 'incomplete', not 'stop'. + """A response with only reasoning items and no content should be 'incomplete' for Codex backends. - Without this fix, reasoning-only responses get finish_reason='stop' which - sends them into the empty-content retry loop (3 retries then failure). + Codex CLI uses reasoning-only responses as a signal that the model is still + thinking and needs another turn. This test verifies the Codex-specific path + where issuer_kind="codex_backend" preserves the old behavior. """ agent = _build_agent(monkeypatch) from agent.codex_responses_adapter import _normalize_codex_response assistant_message, finish_reason = _normalize_codex_response( - _codex_reasoning_only_response() + _codex_reasoning_only_response(), issuer_kind="codex_backend" ) assert finish_reason == "incomplete" @@ -2377,6 +2378,74 @@ def test_normalize_codex_response_marks_reasoning_only_as_incomplete(monkeypatch assert assistant_message.codex_reasoning_items[0]["encrypted_content"] == "enc_abc123" +def test_normalize_codex_response_reasoning_only_completed_is_stop_for_other_backends(monkeypatch): + """Reasoning-only with status='completed' should be 'stop' for non-Codex backends. + + When response.status == "completed" and no items are queued/in_progress, + reasoning alone is a valid final state for non-Codex backends. Forcing + "incomplete" here causes multi-minute stalls (3 retries x up to 240s each). + See https://github.com/NousResearch/hermes-agent/issues/64434 + """ + agent = _build_agent(monkeypatch) + from agent.codex_responses_adapter import _normalize_codex_response + response = _codex_reasoning_only_response() + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="other:example-relay" + ) + + assert finish_reason == "stop" + assert assistant_message.content == "" + assert assistant_message.codex_reasoning_items is not None + assert len(assistant_message.codex_reasoning_items) == 1 + + +def test_normalize_codex_response_reasoning_only_completed_is_stop_without_issuer(monkeypatch): + """Default issuer (None) should also trust response.status='completed' for reasoning-only. + + When no issuer_kind is provided (test or default scenario) and the provider + says status='completed', reasoning-only should be treated as 'stop'. + """ + agent = _build_agent(monkeypatch) + from agent.codex_responses_adapter import _normalize_codex_response + response = _codex_reasoning_only_response() + assistant_message, finish_reason = _normalize_codex_response(response) + + assert finish_reason == "stop" + assert assistant_message.content == "" + + +def test_normalize_codex_response_reasoning_only_stays_incomplete_for_xai_backend(monkeypatch): + """xAI backend also preserves incomplete for reasoning-only (same as Codex).""" + agent = _build_agent(monkeypatch) + from agent.codex_responses_adapter import _normalize_codex_response + response = _codex_reasoning_only_response() + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="xai_responses" + ) + + assert finish_reason == "incomplete" + assert assistant_message.content == "" + + +def test_normalize_codex_response_reasoning_only_stays_incomplete_for_github_backend(monkeypatch): + """GitHub/Copilot Responses backend preserves incomplete for reasoning-only. + + Copilot fronts the same OpenAI model family as codex_backend and exhibits + the same reasoning-only "still thinking" degeneration mode, so it must + stay on the continuation path — only unrecognized (other:*) backends + trust response.status='completed' as terminal. + """ + agent = _build_agent(monkeypatch) + from agent.codex_responses_adapter import _normalize_codex_response + response = _codex_reasoning_only_response() + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="github_responses" + ) + + assert finish_reason == "incomplete" + assert assistant_message.content == "" + + def test_normalize_codex_response_reasoning_with_content_is_stop(monkeypatch): """If a response has both reasoning and message content, it should still be 'stop'.""" agent = _build_agent(monkeypatch) @@ -2806,3 +2875,72 @@ def test_run_conversation_codex_invalid_encrypted_content_without_replay_state_d assert all(not any(item.get("type") == "reasoning" for item in payload["input"]) for payload in request_payloads) assert agent._codex_reasoning_replay_enabled is True assert result["messages"][0].get("codex_reasoning_items") is None + + +def test_run_conversation_codex_nudges_after_unreplayable_reasoning_only_interim(monkeypatch): + """A reasoning-only interim with NO encrypted_content (the shape + grok-4.20 on xai-oauth returns when it never emits a message output + item) replays as nothing — without a nudge every continuation request + is byte-identical to the one that just came back incomplete.""" + agent = _build_agent(monkeypatch) + requests = [] + responses = [ + _codex_reasoning_only_response( + encrypted_content=None, + summary_text="Thinking about the repo structure...", + ), + _codex_message_response("Final answer."), + ] + + def _fake_api_call(api_kwargs): + requests.append(api_kwargs) + return responses.pop(0) + + monkeypatch.setattr(agent, "_interruptible_api_call", _fake_api_call) + + result = agent.run_conversation("analyze repo") + + assert result["completed"] is True + assert result["final_response"] == "Final answer." + assert len(requests) == 2 + + replay_input = requests[1]["input"] + nudges = [ + item for item in replay_input + if isinstance(item, dict) + and item.get("role") == "user" + and "only internal reasoning" in str(item.get("content")) + ] + assert len(nudges) == 1, ( + "Continuation after an unreplayable reasoning-only interim must " + "append the nudge user message; otherwise the retry request is " + "identical to the one that just failed." + ) + + +def test_run_conversation_codex_no_nudge_for_replayable_interim(monkeypatch): + """An interim that carries visible content replays fine — the nudge + must not fire and pollute the conversation.""" + agent = _build_agent(monkeypatch) + requests = [] + responses = [ + _codex_incomplete_message_response("Partial visible content."), + _codex_message_response("Done."), + ] + + def _fake_api_call(api_kwargs): + requests.append(api_kwargs) + return responses.pop(0) + + monkeypatch.setattr(agent, "_interruptible_api_call", _fake_api_call) + + result = agent.run_conversation("analyze repo") + + assert result["completed"] is True + replay_input = requests[1]["input"] + assert not any( + isinstance(item, dict) + and item.get("role") == "user" + and "only internal reasoning" in str(item.get("content")) + for item in replay_input + ) diff --git a/tests/run_agent/test_wait_state_visibility.py b/tests/run_agent/test_wait_state_visibility.py new file mode 100644 index 000000000000..ade5babcf5c5 --- /dev/null +++ b/tests/run_agent/test_wait_state_visibility.py @@ -0,0 +1,123 @@ +"""Tests for wait-state visibility — the live "what are we waiting on" notices. + +Long provider waits (slow/overloaded backend, no first byte, reasoning model +thinking for minutes) used to leave CLI/TUI/Desktop users staring at a generic +"cogitating..." spinner with no explanation. ``AIAgent._emit_wait_notice`` +rewrites the live spinner/status line (via ``thinking_callback``, bridged to +``thinking.delta`` for TUI/Desktop) and updates the activity tracker (which the +gateway's "⏳ Working — N min" heartbeat includes). +""" + +from __future__ import annotations + +import sys +import time +import types +from types import SimpleNamespace + +import pytest + +# Stub optional heavy imports so run_agent imports cleanly in isolation. +sys.modules.setdefault("fire", types.SimpleNamespace(Fire=lambda *a, **k: None)) +sys.modules.setdefault("firecrawl", types.SimpleNamespace(Firecrawl=object)) +sys.modules.setdefault("fal_client", types.SimpleNamespace()) + + +def _make_agent(tmp_path, monkeypatch, **kwargs): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / ".env").write_text("", encoding="utf-8") + (tmp_path / "config.yaml").write_text("{}\n", encoding="utf-8") + from run_agent import AIAgent + + return AIAgent( + model="test-model", + api_key="sk-dummy", + base_url="https://openrouter.ai/api/v1", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + platform="cli", + **kwargs, + ) + + +def test_emit_wait_notice_updates_spinner_and_activity(tmp_path, monkeypatch): + """The notice reaches the live display callback AND the activity tracker.""" + seen: list = [] + agent = _make_agent(tmp_path, monkeypatch, thinking_callback=seen.append) + + agent._emit_wait_notice("⏳ waiting on test-model — 30s with no response yet") + + assert seen == ["⏳ waiting on test-model — 30s with no response yet"] + summary = agent.get_activity_summary() + assert "waiting on test-model" in summary["last_activity_desc"] + + +def test_emit_wait_notice_without_callback_still_touches_activity(tmp_path, monkeypatch): + """No thinking_callback bound (gateway sessions) — activity still updates.""" + agent = _make_agent(tmp_path, monkeypatch) + agent.thinking_callback = None + + agent._emit_wait_notice("⏳ waiting on test-model — 60s") + + assert "waiting on test-model" in agent.get_activity_summary()["last_activity_desc"] + + +def test_emit_wait_notice_swallows_callback_errors(tmp_path, monkeypatch): + """A broken display callback must never break the API-call wait loop.""" + + def _boom(text): + raise RuntimeError("display exploded") + + agent = _make_agent(tmp_path, monkeypatch, thinking_callback=_boom) + + agent._emit_wait_notice("⏳ waiting") # must not raise + assert "waiting" in agent.get_activity_summary()["last_activity_desc"] + + +def test_nonstream_wait_loop_emits_explained_notice(tmp_path, monkeypatch): + """After ~30s with no response, interruptible_api_call rewrites the live + line with an explanation (model name, elapsed, overload hint, recovery + deadline) instead of a bare 'waiting for non-streaming response'.""" + from agent import chat_completion_helpers as h + + seen: list = [] + agent = _make_agent(tmp_path, monkeypatch, thinking_callback=seen.append) + agent.api_mode = "codex_responses" + monkeypatch.setattr(agent, "_compute_non_stream_stale_timeout", lambda *a, **k: 60.0) + + # Compress the 30s cadence: the loop fires the notice every 100 polls of + # 0.3s; patch the join timeout down via a tiny thread that stays alive + # briefly, and shrink the poll interval by patching time. Simplest + # reliable approach: run a worker that hangs ~1.2s and patch the modulo + # counter trigger by making the loop's join timeout effectively immediate. + dummy_client = SimpleNamespace() + monkeypatch.setattr(agent, "_create_request_openai_client", lambda **k: dummy_client) + monkeypatch.setattr(agent, "_abort_request_openai_client", lambda c, reason=None: None) + monkeypatch.setattr(agent, "_close_request_openai_client", lambda c, reason=None: None) + + stop = {"flag": False} + + def fake_hang(api_kwargs, client=None, on_first_delta=None): + deadline = time.time() + 10 + while time.time() < deadline and not stop["flag"] and not agent._interrupt_requested: + time.sleep(0.02) + raise RuntimeError("connection closed") + + monkeypatch.setattr(agent, "_run_codex_stream", fake_hang) + # TTFB kill at 1s ends the call quickly; the wait notice fires on the + # 100-poll cadence, so to observe it within the 1s window we shrink the + # cadence by patching threading.Thread.join used in the poll loop is + # overkill — instead just verify the TTFB reconnect notice, which flows + # through the same _emit_wait_notice path. + monkeypatch.setenv("HERMES_CODEX_TTFB_TIMEOUT_SECONDS", "1") + + try: + with pytest.raises(TimeoutError): + h.interruptible_api_call(agent, {"model": "gpt-5.5", "input": "hi"}) + finally: + stop["flag"] = True + + reconnect_notices = [s for s in seen if "reconnecting" in s] + assert reconnect_notices, f"expected a reconnect wait-notice, saw: {seen}" + assert "no response from provider" in reconnect_notices[0] diff --git a/tests/test_assistant_ui_tap_compat.py b/tests/test_assistant_ui_tap_compat.py index 57c9e874819f..4be17141c669 100644 --- a/tests/test_assistant_ui_tap_compat.py +++ b/tests/test_assistant_ui_tap_compat.py @@ -23,9 +23,11 @@ updating. The fix pins ``@assistant-ui/store`` (via root ``overrides``) to the last release that targets ``tap@^0.5.x``. This is a *contract* test, not a snapshot: it does not assert specific version -numbers, only that whatever tap the lockfile hoists satisfies every -``@assistant-ui/*`` package's declared tap requirement. It fails if any future -bump reintroduces a split tap requirement across the cluster. +numbers, only that the cluster resolves a single shared tap (wherever npm +places it — hoisted to root, or nested under the ``apps/desktop`` workspace +since the 0.14 bump dropped the ``store`` override) and that this tap satisfies +every ``@assistant-ui/*`` package's declared requirement. It fails if any +future bump reintroduces a split tap version or requirement across the cluster. """ from __future__ import annotations @@ -84,14 +86,30 @@ def _lock_packages() -> dict: return json.load(fh).get("packages", {}) -def _hoisted_tap_version(packages: dict) -> str: - entry = packages.get(f"node_modules/{TAP}") - assert entry is not None, ( - "package-lock.json has no hoisted node_modules/@assistant-ui/tap " - "entry — the @assistant-ui cluster should resolve a single shared " - "tap version." +def _shared_tap_version(packages: dict) -> str: + """The one tap version every install site resolves to. + + npm hoists tap to root ``node_modules`` when the cluster lives at the top + level, but nests the whole cluster under a workspace + (``apps/desktop/node_modules``) when only that workspace depends on it — as + it does since the 0.14 bump dropped the ``@assistant-ui/store`` override. + Either layout is fine; the invariant is that a single tap version is shared + across every install site, so ``vite build`` never hits a split API. + """ + versions = { + meta["version"] + for key, meta in packages.items() + if key.rsplit("node_modules/", 1)[-1] == TAP + } + assert versions, ( + "package-lock.json has no @assistant-ui/tap entry — the " + "@assistant-ui cluster should resolve a single shared tap version." ) - return entry["version"] + assert len(versions) == 1, ( + f"@assistant-ui/tap resolves to multiple versions {sorted(versions)} — " + "the cluster must share one tap line (see this test's docstring)." + ) + return versions.pop() def test_assistant_ui_cluster_agrees_on_one_tap() -> None: @@ -103,7 +121,7 @@ def test_assistant_ui_cluster_agrees_on_one_tap() -> None: (or a similar API split) breaks ``vite build``. """ packages = _lock_packages() - tap_version = _hoisted_tap_version(packages) + tap_version = _shared_tap_version(packages) offenders: list[str] = [] for key, meta in packages.items(): diff --git a/tests/test_empty_model_fallback.py b/tests/test_empty_model_fallback.py index 410211f06faa..53260f655ed9 100644 --- a/tests/test_empty_model_fallback.py +++ b/tests/test_empty_model_fallback.py @@ -38,14 +38,14 @@ class TestGetDefaultModelForProvider: def test_nous_silent_default_is_not_the_expensive_flagship(self): """Nous Portal is a metered aggregator whose curated list is ordered most-capable-first, so entry [0] is the priciest flagship - (anthropic/claude-opus-4.8). The silent fallback (provider set, no model) + (anthropic/claude-fable-5). The silent fallback (provider set, no model) must NOT escalate to it — otherwise an unconfigured profile silently bills the most expensive model. Regression for the billing footgun. """ from hermes_cli.models import ( _PROVIDER_MODELS, - _PROVIDER_SILENT_DEFAULT_OVERRIDES, get_default_model_for_provider, + get_preferred_silent_default_model, ) result = get_default_model_for_provider("nous") @@ -53,27 +53,108 @@ class TestGetDefaultModelForProvider: assert "opus" not in result.lower(), ( f"silent default escalated to an expensive flagship: {result!r}" ) + assert "claude" not in result.lower(), ( + f"silent default escalated to an expensive flagship: {result!r}" + ) assert result != _PROVIDER_MODELS["nous"][0], ( "silent default must not be the most-capable/priciest catalog entry" ) - # The override must point at a model that actually exists in the catalog. - assert result == _PROVIDER_SILENT_DEFAULT_OVERRIDES["nous"] + # The default must resolve through the catalog-label helper and point + # at a model that actually exists in the curated catalog. + assert result == get_preferred_silent_default_model("nous") assert result in _PROVIDER_MODELS["nous"] - def test_override_falls_back_to_catalog_when_missing(self): - """If an override model is no longer in the catalog, fall back to [0] - rather than returning a stale/absent id.""" + def test_catalog_label_overrides_constant(self): + """A ``"default": true`` label in the cached catalog manifest wins over + the in-repo constant, so maintainers can rotate the silent default + without shipping a release.""" from unittest.mock import patch from hermes_cli import models as models_mod - with patch.dict( - models_mod._PROVIDER_SILENT_DEFAULT_OVERRIDES, - {"openai-codex": "does-not-exist-model"}, - clear=False, + with patch( + "hermes_cli.model_catalog.get_default_model_from_cache", + return_value="qwen/qwen3.7-plus", ): - result = models_mod.get_default_model_for_provider("openai-codex") - assert result == models_mod._PROVIDER_MODELS["openai-codex"][0] + assert ( + models_mod.get_preferred_silent_default_model("nous") + == "qwen/qwen3.7-plus" + ) + # nous catalog carries qwen3.7-plus, so the full resolver follows. + assert ( + models_mod.get_default_model_for_provider("nous") + == "qwen/qwen3.7-plus" + ) + + def test_no_catalog_cache_falls_back_to_constant(self): + """With no cached manifest (fresh install / offline), the in-repo + constant is the silent default.""" + from unittest.mock import patch + + from hermes_cli import models as models_mod + + with patch( + "hermes_cli.model_catalog.get_default_model_from_cache", + return_value=None, + ): + assert ( + models_mod.get_preferred_silent_default_model("openrouter") + == models_mod.PREFERRED_SILENT_DEFAULT_MODEL + ) + + def test_stale_label_not_in_catalog_falls_back(self): + """If the labeled default model is no longer in the provider's curated + catalog, fall back to entry [0] rather than returning an absent id.""" + from unittest.mock import patch + + from hermes_cli import models as models_mod + + with patch( + "hermes_cli.model_catalog.get_default_model_from_cache", + return_value="does-not-exist-model", + ): + result = models_mod.get_default_model_for_provider("nous") + assert result == models_mod._PROVIDER_MODELS["nous"][0] + + +class TestDetectStaticProviderCostSafeDefault: + """detect_static_provider_for_model must apply the same cost-safe default + as get_default_model_for_provider when a bare provider name is typed as a + model (e.g. ``/model nous``).""" + + def test_bare_nous_does_not_escalate_to_flagship(self): + from hermes_cli.models import ( + _PROVIDER_MODELS, + get_default_model_for_provider, + detect_static_provider_for_model, + ) + + result = detect_static_provider_for_model("nous", "openrouter") + assert result is not None + provider, model = result + assert provider == "nous" + # Must match the cost-safe silent default, NOT the priciest catalog + # entry [0]. Regression: this path returned _PROVIDER_MODELS["nous"][0] + # directly, re-introducing the billing footgun on the interactive + # ``/model nous`` path. + assert model == get_default_model_for_provider("nous") + assert "opus" not in model.lower() + assert model != _PROVIDER_MODELS["nous"][0] + + def test_provider_without_override_still_uses_first_model(self): + """Providers outside _SILENT_DEFAULT_PROVIDERS are unchanged.""" + from hermes_cli.models import ( + _PROVIDER_MODELS, + _SILENT_DEFAULT_PROVIDERS, + detect_static_provider_for_model, + ) + + for provider in ("anthropic", "xai"): + if provider in _SILENT_DEFAULT_PROVIDERS: + continue + result = detect_static_provider_for_model(provider, "openrouter") + assert result is not None + assert result[1] == _PROVIDER_MODELS[provider][0] class TestGatewayEmptyModelFallback: diff --git a/tests/tools/test_async_delegation.py b/tests/tools/test_async_delegation.py index cc4f7651d364..37aae286394a 100644 --- a/tests/tools/test_async_delegation.py +++ b/tests/tools/test_async_delegation.py @@ -25,6 +25,13 @@ def _clean_state(): while not process_registry.completion_queue.empty(): process_registry.completion_queue.get_nowait() yield + # Give just-released workers a beat to finalize BEFORE draining, so their + # completion events land now instead of leaking into the next test's + # queue (worker threads push events asynchronously; a drain that races an + # in-flight _finalize misses it). + deadline = time.monotonic() + 2.0 + while ad.active_count() and time.monotonic() < deadline: + time.sleep(0.02) ad._reset_for_tests() while not process_registry.completion_queue.empty(): process_registry.completion_queue.get_nowait() @@ -39,11 +46,30 @@ def _drain_one(timeout=5.0): return None +def _drain_for(delegation_id, timeout=5.0): + """Drain until the event for *delegation_id* appears (discarding others). + + Completion events are pushed asynchronously by worker threads, so a + straggler from a PREVIOUS test can land after that test's teardown drain + and leak into the current test's queue. Matching on delegation_id makes + the assertion immune to that cross-test leak. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not process_registry.completion_queue.empty(): + evt = process_registry.completion_queue.get_nowait() + if evt.get("delegation_id") == delegation_id: + return evt + continue + time.sleep(0.02) + return None + + def test_dispatch_returns_immediately_without_blocking(): gate = threading.Event() def runner(): - gate.wait(timeout=5) + gate.wait(timeout=60) return {"status": "completed", "summary": "done", "api_calls": 1, "duration_seconds": 0.1, "model": "m"} @@ -70,7 +96,7 @@ def test_async_executor_workers_are_daemon_threads(): gate = threading.Event() def runner(): - gate.wait(timeout=5) + gate.wait(timeout=60) return {"status": "completed", "summary": "done"} res = ad.dispatch_async_delegation( @@ -148,7 +174,7 @@ def test_dispatch_rejected_at_capacity(): ev = threading.Event() def blocker(): - ev.wait(timeout=5) + ev.wait(timeout=60) return {"status": "completed", "summary": "x"} for i in range(2): @@ -170,9 +196,15 @@ def test_dispatch_rejected_at_capacity(): def test_interrupt_all_signals_running_children(): ev = threading.Event() interrupted = {"count": 0} + # No short internal timeout: the blocker holds until interrupt_fn fires. + # The old ev.wait(timeout=5) made this test a change-detector for CI + # worker load — on a CPU-starved runner the 5s expired before + # interrupt_all() ran, the record finalized, and interrupt_all() found + # nothing running (n == 0). The pytest-level timeout is the real + # runaway guard. def blocker(): - ev.wait(timeout=5) + ev.wait(timeout=60) return {"status": "interrupted", "summary": None, "error": "cancelled"} @@ -180,7 +212,7 @@ def test_interrupt_all_signals_running_children(): interrupted["count"] += 1 ev.set() - ad.dispatch_async_delegation( + r = ad.dispatch_async_delegation( goal="long task", context=None, toolsets=None, role="leaf", model="m", session_key="", runner=blocker, interrupt_fn=interrupt_fn, max_async_children=3, @@ -188,8 +220,11 @@ def test_interrupt_all_signals_running_children(): n = ad.interrupt_all(reason="test") assert n == 1 assert interrupted["count"] == 1 - # child still emits a completion event after interrupt - evt = _drain_one() + # child still emits a completion event after interrupt. Match on THIS + # delegation's id — straggler 'completed' events from a previous test's + # workers can finalize after that test's teardown drain and leak into + # this queue (observed on loaded CI workers). + evt = _drain_for(r["delegation_id"]) assert evt is not None assert evt["status"] == "interrupted" @@ -408,7 +443,7 @@ def test_delegate_task_background_routes_async_and_does_not_block(monkeypatch): gate = threading.Event() def slow_child(task_index, goal, child=None, parent_agent=None, **kw): - gate.wait(timeout=5) # a sync impl would hang delegate_task here + gate.wait(timeout=60) # a sync impl would hang delegate_task here return { "task_index": 0, "status": "completed", "summary": f"done: {goal}", "api_calls": 1, "duration_seconds": 0.1, "model": "m", @@ -536,7 +571,7 @@ def test_delegate_task_background_batch_runs_as_one_unit(monkeypatch): gate = threading.Event() def _blocking_child(task_index, goal, child=None, parent_agent=None, **kw): - gate.wait(timeout=5) + gate.wait(timeout=60) return { "task_index": task_index, "status": "completed", "summary": f"done: {goal}", "api_calls": 1, @@ -690,7 +725,7 @@ def test_delegate_task_background_detaches_child_from_parent(monkeypatch): gate = threading.Event() def slow_child(task_index, goal, child=None, parent_agent=None, **kw): - gate.wait(timeout=5) + gate.wait(timeout=60) return {"task_index": 0, "status": "completed", "summary": "ok"} def build_and_register(**kw): @@ -722,7 +757,7 @@ def test_concurrent_dispatch_respects_capacity(): gate = threading.Event() def blocker(): - gate.wait(timeout=5) + gate.wait(timeout=60) return {"status": "completed", "summary": "x"} results = [] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 471ca788fb03..febb4ec0d800 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2118,11 +2118,12 @@ def _resolve_model() -> str: if isinstance(m, str) and m: return m.strip() # No env seed and no config preference: fall back to the cost-safe silent - # default, never an expensive Anthropic flagship the user didn't pick. + # default (catalog-labeled, cache-only read), never an expensive Anthropic + # flagship the user didn't pick. try: - from hermes_cli.models import PREFERRED_SILENT_DEFAULT_MODEL + from hermes_cli.models import get_preferred_silent_default_model - return PREFERRED_SILENT_DEFAULT_MODEL + return get_preferred_silent_default_model() except Exception: return "z-ai/glm-5.2" diff --git a/website/docs/reference/model-catalog.md b/website/docs/reference/model-catalog.md index 4e44543354fd..71ed187d72d6 100644 --- a/website/docs/reference/model-catalog.md +++ b/website/docs/reference/model-catalog.md @@ -29,6 +29,7 @@ Published on every merge to `main` via the existing `deploy-site.yml` GitHub Pag "openrouter": { "metadata": {}, "models": [ + {"id": "z-ai/glm-5.2", "description": "default", "default": true}, {"id": "moonshotai/kimi-k2.6", "description": "recommended", "metadata": {}}, {"id": "openai/gpt-5.4", "description": ""} ] @@ -36,6 +37,7 @@ Published on every merge to `main` via the existing `deploy-site.yml` GitHub Pag "nous": { "metadata": {}, "models": [ + {"id": "z-ai/glm-5.2", "default": true}, {"id": "anthropic/claude-opus-4.7"}, {"id": "moonshotai/kimi-k2.6"} ] @@ -48,7 +50,8 @@ Field notes: - **`version`** — integer schema version. Future schemas bump this; Hermes refuses manifests with versions it doesn't understand and falls back to the hardcoded snapshot. - **`metadata`** — free-form dict at the manifest, provider, and model level. Any keys. Hermes ignores unknown fields, so you can annotate entries (`"tier": "paid"`, `"tags": [...]`, etc.) without coordinating a schema change. -- **`description`** — OpenRouter-only. Drives picker badge text (`"recommended"`, `"free"`, or empty). Nous Portal doesn't use this — free-tier gating is determined live from the Portal's pricing endpoint. +- **`description`** — OpenRouter-only. Drives picker badge text (`"recommended"`, `"free"`, `"default"`, or empty). Nous Portal doesn't use this — free-tier gating is determined live from the Portal's pricing endpoint. +- **`default`** — exactly one entry per provider may carry `"default": true`. That model is the **silent default**: what Hermes lands on when the user never selected a model (GUI onboarding confirm card, `provider` configured with no `model`, empty `model.default`). Read cache-only at runtime (`get_default_model_from_cache`) so hot resolution paths never hit the network; when no cached manifest exists, Hermes falls back to the in-repo `PREFERRED_SILENT_DEFAULT_MODEL` constant, which must match the labeled entry. This lets maintainers rotate the silent default without shipping a release. It is deliberately a capable low-cost model, never the priciest flagship. - **Pricing and context length** are NOT in the manifest. Those come from live provider APIs (`/v1/models` endpoints, models.dev) at fetch time. ## Fetch behavior diff --git a/website/static/api/model-catalog.json b/website/static/api/model-catalog.json index b829b00eb03d..07fca06d6964 100644 --- a/website/static/api/model-catalog.json +++ b/website/static/api/model-catalog.json @@ -1,6 +1,6 @@ { "version": 1, - "updated_at": "2026-07-09T18:38:59Z", + "updated_at": "2026-07-15T04:51:12Z", "metadata": { "source": "hermes-agent repo", "docs": "https://hermes-agent.nousresearch.com/docs/reference/model-catalog" @@ -9,7 +9,7 @@ "openrouter": { "metadata": { "display_name": "OpenRouter", - "note": "Descriptions drive picker badges. Live /api/v1/models filters curated ids by tool-calling support and free pricing." + "note": "Descriptions drive picker badges. Live /api/v1/models filters curated ids by tool-calling support and free pricing. The entry labeled \"default\": true is the model Hermes silently lands on when the user never picked one." }, "models": [ { @@ -118,7 +118,8 @@ }, { "id": "z-ai/glm-5.2", - "description": "" + "description": "default", + "default": true }, { "id": "z-ai/glm-5.1", @@ -177,7 +178,7 @@ "nous": { "metadata": { "display_name": "Nous Portal", - "note": "Free-tier gating is determined live via Portal pricing (partition_nous_models_by_tier), not this manifest." + "note": "Free-tier gating is determined live via Portal pricing (partition_nous_models_by_tier), not this manifest. The entry labeled \"default\": true is the model Hermes silently lands on when the user never picked one." }, "models": [ { @@ -256,7 +257,8 @@ "id": "minimax/minimax-m3" }, { - "id": "z-ai/glm-5.2" + "id": "z-ai/glm-5.2", + "default": true }, { "id": "z-ai/glm-5.1"