diff --git a/strix/interface/tui/renderers/proxy_renderer.py b/strix/interface/tui/renderers/proxy_renderer.py index 359b494d6..410372a3b 100644 --- a/strix/interface/tui/renderers/proxy_renderer.py +++ b/strix/interface/tui/renderers/proxy_renderer.py @@ -35,6 +35,17 @@ def _status_style(code: int | None) -> str: return "dim" +def _has_result(status: str) -> bool: + """True once the tool has returned, whether it succeeded or not. + + ``live_view._tool_status_from_result`` maps a ``{"success": false}`` + payload to ``"failed"``, and every proxy-tool error payload sets that + flag, so gating result rendering on ``"completed"`` alone hides exactly + the payloads that carry an ``error``. + """ + return status in {"completed", "failed", "error"} + + @register_tool_renderer class ListRequestsRenderer(BaseToolRenderer): tool_name: ClassVar[str] = "list_requests" @@ -68,7 +79,7 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912, PLR0915 if meta_parts: text.append(f" ({', '.join(meta_parts)})", style="dim") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if "error" in result: text.append(f" error: {_sanitize(str(result['error']), 150)}", style="#ef4444") else: @@ -139,7 +150,7 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912, PLR0915 if search_pattern: text.append(f" /{_truncate(search_pattern, 100)}/", style="dim italic") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if "error" in result: text.append(f" error: {_sanitize(str(result['error']), 150)}", style="#ef4444") elif "hits" in result: @@ -261,9 +272,18 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912, PLR0915 elif modifications and isinstance(modifications, str): text.append(f"\n {_truncate(modifications, 200)}", style="dim italic") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if not result.get("success", True) and result.get("error"): text.append(f"\n error: {_sanitize(str(result['error']), 150)}", style="#ef4444") + elif not result.get("success", True): + # A replay that never reached DONE carries no `error`; report the + # replay status rather than rendering an empty response as if the + # request had come back. + replay_status = _sanitize(str(result.get("status") or "unknown"), 40) + text.append( + f"\n replay did not complete: {replay_status}", + style="#ef4444", + ) else: elapsed_ms = result.get("elapsed_ms") response = result.get("response") or {} @@ -327,7 +347,7 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912, PLR0915 if meta_parts: text.append(f" ({', '.join(meta_parts)})", style="dim") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if "error" in result: text.append(f" error: {_sanitize(str(result['error']), 150)}", style="#ef4444") else: @@ -401,7 +421,7 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912 if entry_id: text.append(f" #{_truncate(str(entry_id), 20)}", style="dim") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if "error" in result: text.append(f" error: {_sanitize(str(result['error']), 150)}", style="#ef4444") elif "entry" in result: @@ -492,7 +512,7 @@ def render(cls, tool_data: dict[str, Any]) -> Static: # noqa: PLR0912, PLR0915 if len(denylist) > 4: text.append(f" +{len(denylist) - 4}", style="dim italic") - if status == "completed" and isinstance(result, dict): + if _has_result(status) and isinstance(result, dict): if "error" in result: text.append(f" error: {_sanitize(str(result['error']), 150)}", style="#ef4444") elif "scopes" in result: diff --git a/tests/test_proxy_renderer.py b/tests/test_proxy_renderer.py index 341a1f40f..f0a351652 100644 --- a/tests/test_proxy_renderer.py +++ b/tests/test_proxy_renderer.py @@ -2,9 +2,18 @@ from __future__ import annotations +import pytest from rich.text import Text -from strix.interface.tui.renderers.proxy_renderer import ViewRequestRenderer +from strix.interface.tui.live_view import _tool_status_from_result +from strix.interface.tui.renderers.proxy_renderer import ( + ListRequestsRenderer, + ListSitemapRenderer, + RepeatRequestRenderer, + ScopeRulesRenderer, + ViewRequestRenderer, + ViewSitemapEntryRenderer, +) def _plain(static: object) -> str: @@ -44,3 +53,80 @@ def test_more_content_hint_shown_when_has_more_flag_set() -> None: content = "\n".join(f"line{i}" for i in range(3)) assert _MARKER in _render(content, has_more=True) + + +# Every proxy-tool error payload sets `success: false`, which +# `live_view._tool_status_from_result` maps to status "failed" -- so the +# renderers must show results for that status too, not just "completed". +@pytest.mark.parametrize( + ("renderer", "args"), + [ + (ListRequestsRenderer, {}), + (ViewRequestRenderer, {"request_id": "req-1"}), + (RepeatRequestRenderer, {"request_id": "req-1"}), + (ListSitemapRenderer, {}), + (ViewSitemapEntryRenderer, {"entry_id": "e-1"}), + (ScopeRulesRenderer, {"action": "list"}), + ], +) +def test_error_is_rendered_for_failed_status(renderer: object, args: dict[str, object]) -> None: + tool_data = { + "args": args, + "result": {"success": False, "error": "proxy tool failed: boom"}, + "status": _tool_status_from_result({"success": False, "error": "x"}), + } + + assert "proxy tool failed: boom" in _plain(renderer.render(tool_data)) # type: ignore[attr-defined] + + +def test_tool_status_from_result_maps_failure_to_failed() -> None: + # Pins the mapping the renderers' gate has to agree with. + assert _tool_status_from_result({"success": False, "error": "x"}) == "failed" + assert _tool_status_from_result({"success": True}) == "completed" + + +def test_incomplete_replay_says_so_instead_of_showing_an_empty_response() -> None: + # _format_replay_tool_result sets success=False with no `error` key when the + # replay never reached DONE; it must not render as a returned response. + tool_data = { + "args": {"request_id": "req-1"}, + "result": { + "success": False, + "status": "TIMEDOUT", + "session_id": "s-1", + "elapsed_ms": 30000, + "response": None, + }, + "status": "failed", + } + text = _plain(RepeatRequestRenderer.render(tool_data)) + + assert "replay did not complete: TIMEDOUT" in text + assert "<<" not in text, "rendered a response line for a replay that never returned" + + +def test_successful_replay_still_renders_its_response() -> None: + tool_data = { + "args": {"request_id": "req-1"}, + "result": { + "success": True, + "status": "DONE", + "elapsed_ms": 12, + "response": {"status_code": 200, "body": "ok"}, + }, + "status": "completed", + } + text = _plain(RepeatRequestRenderer.render(tool_data)) + + assert "200" in text + assert "replay did not complete" not in text + + +def test_successful_result_still_renders() -> None: + tool_data = { + "args": {}, + "result": {"success": True, "entries": [], "page_info": {"has_next_page": False}}, + "status": "completed", + } + + assert "[0 found]" in _plain(ListRequestsRenderer.render(tool_data))