diff --git a/tests/tools/test_tour_tool.py b/tests/tools/test_tour_tool.py new file mode 100644 index 0000000000..f6147f6274 --- /dev/null +++ b/tests/tools/test_tour_tool.py @@ -0,0 +1,80 @@ +"""Tests for the GUI-surface ``tour`` tool.""" + +import json + +from tools import tour_tool as tt +from tools.registry import registry + + +def _run(**kwargs): + kwargs.setdefault("callback", lambda _payload: json.dumps({"success": True})) + return json.loads(tt.tour_tool(**kwargs)) + + +def test_lives_in_the_gui_surface_toolset(monkeypatch): + """Scoped by toolset, not by the backend's env — see AGENTS.md.""" + monkeypatch.delenv("HERMES_DESKTOP", raising=False) + entry = registry.get_entry("tour") + + assert entry is not None + assert entry.toolset == "desktop_ui" + assert entry.check_fn is None + + +def test_requires_callback(): + """Outside the desktop GUI there is no bridge — a clear error, no crash.""" + assert "desktop" in json.loads(tt.tour_tool(action="targets", callback=None))["error"] + + +def test_rejects_unknown_action_and_surface(): + assert "action must be one of" in _run(action="dance")["error"] + assert "surface must be one of" in _run(action="targets", surface="hologram")["error"] + assert "side must be one of" in _run(action="show", selector="#a", side="diagonal")["error"] + + +def test_show_needs_something_to_point_at_or_say(): + assert "show needs" in _run(action="show")["error"] + assert "error" not in _run(action="show", text="just narration") + + +def test_start_validates_its_steps(): + assert "non-empty steps" in _run(action="start")["error"] + assert "non-empty steps" in _run(action="start", steps=[])["error"] + assert "steps[1] must be an object" in _run(action="start", steps=[{"selector": "#a"}, "nope"])["error"] + assert "steps[1] needs" in _run(action="start", steps=[{"selector": "#a"}, {}])["error"] + + +def test_payload_omits_unset_fields_and_defaults_the_surface(): + seen = {} + + def cb(payload): + seen.update(payload) + return json.dumps({"success": True}) + + tt.tour_tool(action="show", selector="#composer", title="Composer", callback=cb) + assert seen == { + "action": "show", + "surface": "app", + "selector": "#composer", + "title": "Composer", + } + + +def test_unanswered_bridge_is_reported_rather_than_faked_as_success(): + assert "error" in _run(action="targets", callback=lambda _p: "") + + +def test_passes_renderer_json_through(): + payload = {"success": True, "matched": True, "step": 2} + assert _run(action="next", callback=lambda _p: json.dumps(payload)) == payload + + +def test_wraps_non_json_text(): + assert _run(action="stop", callback=lambda _p: "stopped") == {"text": "stopped"} + + +def test_callback_failure_is_reported(): + def _boom(_payload): + raise RuntimeError("renderer went away") + + assert "renderer went away" in _run(action="stop", callback=_boom)["error"] diff --git a/tests/tui_gateway/test_tour_bridge_fail_fast.py b/tests/tui_gateway/test_tour_bridge_fail_fast.py new file mode 100644 index 0000000000..0699895d55 --- /dev/null +++ b/tests/tui_gateway/test_tour_bridge_fail_fast.py @@ -0,0 +1,101 @@ +"""A desktop client that cannot answer ``tour.request`` must not cost a full +bridge timeout per call. + +The renderer's handler ships in the desktop bundle; the tool is offered by the +backend. An app build older than the tour tool has no branch for the event, so +nothing ever calls ``tour.respond`` and the agent blocks for the whole deadline +— once per action the model tries. See tui_gateway.server._tour_request. +""" + +import json + +import pytest + +import tui_gateway.server as server + + +@pytest.fixture +def session(monkeypatch): + record = {} + monkeypatch.setitem(server._sessions, "s1", record) + return record + + +@pytest.fixture +def bridge(monkeypatch): + """Record every _block call and serve canned answers.""" + calls = [] + + def fake_block(event, sid, payload, timeout=None, **_kw): + answer = fake_block.answers.pop(0) if fake_block.answers else "" + calls.append({"event": event, "sid": sid, "payload": payload, "timeout": timeout}) + return answer + + fake_block.answers = [] + fake_block.calls = calls + monkeypatch.setattr(server, "_block", fake_block) + return fake_block + + +def test_first_action_is_probed_on_a_short_deadline(session, bridge): + bridge.answers = [json.dumps({"success": True})] + server._tour_request("s1", {"action": "targets"}) + + assert bridge.calls[0]["event"] == "tour.request" + assert bridge.calls[0]["timeout"] == server._TOUR_PROBE_TIMEOUT_S + assert server._TOUR_PROBE_TIMEOUT_S < server._TOUR_TIMEOUT_S + + +def test_a_client_that_answers_gets_the_full_deadline_back(session, bridge): + """The generous deadline exists for a preview tour injecting into a live + page; only an unproven client is held to the probe.""" + bridge.answers = [json.dumps({"success": True}), json.dumps({"success": True})] + server._tour_request("s1", {"action": "targets"}) + server._tour_request("s1", {"action": "show", "selector": "#composer"}) + + assert bridge.calls[1]["timeout"] == server._TOUR_TIMEOUT_S + + +def test_unanswered_probe_explains_the_real_problem(session, bridge): + result = json.loads(server._tour_request("s1", {"action": "targets"})) + + assert result["success"] is False + assert "desktop" in result["error"].lower() + assert "update" in result["error"].lower() + + +def test_later_actions_short_circuit_instead_of_stalling_again(session, bridge): + """The regression: a "give me a tour" turn stacks one timeout per action.""" + server._tour_request("s1", {"action": "targets"}) + for action in ("show", "start", "next", "stop"): + assert json.loads(server._tour_request("s1", {"action": action}))["success"] is False + + assert len(bridge.calls) == 1 + + +def test_a_proven_client_is_not_condemned_by_one_slow_action(session, bridge): + """A transient miss on a live renderer must not disable the tour.""" + bridge.answers = [json.dumps({"success": True})] + server._tour_request("s1", {"action": "targets"}) + server._tour_request("s1", {"action": "show", "selector": "#composer"}) + + bridge.answers = [json.dumps({"success": True})] + assert json.loads(server._tour_request("s1", {"action": "next"}))["success"] is True + assert len(bridge.calls) == 3 + + +def test_a_new_session_reprobes(bridge, monkeypatch): + """The verdict lives on the session record, so it dies with the session.""" + monkeypatch.setitem(server._sessions, "dead", {}) + monkeypatch.setitem(server._sessions, "fresh", {}) + + server._tour_request("dead", {"action": "targets"}) + bridge.answers = [json.dumps({"success": True})] + + assert json.loads(server._tour_request("fresh", {"action": "targets"}))["success"] is True + + +def test_a_session_with_no_record_still_bridges(bridge): + """Detached callers have no session dict; they keep the plain bridge.""" + bridge.answers = [json.dumps({"success": True})] + assert json.loads(server._tour_request("gone", {"action": "targets"}))["success"] is True diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 9493814bd0..11d2cb1791 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3618,6 +3618,74 @@ def _clarify_block(sid: str, q, c, multi_select=False, questions=None) -> str: ) +# A tour action is a DOM operation the renderer performs and answers straight +# back, so a client that implements the bridge replies in milliseconds. The +# generous deadline exists for one case only: a preview tour's first action +# injects the engine into a live page. +_TOUR_TIMEOUT_S = 45 +# Until a session's client has proven it answers at all, hold it to a deadline +# a working renderer cannot miss. See _tour_request. +_TOUR_PROBE_TIMEOUT_S = 10 + +_TOUR_BRIDGE_UNAVAILABLE = json.dumps( + { + "success": False, + "error": ( + "No Hermes Desktop window answered the tour request. The tour is " + "driven by the desktop app's renderer, which updates separately " + "from this backend, so an app build older than the tour tool has " + "nothing listening. Update the Hermes Desktop app and start a new " + "session. Do not retry tour in this session." + ), + } +) + + +def _tour_request(sid: str, payload: dict) -> str: + """Bridge the tour tool callback onto _block, without paying for a client + that cannot answer it. + + The renderer's ``tour.request`` handler ships in the desktop bundle, but + the tool is offered by this backend — and the two update on different + clocks. Against an app older than the tool the event lands in a renderer + with no branch for it, nobody ever calls ``tour.respond``, and the agent + blocks for the full deadline. The model then does what the schema tells it + to and tries the next action, so a single "give me a tour" turn stacks + those waits (the timeouts reported against #89620). + + A session's first action therefore gets the probe deadline, and an + unanswered probe marks the bridge unavailable for that session: every later + call returns immediately, telling the user what to actually fix instead of + stalling again. Once a client has answered, real actions get the full + deadline back and a single slow one no longer condemns it. The verdict + lives on the session record, so it dies with the session and a new one + re-probes. + """ + # A detached caller has no session record; the throwaway keeps it on the + # plain bridge, unprobed. + session = _sessions.get(sid) + if session is None: + session = {} + state = session.get("tour_bridge") + + if state == "unanswered": + return _TOUR_BRIDGE_UNAVAILABLE + + answer = _block( + "tour.request", + sid, + dict(payload), + timeout=_TOUR_TIMEOUT_S if state == "answered" else _TOUR_PROBE_TIMEOUT_S, + ) + + if answer: + session["tour_bridge"] = "answered" + elif state != "answered": + session["tour_bridge"] = "unanswered" + + return answer or _TOUR_BRIDGE_UNAVAILABLE + + def _clear_pending(sid: str | None = None) -> None: """Release pending prompts with an empty answer. @@ -6325,14 +6393,8 @@ def _agent_cbs(sid: str) -> dict: # tour tool (desktop GUI): the renderer drives driver.js — highlighting # elements in the app's own DOM or injecting the engine into the # preview pane's webview — and answers tour.respond with the outcome - # (did the selector match, which step is active). Generous timeout: a - # preview tour's first action loads the engine into a live page. - "tour_callback": lambda payload: _block( - "tour.request", - sid, - dict(payload), - timeout=45, - ), + # (did the selector match, which step is active). + "tour_callback": lambda payload: _tour_request(sid, payload), } # Interim assistant commentary (text alongside tool calls, or the attempted