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)
This commit is contained in:
43
tests/gateway/test_allowlist_warning_platform_gate.py
Normal file
43
tests/gateway/test_allowlist_warning_platform_gate.py
Normal file
@@ -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
|
||||
65
tests/gateway/test_interim_only_consumer_warning.py
Normal file
65
tests/gateway/test_interim_only_consumer_warning.py
Normal file
@@ -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)
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user