From 02692cd64ab18b34bfbc15ba84d748ced204e537 Mon Sep 17 00:00:00 2001 From: b3nw Date: Mon, 6 Jul 2026 23:19:46 +0000 Subject: [PATCH 1/3] fix: surface agent's graceful summary on iteration-limit exhaustion (closes #5494) The WebUI already surfaces the model-generated summary when the agent's tool-iteration budget is exhausted. The remaining gap: when the agent hits the limit but its summary call yields no usable assistant content (common with reasoning-only responses), the user sees a bare `tool_limit_reached` error instead of any closure text. `hermes-agent.handle_max_iterations()` always returns a non-empty `final_response` (either the summary or a graceful fallback), and `AIAgent` exposes that string on the result dict. The WebUI, by contrast, only reads `result['messages']`, so an empty summary left no final assistant answer and routed to the error path. Inject `result['final_response']` as a final assistant turn when `max_iterations_reached` fires and no usable answer exists in `messages`. The existing `_mark_latest_assistant_tool_limit_status` flow then attaches the status card and the user sees the closure text instead of the bare error. Hermes-agent parity preserved (it's literally the same string); the bare-error path stays for the case where the agent died entirely (no fallback either). Tests cover: fallback-injection surfaces closure text + status card, no double-injection when a real summary already exists, the existing 'died entirely' apperror path is unchanged, and three unit cases for the helper's skip conditions. Co-authored-by: b3nw --- api/streaming.py | 48 ++++++++++ tests/test_tool_limit_terminal_state.py | 117 ++++++++++++++++++++++++ 2 files changed, 165 insertions(+) diff --git a/api/streaming.py b/api/streaming.py index 6eeb3892..7a6e6209 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1389,6 +1389,40 @@ def _agent_result_tool_limit_reached(result) -> bool: return False +def _maybe_inject_max_iteration_summary_fallback(messages, result) -> list: + """Append the agent's graceful summary text as an assistant turn when one is missing. + + When ``AIAgent`` exhausts its iteration budget, ``agent.handle_max_iterations`` + always returns a non-empty ``final_response`` — either the model-generated + summary or a graceful fallback (e.g. ``"I reached the iteration limit and + couldn't generate a summary."``). Hermes Agent surfaces that string as the + final answer to the user; the WebUI, by contrast, reads only ``messages``, + so an empty summary (common with reasoning-only responses) left the user + with a bare ``tool_limit_reached`` error instead of any closure text. + + When ``_tool_limit_reached`` is true and ``messages`` ends without a final + assistant answer, inject ``result['final_response']`` as a new assistant + turn so ``_mark_latest_assistant_tool_limit_status`` can attach the status + card in the normal flow and the user sees the same closure text as + hermes-agent. Returns the (possibly new) messages list; does nothing when + a usable assistant answer already exists or when ``result`` carries no + graceful fallback text. + """ + if not isinstance(result, dict): + return list(messages or []) + fallback = result.get('final_response') + if not isinstance(fallback, str) or not fallback.strip(): + return list(messages or []) + out = list(messages or []) + if not _session_lacks_final_assistant_answer(out): + return out + # Append a synthetic summary turn. Tag it so downstream consumers can + # distinguish it from model-emitted assistant turns if needed; mirrors the + # synthetic-scaffolding flag convention already used elsewhere (#5334). + out.append({"role": "assistant", "content": fallback, "_max_iteration_summary_fallback": True}) + return out + + def _mark_latest_assistant_tool_limit_status(messages) -> bool: """Annotate the latest usable assistant final answer as limit-stopped.""" for msg in reversed(list(messages or [])): @@ -8239,6 +8273,20 @@ def _run_agent_streaming( _result_messages, enabled=_tool_limit_reached, ) + # #5494 — parity with hermes-agent's handle_max_iterations() return + # value. When the agent produced no usable summary assistant + # message but result['final_response'] carries a graceful fallback + # string, inject it as a final assistant turn so the user sees + # closure text instead of a bare tool_limit_reached error. Apply + # the synthesis to result['messages'] AND _result_messages so + # downstream _all_result_messages checks (silent-failure detection + # at #all_result_messages on the same turn) see the fallback too. + if _tool_limit_reached: + _result_messages = _maybe_inject_max_iteration_summary_fallback( + _result_messages, result + ) + if isinstance(result, dict) and result.get('messages') is not None: + result = {**result, 'messages': _result_messages} if cancel_event.is_set(): _finalize_cancelled_turn(s, ephemeral=False) try: diff --git a/tests/test_tool_limit_terminal_state.py b/tests/test_tool_limit_terminal_state.py index 80e4cd38..1a2becdc 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -280,6 +280,123 @@ def test_streaming_tool_limit_without_final_answer_emits_no_final_apperror(tmp_p ) +def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text(tmp_path, monkeypatch): + """#5494 — when the agent produced no usable summary but returned a graceful + fallback string (handle_max_iterations() always returns one), inject that as a + final assistant turn so the user sees closure text instead of a bare error. + """ + graceful = "I reached the iteration limit and couldn't generate a summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": graceful, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + # The user sees the graceful fallback, not a bare tool_limit_reached error. + assert not [ + ap for ev, ap in events if ev == "apperror" + and ap.get("type") == "tool_limit_reached" + ], "expected no tool_limit_reached apperror when fallback was returned" + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads, "expected done SSE payload" + assert done_payloads[-1]["terminal_state"] == "tool_limit_reached" + + # Fallback text is shown as a final assistant message and is annotated + # with the status card so the UI can render the 'limit reached' chip. + assistant = payload["messages"][-1] + assert assistant["role"] == "assistant" + assert assistant["content"] == graceful + assert assistant["_terminal_state"] == "tool_limit_reached" + assert assistant["_statusCard"]["title"] == "Tool iteration limit reached" + # Synthetic scaffolding turn was still dropped, even after fallback injection. + assert all( + message.get("content") != streaming._MAX_ITERATION_SUMMARY_REQUEST + for message in payload["messages"] + ) + + +def test_streaming_tool_limit_with_fallback_does_not_double_inject_when_assistant_exists(tmp_path, monkeypatch): + """#5494 — when the agent already appended a model-generated summary, the + fallback flow is a no-op; the existing summary is preserved as before. + """ + summary = "I reached the limit; here is the summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": summary, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + {"role": "assistant", "content": summary}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads + assistant_msgs = [ + m for m in payload["messages"] + if m.get("role") == "assistant" and m.get("content") == summary + ] + assert len(assistant_msgs) == 1, "fallback must not duplicate the existing summary" + assert assistant_msgs[0]["_terminal_state"] == "tool_limit_reached" + + +def test_maybe_inject_max_iteration_summary_fallback_unit(): + """Unit-level coverage for the injection helper.""" + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + ] + graceful = "I reached the iteration limit and couldn't generate a summary." + result = {"final_response": graceful} + + injected = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + + assert injected[-1]["role"] == "assistant" + assert injected[-1]["content"] == graceful + assert injected[-1]["_max_iteration_summary_fallback"] is True + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_assistant_present(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "real summary"}, + ] + result = {"final_response": "fallback text"} + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + assert out == messages + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_no_fallback(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + ] + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, {}) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback( + messages, {"final_response": " "} + ) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, None) + assert out == messages + + def test_streaming_tool_limit_terminal_failure_does_not_mark_final_answer(tmp_path, monkeypatch): result = { "status": "partial", From 34fcc3a38799fb15e5902e1621febb555721981c Mon Sep 17 00:00:00 2001 From: b3nw Date: Tue, 7 Jul 2026 00:04:27 +0000 Subject: [PATCH 2/3] fix: drop messages-None writeback guard, reframe for invariant lineage Per Greptile's review on #5717: the previous `if result.get('messages') is not None` guard created an asymmetric condition where the silent-failure classifier could miss the injected fallback when the agent result had `messages: None` (which never actually happens in practice since `finalize_turn` always returns a list, but the asymmetry read as code smell). Drop the guard; the writeback is now unconditional on `isinstance(result, dict)`. Per the maintainer's review on #5717: there may be a `finalize_turn` invariant on the agent branch this WebUI ships against that already appends the fallback as an assistant row, in which case the live bug is gone and this PR becomes a regression guard. Reframe the test docstrings as dual-purpose pins of the user- visible closure-text contract so they read correctly under both interpretations. Co-authored-by: b3nw --- api/streaming.py | 10 +++++++--- tests/test_tool_limit_terminal_state.py | 19 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/api/streaming.py b/api/streaming.py index 7a6e6209..4bc49fe0 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -8278,14 +8278,18 @@ def _run_agent_streaming( # message but result['final_response'] carries a graceful fallback # string, inject it as a final assistant turn so the user sees # closure text instead of a bare tool_limit_reached error. Apply - # the synthesis to result['messages'] AND _result_messages so + # the synthesis to result['messages'] AND _result_messages so the # downstream _all_result_messages checks (silent-failure detection - # at #all_result_messages on the same turn) see the fallback too. + # at api/streaming.py:_assistant_reply_added_after_current_turn) + # see the fallback too. `finalize_turn` in the agent always returns + # messages as a list, but we write back unconditionally so the + # contract is "if we built a result-messages list, the silent-failure + # classifier reads the augmented version." if _tool_limit_reached: _result_messages = _maybe_inject_max_iteration_summary_fallback( _result_messages, result ) - if isinstance(result, dict) and result.get('messages') is not None: + if isinstance(result, dict): result = {**result, 'messages': _result_messages} if cancel_event.is_set(): _finalize_cancelled_turn(s, ephemeral=False) diff --git a/tests/test_tool_limit_terminal_state.py b/tests/test_tool_limit_terminal_state.py index 1a2becdc..dcc6e61e 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -281,9 +281,14 @@ def test_streaming_tool_limit_without_final_answer_emits_no_final_apperror(tmp_p def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text(tmp_path, monkeypatch): - """#5494 — when the agent produced no usable summary but returned a graceful - fallback string (handle_max_iterations() always returns one), inject that as a - final assistant turn so the user sees closure text instead of a bare error. + """#5494 — handle_max_iterations() guarantees a non-empty ``final_response`` + on iteration-limit exhaustion. This test pins the WebUI contract that, + when ``messages`` ends without a final assistant turn and ``final_response`` + is set, the user sees that closure text instead of a bare + ``tool_limit_reached`` error. Serves both as a live-bug fix pin and as a + regression guard for the agent's "delivered final_response ⇒ assistant + row" invariant: if a future agent regression drops that invariant, this + test still passes because the WebUI honors the contract locally. """ graceful = "I reached the iteration limit and couldn't generate a summary." result = { @@ -300,6 +305,9 @@ def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) # The user sees the graceful fallback, not a bare tool_limit_reached error. + # Either (a) we synthesized the fallback here, or (b) the agent guarantee + # already added an assistant row and `_mark_latest_assistant_tool_limit_status` + # attached the status card. Both routes satisfy the contract. assert not [ ap for ev, ap in events if ev == "apperror" and ap.get("type") == "tool_limit_reached" @@ -323,8 +331,9 @@ def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text def test_streaming_tool_limit_with_fallback_does_not_double_inject_when_assistant_exists(tmp_path, monkeypatch): - """#5494 — when the agent already appended a model-generated summary, the - fallback flow is a no-op; the existing summary is preserved as before. + """#5494 — when the agent already appended a model-generated summary + AND ``final_response`` carries the same text, the WebUI must not duplicate + the assistant turn. Pins the no-op contract on the synthesis path. """ summary = "I reached the limit; here is the summary." result = { From 6c7746aef65e52702764f89132659d836aef2e82 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 7 Jul 2026 01:44:48 +0000 Subject: [PATCH 3/3] docs(changelog): iteration-limit graceful summary (#5717, #5494) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd2464dc..576f6f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **The agent's closing summary now shows when it hits the iteration limit.** When a turn exhausted its tool/iteration budget, the agent produces a graceful closing message (a summary of what it got done, or a fallback like "I reached the iteration limit…"), but the WebUI showed a bare `tool_limit_reached` error card instead — most visible with reasoning-only models (extended-thinking Claude, DeepSeek, Codex Responses) whose summary arrives empty. The closing text is now surfaced as a proper assistant message. Purely additive and guarded — it never fires on a turn that already produced a final answer, never double-injects, and degrades to the old behavior when no fallback text is available. Thanks @claw-io (co-authored by @b3nw). (#5717, #5494) + - **The workspace file tree no longer jumps to the top when you expand a folder.** Every folder expand/collapse, breadcrumb navigation, refresh, and hidden-files toggle re-rendered the tree by wiping its container, which collapsed its height and made the browser reset the scroll position to the top — teleporting you away from where you were in a long tree. The scroll position is now captured before the re-render and restored after. Reported by @claw-io. (#5664, #5657) - **Self-update recovers when a `.git/index.lock` is blocking it.** If "Update Now" failed because a stale `.git/index.lock` was present (a previous git process left it behind), the WebUI gave no in-UI way out — you were stuck on an un-updatable install. The update banner now detects that specific failure, shows the exact `rm -f …/.git/index.lock` command to run on the host, and offers a "Retry update" button that re-runs the normal update once the lock is gone. The server never deletes anything under `.git` itself (fail-closed — git's lock can't be safely auto-cleared without racing), so this is purely a detect-and-guide recovery. Thanks @b3nw. (#5688, #5687)