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) diff --git a/api/streaming.py b/api/streaming.py index 6eeb3892..4bc49fe0 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,24 @@ 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 the + # downstream _all_result_messages checks (silent-failure detection + # 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): + 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..dcc6e61e 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -280,6 +280,132 @@ 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 — 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 = { + "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. + # 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" + ], "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 + 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 = { + "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",