From 7987dbd7955708caafaa14aec705de1a5c889249 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:22:17 -0700 Subject: [PATCH] test: restore gateway diagnostic and prompt-branch guards dropped by #120071 Each is the only remaining guard for its issue; all drive production code. - test_allowlist_warning_platform_gate.py::test_allowlist_warning_requires_an_enabled_messaging_platform guards: the "No env user allowlists" startup reminder fires only when a messaging platform is enabled, never for an API-server-only gateway (#115439). Now builds the runner with object.__new__ (the check reads only self.config): 4.1s -> ~0.2s. - test_interim_only_consumer_warning.py::{test_stream_capable_consumer_still_warns, test_interim_only_consumer_skips_duplicate_warning} guards: no false "possible duplicate send" warning per turn for interim-only stream consumers, control case keeps it for stream-capable ones (#105341) - test_kanban_notifier.py::{test_block_loop_technical_kind_uses_neutral_orchestration_wording, test_block_loop_owner_input_keeps_decision_wording} guards: a technical block loop ping must not claim a human decision; needs_input keeps it (#111125) - test_restart_resume_pending.py::TestResumePendingSystemNote::test_empty_message_noninteractive_note_continues_task guards: webhook/API-server resumes tell the model to continue the interrupted task instead of asking "what next" (#57056) --- .../test_allowlist_warning_platform_gate.py | 43 ++++++++++++ .../test_interim_only_consumer_warning.py | 65 +++++++++++++++++++ tests/gateway/test_kanban_notifier.py | 52 +++++++++++++++ tests/gateway/test_restart_resume_pending.py | 13 ++++ 4 files changed, 173 insertions(+) create mode 100644 tests/gateway/test_allowlist_warning_platform_gate.py create mode 100644 tests/gateway/test_interim_only_consumer_warning.py diff --git a/tests/gateway/test_allowlist_warning_platform_gate.py b/tests/gateway/test_allowlist_warning_platform_gate.py new file mode 100644 index 0000000000..dab81ec3b9 --- /dev/null +++ b/tests/gateway/test_allowlist_warning_platform_gate.py @@ -0,0 +1,43 @@ +"""The startup "No env user allowlists configured" reminder fires only when a messaging +platform is enabled — an API-server-only gateway has no sender to gate (#115439).""" + +import logging + +import pytest + +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.run import GatewayRunner + +_WARNING = "No env user allowlists configured" + + +def _runner(platforms, tmp_path): + # The policy check reads only ``self.config``; skip the full runner construction. + runner = object.__new__(GatewayRunner) + runner.config = GatewayConfig(platforms=platforms, sessions_dir=tmp_path / "sessions") + return runner + + +@pytest.mark.parametrize( + ("platforms", "expect_warning"), + [ + ({Platform.API_SERVER: PlatformConfig(enabled=True, extra={"key": "k" * 40})}, False), + ({Platform.API_SERVER: PlatformConfig(enabled=True), Platform.TELEGRAM: PlatformConfig(enabled=True)}, True), + ({Platform.TELEGRAM: PlatformConfig(enabled=False)}, False), + ], + ids=["api_server_only", "api_server_plus_telegram", "telegram_disabled"], +) +def test_allowlist_warning_requires_an_enabled_messaging_platform( + monkeypatch, tmp_path, caplog, platforms, expect_warning, +): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + for var in list(GatewayRunner._BUILTIN_ALLOWED_USERS_VARS) + list(GatewayRunner._BUILTIN_ALLOW_ALL_VARS): + monkeypatch.delenv(var, raising=False) + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + runner = _runner(platforms, tmp_path) + + with caplog.at_level(logging.WARNING, logger="gateway.run"): + refused = runner._start_check_access_policy() + + assert refused is False + assert (_WARNING in caplog.text) is expect_warning diff --git a/tests/gateway/test_interim_only_consumer_warning.py b/tests/gateway/test_interim_only_consumer_warning.py new file mode 100644 index 0000000000..fababef960 --- /dev/null +++ b/tests/gateway/test_interim_only_consumer_warning.py @@ -0,0 +1,65 @@ +"""Regression coverage for #105341 — interim-only stream consumers must not +fire the duplicate-send diagnostic. + +With ``streaming.enabled: false`` and ``display.interim_assistant_messages: +true`` the gateway still builds a ``GatewayStreamConsumer`` (interim +commentary is relayed through it), but the consumer is never fed the final +reply's stream deltas. ``_run_agent_mark_streamed_delivery`` could not tell +\"consumer built only for interim messages\" from \"consumer that streamed but +lost its delivery confirmation\", so it logged a guaranteed-false-positive +``possible duplicate send`` warning once per turn. +""" + +import asyncio +from types import SimpleNamespace + +import pytest + +from gateway.run_turn import GatewayTurnMixin + + +class _InterimOnlyHarness(GatewayTurnMixin): + """Mixin with the delivery-confirmation helper stubbed to False so the + turn always reaches the duplicate-risk diagnostic branch.""" + + def _run_agent_stream_confirmed_final_delivery(self, _sc, _final, *, previewed=False): + return False + + +def _run_mark_streamed_delivery(consumer, caplog): + harness = _InterimOnlyHarness() + turn_ctx = SimpleNamespace( + stream_consumer_holder=[consumer], + source=SimpleNamespace(platform="telegram"), + session_key="sess-105341", + ) + with caplog.at_level("WARNING", logger="gateway.run_turn"): + asyncio.run( + harness._run_agent_mark_streamed_delivery( + {"final_response": "olá"}, turn_ctx + ) + ) + return caplog + + +def _consumer(stream_deltas_enabled): + return SimpleNamespace( + final_content_delivered=False, + delivered_final_matches=None, + message_id=None, + stream_deltas_enabled=stream_deltas_enabled, + ) + + +def test_stream_capable_consumer_still_warns(caplog): + """Control: a consumer that CAN receive final deltas keeps the diagnostic + (the wecom ack-timeout case it was written for).""" + caplog = _run_mark_streamed_delivery(_consumer(True), caplog) + assert any("possible duplicate send" in r.message for r in caplog.records) + + +def test_interim_only_consumer_skips_duplicate_warning(caplog): + """#105341: consumer created only for interim commentary (streaming off, + interim messages on) is never fed the final's deltas — no false positive.""" + caplog = _run_mark_streamed_delivery(_consumer(False), caplog) + assert not any("possible duplicate send" in r.message for r in caplog.records) diff --git a/tests/gateway/test_kanban_notifier.py b/tests/gateway/test_kanban_notifier.py index 950f421294..bb3e912986 100644 --- a/tests/gateway/test_kanban_notifier.py +++ b/tests/gateway/test_kanban_notifier.py @@ -624,6 +624,58 @@ def test_notifier_delivers_block_loop_detected_triage_ping(tmp_path, monkeypatch assert remaining == [] +# --------------------------------------------------------------------------- +# #111125 — a repeated-block circuit breaker establishes that orchestration +# attention is needed, NOT that a human decision exists. The formatter must +# use neutral wording unless the block was typed as a genuine owner-input +# request (`needs_input`, the only kind that carries a concrete question). +# --------------------------------------------------------------------------- + + +class _StubEvent: + def __init__(self, payload): + self.payload = payload + + +class _StubNotif: + head = "H123" + + +def _fmt_block_loop(payload): + from gateway.kanban_watchers_notifier import _EVENT_FORMATTERS + + msg, _, _ = _EVENT_FORMATTERS["block_loop_detected"](_StubEvent(payload), _StubNotif()) + return msg + + +def test_block_loop_technical_kind_uses_neutral_orchestration_wording(): + """A repeated technical block (transient/capability/untyped) routed to + triage is an orchestration handoff with no question for the owner, so the + ping must not claim a human decision (#111125).""" + payload = {"reason": "waiting on upstream", "kind": "transient", "recurrences": 2} + msg = _fmt_block_loop(payload) + assert "for orchestration attention" in msg + assert "human decision" not in msg + # Circuit-breaker visibility is preserved. + assert "TRIAGE" in msg + assert "waiting on upstream" in msg + + +def test_block_loop_owner_input_keeps_decision_wording(): + """A `needs_input` block carries a concrete question for the owner, so the + owner-decision wording is correct and must be retained (#111125).""" + payload = { + "reason": "Which API key should this use?", + "kind": "needs_input", + "recurrences": 2, + "limit": kb.BLOCK_RECURRENCE_LIMIT, + } + msg = _fmt_block_loop(payload) + assert "needs a human decision" in msg + assert "for orchestration attention" not in msg + assert "Which API key should this use?" in msg + + # --------------------------------------------------------------------------- # Handoffs that hand a decision back to the origin must wake it, not only ping # it: `review_requested` (implementation done, waiting for a reviewer) and diff --git a/tests/gateway/test_restart_resume_pending.py b/tests/gateway/test_restart_resume_pending.py index df763532b4..1646395e7e 100644 --- a/tests/gateway/test_restart_resume_pending.py +++ b/tests/gateway/test_restart_resume_pending.py @@ -283,6 +283,19 @@ class TestResumePendingSystemNote: last_resume_marked_at=now, ) + def test_empty_message_noninteractive_note_continues_task(self): + """Non-interactive platforms (webhook, API server): nobody can answer + 'what next?', so the resumed turn must complete the interrupted work + instead of acknowledging (#57056).""" + note = build_resume_recovery_note("restart_timeout", "", interactive=False) + assert note != build_resume_recovery_note("restart_timeout", "", interactive=True) + assert "CONTINUE the interrupted task" in note + assert "ask what they would like to do next" not in note + # Must not tell the model to skip the unfinished work it should finish. + assert "skip any unfinished work" not in note + # But still guards against re-running already-recorded tool calls. + assert "already appear in the history" in note +