From 91d8e4117c393ccb91b18ede7504c6803898c7f9 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 15 Jul 2026 02:34:15 -0400 Subject: [PATCH 01/17] fix(tests): make @assistant-ui tap invariant workspace-nesting aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 0.14 upgrade (#63970) dropped the @assistant-ui/store override, so the whole cluster de-hoisted from root node_modules into apps/desktop/node_modules under a single shared tap@0.9.3. The test only looked at the root hoist path (node_modules/@assistant-ui/tap), which no longer exists, and failed on main. Resolve tap wherever npm places it (root or workspace-nested), and assert a single shared version across all install sites — strengthening the invariant to also catch a split tap install, not just split declared ranges. --- tests/test_assistant_ui_tap_compat.py | 40 +++++++++++++++++++-------- 1 file changed, 29 insertions(+), 11 deletions(-) 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(): From f5c2ea49a4716ccd377c99511ee1bb9be1b56275 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:36:37 -0700 Subject: [PATCH 02/17] test: deflake async-delegation interrupt test under CI load (#64767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: deflake async-delegation interrupt test under CI load test_interrupt_all_signals_running_children failed twice on a loaded CI worker: the blocker's ev.wait(timeout=5) expired before interrupt_all() ran, the record finalized on its own, and interrupt_all() found nothing running (n == 0, interrupted count 0 — the exact CI assertion failure, reproduced locally by inserting a 5.6s dispatch->interrupt gap). Raise the internal safety timeouts from 5s to 60s across the file's gated runners — they exist only as runaway guards (every test releases its gate explicitly); the pytest-level timeout is the real backstop. Same flake class as the removed test_crashed_runner_produces_error_ completion (#64431). * test: fix cross-test completion-event leak (second flake mechanism) The first CI failure was the 5s guard-timeout race; the rerun exposed a SECOND mechanism with a different signature: 'completed' == 'interrupted'. Prior tests (e.g. test_dispatch_rejected_at_capacity) release their gate and return immediately, but their workers finalize asynchronously — on a loaded runner the teardown drain races the in-flight _finalize, and the straggler 'completed' events leak into the NEXT test's queue, where _drain_one() picks one up instead of the interrupt event. Reproduced locally: gate release + immediate drain + 0.15s finalize delay leaked 2 events. Fixes: - teardown waits (bounded 2s) for active workers to finalize before draining, so events land in the owning test - the interrupt test matches its OWN delegation_id via _drain_for() instead of taking whatever event arrives first --- tests/tools/test_async_delegation.py | 57 ++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 11 deletions(-) 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 = [] From dd923619c79a817e238f071da7ff6cf3ab947f2b Mon Sep 17 00:00:00 2001 From: liuwei666888 Date: Thu, 25 Jun 2026 16:51:38 +0800 Subject: [PATCH 03/17] fix: prevent TypeError in _summarize_tool_result when tool args contain non-string values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When LLMs return non-string parameter values (e.g. bool, int) in tool call arguments, _summarize_tool_result() crashes with TypeError because it calls len(), .count(), or slicing directly on args.get() return values. This causes an infinite crash loop in the TUI — context compression triggers on session resume, which crashes, which restarts, which triggers compression again. Add _str_arg() helper that coerces any value to str, and use it in all 5 vulnerable call sites: - terminal: len(cmd) - write_file: content.count() - delegate_task: len(goal) - execute_code: len(code) - vision_analyze: question[:50] --- agent/context_compressor.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index ec4314ab40b0..5c626669c4a2 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. @@ -609,7 +624,7 @@ def _summarize_tool_result(tool_name: str, tool_args: str, tool_content: str) -> 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 +638,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 +673,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 +690,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": From be100d4dae0473486821c026218ad764f77475dc Mon Sep 17 00:00:00 2001 From: liuwei666888 <527711370@qq.com> Date: Thu, 25 Jun 2026 17:43:15 +0800 Subject: [PATCH 04/17] test: add type safety tests for _summarize_tool_result 27 tests covering: - Non-string args (bool, int, None, list, dict) for all 5 vulnerable sites - Normal string args (regression tests) - Edge cases (empty args, invalid JSON, null args, unknown tool) Verifies the fix prevents TypeError/AttributeError crashes. --- .../test_summarize_tool_result_type_safety.py | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 tests/agent/test_summarize_tool_result_type_safety.py 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..fbda82265500 --- /dev/null +++ b/tests/agent/test_summarize_tool_result_type_safety.py @@ -0,0 +1,207 @@ +"""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_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 From 05473428cb0ebbe488ad9c4d473998006131267c Mon Sep 17 00:00:00 2001 From: Frowtek Date: Sat, 11 Jul 2026 13:39:10 +0300 Subject: [PATCH 05/17] fix(acp): don't let a malformed tool argument abort the tool-call render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_tool_start renders every ACP (Zed) tool call — on the live tool-progress callback (acp_adapter/events.py) and during session history replay (acp_adapter/server.py). It called build_tool_title and extract_locations directly, so a model that emits a malformed argument crashed the render: - terminal `command` as null/number -> TypeError (len() in build_tool_title) - delegate_task `goal` as a number -> TypeError (len()) - read_file `path` as a non-string -> pydantic ValidationError building a ToolCallLocation A live crash breaks the tool-call event; a persisted one breaks history replay on every resume of that session. The sibling CLI label builder get_cute_tool_message was already wrapped for exactly this reason (agent/display.py: "display must never abort a turn"). Wrap build_tool_start the same way: on any builder failure, fall back to a minimal, valid start event (tool name as title, resolved kind). The happy path is unchanged. Adds tests for the non-string command, path, and goal cases. --- acp_adapter/tools.py | 35 ++++++++++++++++++++++++++++++++++- tests/acp/test_tools.py | 20 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) 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/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"]} From 3804df5b36aeb2fcb6ae1c2bc38dd53f46c4de01 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:12:54 -0700 Subject: [PATCH 06/17] fix: harden remaining display-layer surfaces against non-string tool args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidation follow-up on top of @liuwei666888's compressor fix (#52431) and @Frowtek's ACP render guard (#62588) — completes the class fix across every display-layer consumer of tool-call arguments: - agent/context_compressor.py: wrap _summarize_tool_result in a never-raises backstop (same pattern as get_cute_tool_message and the ACP guard). The per-branch _str_arg coercions keep summaries informative; the wrapper guarantees compression can never crash-loop on a summary branch we didn't anticipate. Also reject non-dict parsed args (a JSON list/scalar would crash every args.get call). - agent/display.py: build_tool_preview's process branch sliced session_id/data without coercion — crashed build_tool_preview and build_tool_label on the live tool-progress callback (probed live: TypeError 'int' object is not subscriptable). Coerce like the sibling branches. - tests: backstop fuzz matrix (16 tools x 6 hostile value shapes), fallback-shape contracts, display process-preview regressions, non-dict args guard. --- agent/context_compressor.py | 19 +++++ agent/display.py | 7 +- .../test_summarize_tool_result_type_safety.py | 79 +++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 5c626669c4a2..fb998a1dbe3d 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -613,11 +613,30 @@ 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) 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/tests/agent/test_summarize_tool_result_type_safety.py b/tests/agent/test_summarize_tool_result_type_safety.py index fbda82265500..8cfa5abb6557 100644 --- a/tests/agent/test_summarize_tool_result_type_safety.py +++ b/tests/agent/test_summarize_tool_result_type_safety.py @@ -187,6 +187,13 @@ class TestEdgeCases: 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"}) @@ -205,3 +212,75 @@ class TestEdgeCases: 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) From d6c14a952f69fb7c7ce4f01f7470faea09dc9be1 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:28:04 -0700 Subject: [PATCH 07/17] chore(release): map liuwei666888 in AUTHOR_MAP --- scripts/release.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/release.py b/scripts/release.py index 80f9d69ca18f..f246ddd2c86a 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -333,6 +333,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", From a7784f11fb13d66948670fc9375a5e0cc767ac62 Mon Sep 17 00:00:00 2001 From: LeonSGP43 Date: Sat, 11 Jul 2026 21:50:31 -0700 Subject: [PATCH 08/17] fix(codex): keep large-request ttfb watchdog active --- agent/chat_completion_helpers.py | 33 ++++++++++--------- tests/agent/test_codex_ttfb_watchdog.py | 43 +++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index dbc946ce899a..2987784ca700 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( 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.""" From 97375e0f0652f49fd25a998d55964d84f4c44592 Mon Sep 17 00:00:00 2001 From: HumphreySun98 Date: Wed, 8 Jul 2026 16:34:36 -0500 Subject: [PATCH 09/17] fix(models): route bare-provider /model switch through the cost-safe default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `detect_static_provider_for_model` handles a bare provider name typed as a model (e.g. `/model nous`) by returning `_PROVIDER_MODELS[provider][0]` — the first curated entry. For metered aggregators whose curated list is ordered most-capable-first (Nous Portal), entry [0] is the priciest flagship, so `/model nous` silently switched to it. This is exactly the billing footgun `_PROVIDER_SILENT_DEFAULT_OVERRIDES` / `get_default_model_for_provider` exist to prevent (per their docstring, a missing model "escalated to Opus and billed 863 requests before the user noticed"). The non-interactive fallback already routes through that cost-safe helper; the interactive `/model ` path did not. Route this path through `get_default_model_for_provider` too. Providers without a silent-default override are unchanged (the helper returns `models[0]`), so only overridden providers (currently `nous`) change — from the flagship to the low-cost default. Adds regression tests: `/model nous` resolves to the cost-safe default, and non-overridden providers still resolve to their first catalog model. --- hermes_cli/models.py | 13 +++++++++- tests/test_empty_model_fallback.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 9941760aad10..40b64fb58204 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -1976,7 +1976,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 + # ``_PROVIDER_SILENT_DEFAULT_OVERRIDES`` exists to prevent. For + # providers without an override 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/tests/test_empty_model_fallback.py b/tests/test_empty_model_fallback.py index 410211f06faa..335d38df1c10 100644 --- a/tests/test_empty_model_fallback.py +++ b/tests/test_empty_model_fallback.py @@ -76,6 +76,46 @@ class TestGetDefaultModelForProvider: assert result == models_mod._PROVIDER_MODELS["openai-codex"][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 with no silent-default override are unchanged.""" + from hermes_cli.models import ( + _PROVIDER_MODELS, + _PROVIDER_SILENT_DEFAULT_OVERRIDES, + detect_static_provider_for_model, + ) + + for provider in ("anthropic", "xai"): + if provider in _PROVIDER_SILENT_DEFAULT_OVERRIDES: + continue + result = detect_static_provider_for_model(provider, "openrouter") + assert result is not None + assert result[1] == _PROVIDER_MODELS[provider][0] + + class TestGatewayEmptyModelFallback: """Test that _resolve_session_agent_runtime fills in empty model from provider catalog.""" From b34e565957f24b274ec88eb90f2e3575b082855a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:54:41 -0700 Subject: [PATCH 10/17] =?UTF-8?q?feat(models):=20catalog-labeled=20silent?= =?UTF-8?q?=20default=20=E2=80=94=20GLM-5.2=20marked=20"default":=20true?= =?UTF-8?q?=20in=20the=20model=20catalog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote model catalog (website/static/api/model-catalog.json) now labels exactly one entry per provider block with "default": true — z-ai/glm-5.2 for both OpenRouter and Nous Portal. That labeled entry is the model Hermes silently lands on when the user never picked one, and it can be rotated by editing the manifest alone: no release needed. - model_catalog.py: get_default_model_from_cache() reads the label from the in-process/disk cache only — never triggers a network fetch, so hot resolution paths (agent build, gateway session setup) stay network-free. - models.py: get_preferred_silent_default_model() resolves catalog label first, PREFERRED_SILENT_DEFAULT_MODEL constant second (offline/fresh install). _PROVIDER_SILENT_DEFAULT_OVERRIDES dict replaced by _SILENT_DEFAULT_PROVIDERS routing through the shared resolver. fetch_openrouter_models() preserves the "default" badge through live /v1/models refreshes so the picker shows it. - scripts/build_model_catalog.py: generator emits the default label so regeneration can't drop it. - website/docs/reference/model-catalog.md: schema documents the new field. - Salvaged from PR #61141 (@HumphreySun98): bare-provider /model switches (/model nous) route through the cost-safe default instead of curated entry [0]. - tests: catalog-label precedence, constant fallback, stale-label fallback, cache-only (no network) guarantee, and a shipped-manifest contract test pinning the labeled entry to PREFERRED_SILENT_DEFAULT_MODEL. E2E (temp HERMES_HOME): fresh-install constant fallback, shipped-manifest label read, release-free rotation (relabeled cache -> new default across models.py, tui_gateway, and gateway empty-model paths) all verified. --- hermes_cli/model_catalog.py | 36 ++++++++ hermes_cli/models.py | 114 +++++++++++++++--------- hermes_cli/web_server.py | 4 +- scripts/build_model_catalog.py | 33 +++++-- tests/hermes_cli/test_model_catalog.py | 62 +++++++++++++ tests/hermes_cli/test_models.py | 5 +- tests/test_empty_model_fallback.py | 73 +++++++++++---- tui_gateway/server.py | 7 +- website/docs/reference/model-catalog.md | 5 +- website/static/api/model-catalog.json | 12 +-- 10 files changed, 275 insertions(+), 76 deletions(-) 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 40b64fb58204..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) @@ -1980,10 +2008,10 @@ def detect_static_provider_for_model( # ``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 - # ``_PROVIDER_SILENT_DEFAULT_OVERRIDES`` exists to prevent. For - # providers without an override this is unchanged (it returns - # ``models[0]``). + # 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], 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/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/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/test_empty_model_fallback.py b/tests/test_empty_model_fallback.py index 335d38df1c10..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,68 @@ 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: @@ -101,15 +142,15 @@ class TestDetectStaticProviderCostSafeDefault: assert model != _PROVIDER_MODELS["nous"][0] def test_provider_without_override_still_uses_first_model(self): - """Providers with no silent-default override are unchanged.""" + """Providers outside _SILENT_DEFAULT_PROVIDERS are unchanged.""" from hermes_cli.models import ( _PROVIDER_MODELS, - _PROVIDER_SILENT_DEFAULT_OVERRIDES, + _SILENT_DEFAULT_PROVIDERS, detect_static_provider_for_model, ) for provider in ("anthropic", "xai"): - if provider in _PROVIDER_SILENT_DEFAULT_OVERRIDES: + if provider in _SILENT_DEFAULT_PROVIDERS: continue result = detect_static_provider_for_model(provider, "openrouter") assert result is not None 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" From 0a940972f4d8d66cb9a9c49543a86232e1937c05 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:11:04 -0700 Subject: [PATCH 11/17] fix(moa): aggregator resolves reasoning like an acting model when slot is unset (#64756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The aggregator is MoA's acting model, but the main loop's reasoning gates key off the virtual moa://local identity and never fire — so with no per-slot reasoning_effort the aggregator silently ran at the backend default, ignoring the user's reasoning config entirely (#64187). New _aggregator_reasoning_config(): slot value > full acting-model resolution via the shared chokepoint (agent.reasoning_overrides for the slot's model > global agent.reasoning_effort; YAML False stays 'disabled'). Applied to both aggregator call sites (acting turn + one-shot /moa synthesis). Reference advisors intentionally keep slot-or-default: inheriting a global xhigh into every advisor fan-out would silently multiply cost. Fixes #64187. --- agent/moa_loop.py | 34 ++++++++- tests/agent/test_moa_reasoning_effort.py | 90 ++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) 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/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 From 306a3774b68a00e8d70bd94522371e1d82dee4df Mon Sep 17 00:00:00 2001 From: webtecnica <75556242+webtecnica@users.noreply.github.com> Date: Tue, 14 Jul 2026 11:24:55 -0300 Subject: [PATCH 12/17] fix: finish_reason misclassified as incomplete for codex_responses (#64434) --- agent/codex_responses_adapter.py | 23 ++++++-- .../test_run_agent_codex_responses.py | 58 +++++++++++++++++-- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 81c3131eace7..6d41b2a9e8ba 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1380,12 +1380,23 @@ 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 known Codex/xAI backends, 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"): + finish_reason = "stop" + else: + finish_reason = "incomplete" else: finish_reason = "stop" return assistant_message, finish_reason diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 51115fc1d5e3..c0d86ddd731b 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,55 @@ 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_is_stop_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_with_content_is_stop(monkeypatch): """If a response has both reasoning and message content, it should still be 'stop'.""" agent = _build_agent(monkeypatch) From 01b76092529335727f3bd3cf26cd3ac35e024815 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:47:41 -0700 Subject: [PATCH 13/17] fix(codex): keep GitHub/Copilot Responses on the reasoning-only continuation path Follow-up to the salvaged #64449: the status-trusting branch flipped github_responses to 'stop' alongside unknown relays. Copilot fronts the same OpenAI model family as codex_backend and shows the same reasoning-only 'still thinking' degeneration, so it stays on the continuation path. Only unrecognized (other:*) backends trust response.status='completed' as terminal. --- agent/codex_responses_adapter.py | 15 ++++++++----- .../test_run_agent_codex_responses.py | 21 ++++++++++++++++++- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 6d41b2a9e8ba..1d14148c7aeb 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1382,10 +1382,11 @@ def _normalize_codex_response( # Response contains only reasoning (encrypted thinking state and/or # human-readable summary) with no visible content or tool calls. # - # For known Codex/xAI backends, 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 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 @@ -1393,7 +1394,11 @@ def _normalize_codex_response( # 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"): + if response_status == "completed" and issuer_kind not in ( + "codex_backend", + "xai_responses", + "github_responses", + ): finish_reason = "stop" else: finish_reason = "incomplete" diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index c0d86ddd731b..6aa381dd2321 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -2414,7 +2414,7 @@ def test_normalize_codex_response_reasoning_only_completed_is_stop_without_issue assert assistant_message.content == "" -def test_normalize_codex_response_reasoning_only_is_stop_for_xai_backend(monkeypatch): +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 @@ -2427,6 +2427,25 @@ def test_normalize_codex_response_reasoning_only_is_stop_for_xai_backend(monkeyp 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) From 07443ea21a30d7a4a341d3ad01600cf77de3279c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:31:19 -0700 Subject: [PATCH 14/17] test(codex): pin codex_backend issuer in summary-only reasoning sibling test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #64434 change makes unrecognized issuers trust response.status='completed' for reasoning-only turns, so this sibling test (which exercised the old default-path behavior) now pins the Codex backend explicitly — the surface where reasoning-only still means 'still thinking'. --- tests/agent/test_codex_responses_adapter.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/agent/test_codex_responses_adapter.py b/tests/agent/test_codex_responses_adapter.py index 72b8787e02e4..4e8a25a661ba 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 == "" From 05d1ca549be07af9edbf3dd9998a9c73d0180410 Mon Sep 17 00:00:00 2001 From: Ignacio Pastor Date: Mon, 13 Jul 2026 11:07:08 +0200 Subject: [PATCH 15/17] fix(codex): rescue reasoning-only turns that die with 'remained incomplete after 3 continuation attempts' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grok-4.x on the xAI /v1/responses surface sometimes ends a turn with only reasoning items — no message output item, no tool calls — and those reasoning items carry no encrypted_content. Two compounding problems: 1. The model occasionally emits its final answer INSIDE the reasoning channel, delimited by grok's internal "" tag. The answer exists but is classified reasoning-only → finish_reason=incomplete. 2. An interim assistant message holding only plain-text reasoning replays as nothing in _chat_messages_to_responses_input, so every continuation request is byte-identical to the one that just failed. The model deterministically repeats the reasoning-only response until the retry budget is exhausted and the turn dies with "Codex response remained incomplete after 3 continuation attempts". Fixes: - _normalize_codex_response (xai_responses only): salvage the -delimited tail from the reasoning text and promote it to assistant content; the untagged prefix stays as thinking text. - Codex-incomplete continuation path: when the interim message has nothing the input converter will replay (no content, no encrypted reasoning items, no message items), append a user-role nudge so the retry actually differs and explicitly asks for the final answer / pending tool call. Mirrors the existing _get_continuation_prompt pattern used for length truncation. Observed live with grok-4.20 on xai-oauth (2026-07-13); sibling of the grok-composer web_search incomplete-loop fix in transports/codex.py. Co-Authored-By: Claude Fable 5 --- agent/codex_responses_adapter.py | 39 ++++++++++ agent/conversation_loop.py | 42 ++++++++++ tests/agent/test_codex_responses_adapter.py | 78 +++++++++++++++++++ .../test_run_agent_codex_responses.py | 69 ++++++++++++++++ 4 files changed, 228 insertions(+) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index 1d14148c7aeb..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, diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 5a81d2f7793e..90173395ded9 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,6 +4484,33 @@ 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 + ) + if not _already_nudged: + 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)") agent._session_messages = messages diff --git a/tests/agent/test_codex_responses_adapter.py b/tests/agent/test_codex_responses_adapter.py index 4e8a25a661ba..4a14cadf4a3d 100644 --- a/tests/agent/test_codex_responses_adapter.py +++ b/tests/agent/test_codex_responses_adapter.py @@ -437,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/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 6aa381dd2321..93823144a19b 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -2875,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 + ) From 2fc0e3d1aa63f892fce8dc3e31423f4110383e4c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:54:40 -0700 Subject: [PATCH 16/17] fix(codex): guard the continuation nudge against role-alternation violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #63690: when the interim assistant message is too empty to append (no content and no reasoning of any kind), the last message in history is still the prior user/tool turn — appending the user-role nudge there would create a user→user or tool→user sequence that strict providers reject. Only append the nudge when the last message is an assistant turn. Also add IpastorSan to AUTHOR_MAP. --- agent/conversation_loop.py | 13 ++++++++++++- scripts/release.py | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 90173395ded9..6fe9dc68b320 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4506,7 +4506,18 @@ def run_conversation( and _last_msg.get("role") == "user" and _last_msg.get("content") == _CODEX_INCOMPLETE_NUDGE ) - if not _already_nudged: + # 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, diff --git a/scripts/release.py b/scripts/release.py index f246ddd2c86a..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). From 569b912d7d0931c7256e9f5fb326609e9deda377 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:14:05 -0700 Subject: [PATCH 17/17] feat(agent): explain long provider waits on the live status line (#64775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Community reports of GPT 'infinitely thinking' are usually a slow or overloaded provider plus silent retry machinery: the CLI/TUI/Desktop spinner shows a generic 'cogitating...' verb for the whole wait and the gateway heartbeat says only 'Working — N min'. Add AIAgent._emit_wait_notice(): rewrites the live spinner/status line (thinking_callback → CLI prompt_toolkit widget, thinking.delta → TUI + Desktop) and updates the activity tracker (included in the gateway's '⏳ Working — N min' heartbeat). Wired at the four wait points: - non-streaming wait loop: after 30s with no response, the line becomes '⏳ waiting on — Ns with no response yet (provider may be slow or overloaded; auto-reconnect at Ns)' - streaming wait loop: same explanation after 30s with no chunks, including the long-thinking case - TTFB / stale-stream kills: '⚠ no response from provider in Ns — reconnecting...' - Codex continuation retries: '↻ model returned reasoning with no final answer — asking it to continue (n/3)' instead of silence Notices are fail-open (display errors never break the wait loop) and gateway sessions without a display callback still get the improved activity description. --- agent/chat_completion_helpers.py | 53 +++++++- agent/conversation_loop.py | 10 ++ run_agent.py | 24 ++++ tests/run_agent/test_wait_state_visibility.py | 123 ++++++++++++++++++ 4 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 tests/run_agent/test_wait_state_visibility.py diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 2987784ca700..6615dace60a8 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -581,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 @@ -631,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" ) @@ -3137,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 @@ -3182,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/conversation_loop.py b/agent/conversation_loop.py index 6fe9dc68b320..a628093e1f7b 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4524,6 +4524,16 @@ def run_conversation( }) 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/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/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]