From 4eb69cbce5ad1a07419e00ad5a16dba7a1e055b1 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:47:51 -0700 Subject: [PATCH] fix(bot-mode): Bot Chat identity + one-shot DM transport apply the silence rule; trim tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #110786 commit: - `_is_bot_mode_session` mirrors the system-prompt gate (`agent._session_title_hint` first, then the live DB title) instead of reading `pending_title`/`title` off the session dict: `pending_title` is cleared after turn 1 and the record never carries `title`, so the contributor's gate matched only the very first Bot Chat turn. - `tools/bot_mode_dm.py::_run_local_turn` (the `hermes -p X chat -c "Bot Chat" -Q` transport behind `message_agent` when no live owner holds the target) re-emits "" for a successful bare marker — the third delivery path of the same class. - Tests trimmed to one invariant per surface (live completion, relay RPC, one-shot transport), each proven red on origin/main sources. - Bot Mode docs gain a "Staying silent" line pointing at the shared token list. --- tests/tools/test_bot_mode_dm.py | 16 ++++ .../test_bot_mode_silence_delivery.py | 76 ++++++++----------- tests/tui_gateway/test_bot_relay_methods.py | 37 +++------ tools/bot_mode_dm.py | 11 ++- tui_gateway/prompt_turn.py | 10 ++- website/docs/user-guide/bot-mode.md | 2 + 6 files changed, 76 insertions(+), 76 deletions(-) diff --git a/tests/tools/test_bot_mode_dm.py b/tests/tools/test_bot_mode_dm.py index 8088ee24dd..e917c65d3d 100644 --- a/tests/tools/test_bot_mode_dm.py +++ b/tests/tools/test_bot_mode_dm.py @@ -593,6 +593,22 @@ def test_delivery_runner_surfaces_live_owner_refusal(tmp_path, capsys): assert "NOT delivered" in payload["error"] +def test_local_turn_reemits_empty_stdout_for_a_bare_silence_marker(tmp_path, capsys): + """#110782: the one-shot ``hermes chat -c "Bot Chat"`` transport applies the gateway's + silence rule — a successful bare marker reaches the sender as "", prose stays verbatim.""" + dm_file = tmp_path / "message.txt" + dm_file.write_text("thanks, bye", encoding="utf-8") + child = tmp_path / "quiet.py" + child.write_text("import sys\nprint(sys.argv[1])\n", encoding="utf-8") + + assert bot_mode_dm._run_local_turn([sys.executable, str(child), "NO_REPLY"], str(dm_file)) == 0 + assert capsys.readouterr().out == "" + + prose = "The NO_REPLY marker means do not answer." + assert bot_mode_dm._run_local_turn([sys.executable, str(child), prose], str(dm_file)) == 0 + assert capsys.readouterr().out.strip() == prose + + def test_query_file_delivery_closes_stdin_for_initial_attempt_and_retry( tmp_path, monkeypatch ): diff --git a/tests/tui_gateway/test_bot_mode_silence_delivery.py b/tests/tui_gateway/test_bot_mode_silence_delivery.py index bb481c4fb1..d092b95c3d 100644 --- a/tests/tui_gateway/test_bot_mode_silence_delivery.py +++ b/tests/tui_gateway/test_bot_mode_silence_delivery.py @@ -1,56 +1,44 @@ -"""Regression coverage for Bot Mode's shared completed-reply delivery boundary.""" +"""Bot Mode never renders or relays a bare intentional-silence marker (#110782). + +Silence is a delivery decision shared with the gateway (``gateway/response_filters``): +the assistant row stays persisted, only the outbound text is emptied; failed turns and +prose that merely mentions a marker are delivered unchanged. +""" import contextlib from types import SimpleNamespace -import pytest - import tui_gateway.server as srv -from tui_gateway.prompt_turn import _bot_mode_delivery_text, _is_bot_mode_session -@pytest.mark.parametrize("response", [ - "NO_REPLY", " [silent] ", "silent", "no reply", "*NO_REPLY*", -]) -def test_bot_mode_delivery_hides_successful_canonical_silence_markers(response): - assert _bot_mode_delivery_text(response, successful=True) == "" - - -@pytest.mark.parametrize("response", [ - "The NO_REPLY marker means do not answer.", - "[SILENT] is mentioned here, but this is a real answer.", -]) -def test_bot_mode_delivery_keeps_substantive_marker_mentions(response): - assert _bot_mode_delivery_text(response, successful=True) == response - - -def test_bot_mode_delivery_keeps_failed_marker_response_visible(): - assert _bot_mode_delivery_text("NO_REPLY", successful=False) == "NO_REPLY" - - -def test_only_canonical_bot_chat_sessions_use_the_live_delivery_boundary(): - assert _is_bot_mode_session({"pending_title": "Bot Chat"}) - assert _is_bot_mode_session({"title": "Bot Chat"}) - assert not _is_bot_mode_session({"pending_title": "Scratch"}) - - -def test_live_bot_chat_completion_suppresses_markers_but_failed_turns_fail_open(monkeypatch): - """The prompt.submit completion path applies the shared delivery boundary.""" - monkeypatch.setattr(srv, "_get_usage", lambda _agent: {}) - monkeypatch.setattr(srv, "render_message", lambda _text, _cols: None) - monkeypatch.setattr(srv, "_clear_inflight_turn", lambda _session: None) - - session = {"pending_title": "Bot Chat", "history_lock": contextlib.nullcontext()} - turn = SimpleNamespace( - result={"final_response": "NO_REPLY"}, agent=object(), terminal_callback=None, +def _turn(result): + return SimpleNamespace( + result=result, agent=SimpleNamespace(_session_title_hint="Bot Chat"), terminal_callback=None, receipt_committed=True, receipt_attempted=False, marker_key="", error_retained=False, error_detail="", prompt_text="ping", ) - payload, _, status = srv._complete_turn_payload(session, turn, None, 80) - assert status == "complete" - assert payload["text"] == "" - turn.result = {"final_response": "NO_REPLY", "error": "provider failed", "failed": True} - payload, _, status = srv._complete_turn_payload(session, turn, None, 80) - assert status == "error" + +def test_live_bot_chat_completion_empties_marker_only_for_successful_turns(monkeypatch): + monkeypatch.setattr(srv, "_get_usage", lambda _agent: {}) + monkeypatch.setattr(srv, "render_message", lambda _text, _cols: None) + monkeypatch.setattr(srv, "_clear_inflight_turn", lambda _session: None) + session = {"pending_title": None, "session_key": "k", "history_lock": contextlib.nullcontext(), + "agent": SimpleNamespace(_session_title_hint="Bot Chat")} + + payload, _, status = srv._complete_turn_payload(session, _turn({"final_response": " *NO_REPLY* "}), None, 80) + assert (status, payload["text"]) == ("complete", "") + + prose = "[SILENT] is mentioned here, but this is a real answer." + payload, _, _ = srv._complete_turn_payload(session, _turn({"final_response": prose}), None, 80) + assert payload["text"] == prose + + failed = {"final_response": "NO_REPLY", "error": "provider failed", "failed": True} + payload, _, status = srv._complete_turn_payload(session, _turn(failed), None, 80) + assert (status, payload["text"]) == ("error", "NO_REPLY") + + # A plain (non-Bot-Chat) desktop session keeps the marker: the gate is the canonical title. + session["agent"] = SimpleNamespace(_session_title_hint="Scratch") + monkeypatch.setattr(srv, "_session_live_title", lambda _s, _k: "Scratch") + payload, _, _ = srv._complete_turn_payload(session, _turn({"final_response": "NO_REPLY"}), None, 80) assert payload["text"] == "NO_REPLY" diff --git a/tests/tui_gateway/test_bot_relay_methods.py b/tests/tui_gateway/test_bot_relay_methods.py index e5270d7cb2..3d96bff401 100644 --- a/tests/tui_gateway/test_bot_relay_methods.py +++ b/tests/tui_gateway/test_bot_relay_methods.py @@ -108,38 +108,19 @@ def test_deliver_requires_params(home): assert "error" in err -@pytest.mark.parametrize("reply", [ - "NO_REPLY", " [silent] ", "silent", "no reply", "*NO_REPLY*", -]) -def test_deliver_suppresses_successful_silence_markers(home, monkeypatch, reply): - """A successful subprocess relay never returns a bare silence marker to Bot Mode.""" +def test_deliver_relays_empty_reply_for_a_bare_silence_marker(home, monkeypatch): + """#110782: the subprocess transport applies the gateway's silence rule — a bare marker + relays as "", prose that merely mentions one is relayed verbatim.""" class _Proc: - returncode = 0 - stdout = reply - stderr = "" + returncode, stderr = 0, "" + stdout = " *NO_REPLY* " - monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: _Proc()) + monkeypatch.setattr("subprocess.run", lambda *_a, **_k: _Proc()) + assert _result(srv._methods["bot_relay.deliver"](1, {"profile": "ops", "message": "ping"}))["reply"] == "" + _Proc.stdout = "The NO_REPLY marker means do not answer." out = _result(srv._methods["bot_relay.deliver"](1, {"profile": "ops", "message": "ping"})) - - assert out["reply"] == "" - - -@pytest.mark.parametrize("reply", [ - "The NO_REPLY marker means do not answer.", - "[SILENT] is mentioned here, but this is a real answer.", -]) -def test_deliver_keeps_substantive_marker_mentions(home, monkeypatch, reply): - class _Proc: - returncode = 0 - stdout = reply - stderr = "" - - monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: _Proc()) - - out = _result(srv._methods["bot_relay.deliver"](1, {"profile": "ops", "message": "ping"})) - - assert out["reply"] == reply + assert out["reply"] == _Proc.stdout.strip() def test_deliver_lands_in_live_bot_chat_instead_of_subprocess(home, monkeypatch): diff --git a/tools/bot_mode_dm.py b/tools/bot_mode_dm.py index a765705663..4755cc399e 100644 --- a/tools/bot_mode_dm.py +++ b/tools/bot_mode_dm.py @@ -403,8 +403,15 @@ def _run_local_turn(argv: list[str], dm_file: str, *, env: Optional[dict[str, st })) return 1 # Re-emit the transport's streams: stdout is the reply text the - # completion notification carries back to the sending agent. - for stream, text in ((sys.stdout, proc.stdout), (sys.stderr, proc.stderr)): + # completion notification carries back to the sending agent. A successful bare + # silence marker is a delivery decision (same rule as the gateway and the live + # Bot Chat completion): the turn stays in the target's transcript, the sender + # never sees the marker as prose. + from gateway.response_filters import is_intentional_silence_response + reply = proc.stdout or "" + if proc.returncode == 0 and is_intentional_silence_response(reply): + reply = "" + for stream, text in ((sys.stdout, reply), (sys.stderr, proc.stderr)): if text: stream.write(text) stream.flush() diff --git a/tui_gateway/prompt_turn.py b/tui_gateway/prompt_turn.py index 6eb940abeb..4a3cfcf392 100644 --- a/tui_gateway/prompt_turn.py +++ b/tui_gateway/prompt_turn.py @@ -29,9 +29,15 @@ def _bot_mode_delivery_text(response: Any, *, successful: bool) -> Any: def _is_bot_mode_session(session: dict) -> bool: - """Whether this completion belongs to the canonical Bot Chat surface.""" + """Whether this completion belongs to the canonical Bot Chat surface. + + Same resolution as the system-prompt gate: the agent's title hint first (the DB + title lands after turn 1 and ``pending_title`` is cleared once it does), then the + live title from the session store. + """ from tools.bot_mode_probe import BOT_CHAT_TITLE - return any(session.get(field) == BOT_CHAT_TITLE for field in ("pending_title", "title")) + hint = str(getattr(session.get("agent"), "_session_title_hint", "") or "").strip() + return (hint or _session_live_title(session, _session_lookup_key(session))) == BOT_CHAT_TITLE def _hook_failure(what: str, exc: BaseException) -> None: diff --git a/website/docs/user-guide/bot-mode.md b/website/docs/user-guide/bot-mode.md index 4c13fec82e..8d1c1cb2b6 100644 --- a/website/docs/user-guide/bot-mode.md +++ b/website/docs/user-guide/bot-mode.md @@ -134,6 +134,8 @@ Bots message each other with attribution, and you can hand work off from any cha Local messages also reach a Bot Chat that stays open in Desktop or the TUI. The receiving backend keeps ownership: it reads durable ingress on its existing notification poller, admits immediately when idle, or waits until the running turn and already queued human prompts finish. A `queued` acknowledgement confirms durable admission, **not** a completed reply. The target profile retains the delivery ID and receipt under `runtime/bot_live_delivery/`; `settled` confirms completion. A crashed or cancelled imported turn is not automatically replayed, and pending work pinned to a departed owner remains inspectable rather than being silently rerun. Do not resend a delivery whose outcome is unknown. Older backends without live-delivery capability retain the existing ownership refusal; restart that backend after upgrading. +- **Staying silent** — a Bot that has nothing to add may end a turn with one of the [intentional silence tokens](./messaging/index.md#intentional-silence-tokens) (`[SILENT]`, `NO_REPLY`, …). The Bot Chat keeps that turn in its transcript but renders nothing, and a teammate that messaged it gets an empty reply instead of the token. Failed turns and prose that merely mentions a token are shown as-is. + The backend teaches each Bot's canonical Bot Chat session the messaging protocol automatically at prompt-build time — including when a teammate opens it headlessly from the CLI. Only the canonical Bot Chat gets the protocol section; your regular sessions and your SOUL.md stay untouched. This is controlled by `agent.bot_mode_protocol` in `config.yaml` (default: on): ```yaml