Merge remote-tracking branch 'origin/main' into pr48275-rebase
# Conflicts: # cron/scheduler.py
This commit is contained in:
commit
a58287afcb
162 changed files with 8521 additions and 634 deletions
|
|
@ -38,6 +38,20 @@ def _jwt_with_claims(claims: dict) -> str:
|
|||
return f"{header}.{payload}.sig"
|
||||
|
||||
|
||||
class _FakeAnthropicStream:
|
||||
def __init__(self, final_message):
|
||||
self._final_message = final_message
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def get_final_message(self):
|
||||
return self._final_message
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_env(monkeypatch):
|
||||
"""Strip provider env vars so each test starts clean."""
|
||||
|
|
@ -990,6 +1004,37 @@ class TestVisionClientFallback:
|
|||
assert client.__class__.__name__ == "AnthropicAuxiliaryClient"
|
||||
assert model == "claude-haiku-4-5-20251001"
|
||||
|
||||
def test_anthropic_auxiliary_client_aggregates_stream_response(self):
|
||||
from agent.auxiliary_client import AnthropicAuxiliaryClient
|
||||
|
||||
final_message = SimpleNamespace(
|
||||
content=[SimpleNamespace(type="text", text="streamed aux response")],
|
||||
stop_reason="end_turn",
|
||||
usage=SimpleNamespace(input_tokens=3, output_tokens=4),
|
||||
)
|
||||
messages_api = SimpleNamespace(
|
||||
stream=MagicMock(return_value=_FakeAnthropicStream(final_message)),
|
||||
create=MagicMock(return_value="raw event-stream text"),
|
||||
)
|
||||
real_client = SimpleNamespace(messages=messages_api)
|
||||
client = AnthropicAuxiliaryClient(
|
||||
real_client,
|
||||
"claude-sonnet-4-20250514",
|
||||
"sk-test",
|
||||
"https://sse-only.example/v1",
|
||||
)
|
||||
|
||||
response = client.chat.completions.create(
|
||||
messages=[{"role": "user", "content": "summarize"}],
|
||||
max_tokens=16,
|
||||
)
|
||||
|
||||
messages_api.stream.assert_called_once()
|
||||
messages_api.create.assert_not_called()
|
||||
assert response.choices[0].message.content == "streamed aux response"
|
||||
assert response.usage.prompt_tokens == 3
|
||||
assert response.usage.completion_tokens == 4
|
||||
|
||||
|
||||
class TestAuxiliaryPoolAwareness:
|
||||
def test_try_nous_uses_pool_entry(self):
|
||||
|
|
|
|||
25
tests/agent/test_message_content.py
Normal file
25
tests/agent/test_message_content.py
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
from agent.message_content import flatten_message_text
|
||||
|
||||
|
||||
def test_flatten_message_text_accepts_chat_and_responses_text_parts():
|
||||
content = [
|
||||
{"type": "text", "text": "chat text"},
|
||||
{"type": "input_text", "text": "user text"},
|
||||
{"type": "output_text", "text": "assistant text"},
|
||||
{"type": "summary_text", "text": "summary text"},
|
||||
]
|
||||
|
||||
assert flatten_message_text(content) == "chat text\nuser text\nassistant text\nsummary text"
|
||||
|
||||
|
||||
def test_flatten_message_text_accepts_object_parts():
|
||||
content = [
|
||||
SimpleNamespace(type="output_text", text="object text"),
|
||||
{"content": "legacy content"},
|
||||
]
|
||||
|
||||
assert flatten_message_text(content) == "object text\nlegacy content"
|
||||
130
tests/agent/test_secret_scope.py
Normal file
130
tests/agent/test_secret_scope.py
Normal file
|
|
@ -0,0 +1,130 @@
|
|||
"""Tests for the profile-scoped credential primitive (Workstream A / Phase 2)."""
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope as ss
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_multiplex():
|
||||
"""Ensure each test starts and ends with multiplexing off (it's a global)."""
|
||||
ss.set_multiplex_active(False)
|
||||
yield
|
||||
ss.set_multiplex_active(False)
|
||||
|
||||
|
||||
class TestMultiplexInactiveBackwardCompat:
|
||||
"""Default deployment: get_secret transparently reads os.environ."""
|
||||
|
||||
def test_reads_environ(self, monkeypatch):
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test")
|
||||
assert ss.get_secret("ANTHROPIC_API_KEY") == "sk-test"
|
||||
|
||||
def test_missing_returns_default(self, monkeypatch):
|
||||
monkeypatch.delenv("NOPE_KEY", raising=False)
|
||||
assert ss.get_secret("NOPE_KEY") is None
|
||||
assert ss.get_secret("NOPE_KEY", "fallback") == "fallback"
|
||||
|
||||
def test_no_raise_without_scope(self, monkeypatch):
|
||||
monkeypatch.delenv("SOME_KEY", raising=False)
|
||||
# multiplex off => unscoped read is fine, returns default
|
||||
assert ss.get_secret("SOME_KEY") is None
|
||||
|
||||
|
||||
class TestMultiplexActiveFailClosed:
|
||||
"""Multiplex on: an unscoped secret read raises instead of leaking."""
|
||||
|
||||
def test_unscoped_read_raises(self, monkeypatch):
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-leaky")
|
||||
ss.set_multiplex_active(True)
|
||||
with pytest.raises(ss.UnscopedSecretError):
|
||||
ss.get_secret("ANTHROPIC_API_KEY")
|
||||
|
||||
def test_scoped_read_uses_scope_not_environ(self, monkeypatch):
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-from-environ")
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope({"ANTHROPIC_API_KEY": "sk-from-scope"})
|
||||
try:
|
||||
assert ss.get_secret("ANTHROPIC_API_KEY") == "sk-from-scope"
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
def test_scoped_missing_key_returns_default_not_environ(self, monkeypatch):
|
||||
# Even though the value exists in os.environ, a scope is authoritative:
|
||||
# an absent scope key must NOT fall through to the (cross-profile) env.
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-other-profile")
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope({"ANTHROPIC_API_KEY": "sk-mine"})
|
||||
try:
|
||||
assert ss.get_secret("OPENAI_API_KEY") is None
|
||||
assert ss.get_secret("OPENAI_API_KEY", "d") == "d"
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
def test_global_env_still_reads_environ_under_multiplex(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", "/opt/data")
|
||||
ss.set_multiplex_active(True)
|
||||
# No scope, multiplex on — but HERMES_HOME is global, so no raise.
|
||||
assert ss.get_secret("HERMES_HOME") == "/opt/data"
|
||||
|
||||
def test_kanban_prefix_is_global(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_KANBAN_DB", "/x/kanban.db")
|
||||
ss.set_multiplex_active(True)
|
||||
assert ss.get_secret("HERMES_KANBAN_DB") == "/x/kanban.db"
|
||||
|
||||
|
||||
class TestScopeIsolation:
|
||||
"""Two scopes never see each other's secrets."""
|
||||
|
||||
def test_nested_scopes_restore(self):
|
||||
ss.set_multiplex_active(True)
|
||||
t1 = ss.set_secret_scope({"K": "a"})
|
||||
try:
|
||||
assert ss.get_secret("K") == "a"
|
||||
t2 = ss.set_secret_scope({"K": "b"})
|
||||
try:
|
||||
assert ss.get_secret("K") == "b"
|
||||
finally:
|
||||
ss.reset_secret_scope(t2)
|
||||
assert ss.get_secret("K") == "a"
|
||||
finally:
|
||||
ss.reset_secret_scope(t1)
|
||||
|
||||
|
||||
class TestEnvFileParsing:
|
||||
"""load_env_file parses without mutating os.environ."""
|
||||
|
||||
def test_parses_basic(self, tmp_path):
|
||||
env = tmp_path / ".env"
|
||||
env.write_text(
|
||||
"# comment\n"
|
||||
"ANTHROPIC_API_KEY=sk-abc\n"
|
||||
"export OPENAI_API_KEY=sk-def\n"
|
||||
'QUOTED="quoted-value"\n'
|
||||
"SINGLE='single'\n"
|
||||
"\n"
|
||||
"BAD_LINE_NO_EQUALS\n"
|
||||
)
|
||||
out = ss.load_env_file(env)
|
||||
assert out == {
|
||||
"ANTHROPIC_API_KEY": "sk-abc",
|
||||
"OPENAI_API_KEY": "sk-def",
|
||||
"QUOTED": "quoted-value",
|
||||
"SINGLE": "single",
|
||||
}
|
||||
|
||||
def test_does_not_mutate_environ(self, tmp_path, monkeypatch):
|
||||
monkeypatch.delenv("ZZZ_KEY", raising=False)
|
||||
env = tmp_path / ".env"
|
||||
env.write_text("ZZZ_KEY=secret\n")
|
||||
ss.load_env_file(env)
|
||||
import os
|
||||
assert "ZZZ_KEY" not in os.environ
|
||||
|
||||
def test_missing_file_returns_empty(self, tmp_path):
|
||||
assert ss.load_env_file(tmp_path / "nope.env") == {}
|
||||
|
||||
def test_build_profile_secret_scope(self, tmp_path):
|
||||
(tmp_path / ".env").write_text("ANTHROPIC_API_KEY=sk-profile\n")
|
||||
assert ss.build_profile_secret_scope(tmp_path) == {
|
||||
"ANTHROPIC_API_KEY": "sk-profile"
|
||||
}
|
||||
|
|
@ -534,6 +534,14 @@ def pytest_configure(config): # noqa: D401 — pytest hook
|
|||
"behaviour — e.g. PTY tests that signal their own child).",
|
||||
)
|
||||
|
||||
# The pyproject addopts pin ``--timeout-method=signal`` relies on
|
||||
# ``signal.SIGALRM``, which does not exist on Windows — pytest-timeout
|
||||
# raises AttributeError at timer setup and the whole run aborts before any
|
||||
# test executes. Fall back to the thread-based timer on Windows so the
|
||||
# suite runs natively there (POSIX keeps the more reliable signal method).
|
||||
if sys.platform == "win32" and getattr(config.option, "timeout_method", None) == "signal":
|
||||
config.option.timeout_method = "thread"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _live_system_guard(request, monkeypatch):
|
||||
|
|
|
|||
|
|
@ -75,3 +75,68 @@ async def test_send_without_transport_returns_failure():
|
|||
result = await a.send("chat1", "hello")
|
||||
assert result.success is False
|
||||
assert result.error == "no transport"
|
||||
|
||||
|
||||
class _CaptureTransport:
|
||||
"""Minimal RelayTransport stand-in that records the outbound action."""
|
||||
|
||||
def __init__(self):
|
||||
self.sent = None
|
||||
|
||||
def set_inbound_handler(self, h): # noqa: D401
|
||||
self._h = h
|
||||
|
||||
async def send_outbound(self, action):
|
||||
self.sent = action
|
||||
return {"success": True, "message_id": "m1"}
|
||||
|
||||
|
||||
def _make_event(chat_id="chan-1", guild_id="guild-9"):
|
||||
from gateway.platforms.base import MessageEvent, MessageType
|
||||
from gateway.session import SessionSource
|
||||
|
||||
src = SessionSource(
|
||||
platform=Platform.RELAY,
|
||||
chat_id=chat_id,
|
||||
chat_type="channel",
|
||||
guild_id=guild_id,
|
||||
)
|
||||
return MessageEvent(text="hi", source=src, message_type=MessageType.TEXT)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_reattaches_guild_id_from_inbound_scope():
|
||||
"""The connector's egress guard resolves the owning tenant from
|
||||
metadata.guild_id; the gateway's generic delivery path drops it, so the
|
||||
relay adapter must re-attach the guild scope learned from the inbound event.
|
||||
Regression for live 'discord egress declined: target not routed to an
|
||||
onboarded tenant'."""
|
||||
t = _CaptureTransport()
|
||||
a = RelayAdapter(PlatformConfig(), make_desc(platform="discord"), transport=t)
|
||||
# Simulate the connector delivering an inbound message in guild-9 / chan-1,
|
||||
# but don't run the full handle_message pipeline — just the scope capture.
|
||||
a._capture_scope(_make_event(chat_id="chan-1", guild_id="guild-9"))
|
||||
|
||||
await a.send("chan-1", "the reply")
|
||||
|
||||
assert t.sent["metadata"].get("guild_id") == "guild-9"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_without_known_scope_omits_guild_id():
|
||||
"""A chat we never saw inbound (e.g. a DM) gets no guild_id — no-op, never
|
||||
invents a scope."""
|
||||
t = _CaptureTransport()
|
||||
a = RelayAdapter(PlatformConfig(), make_desc(platform="discord"), transport=t)
|
||||
await a.send("unknown-chat", "hi")
|
||||
assert "guild_id" not in t.sent["metadata"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_preserves_explicit_guild_id():
|
||||
"""An explicitly-provided metadata.guild_id is never overwritten."""
|
||||
t = _CaptureTransport()
|
||||
a = RelayAdapter(PlatformConfig(), make_desc(platform="discord"), transport=t)
|
||||
a._capture_scope(_make_event(chat_id="chan-1", guild_id="guild-9"))
|
||||
await a.send("chan-1", "hi", metadata={"guild_id": "explicit-1"})
|
||||
assert t.sent["metadata"]["guild_id"] == "explicit-1"
|
||||
|
|
|
|||
|
|
@ -177,3 +177,25 @@ async def test_disconnect_fails_pending_waiters_cleanly(server):
|
|||
# After disconnect, an outbound returns a structured failure rather than hanging.
|
||||
result = await t.send_outbound({"op": "send", "chat_id": "c", "content": "x"})
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
def test_https_url_normalized_to_wss():
|
||||
"""The relay URL is configured once as the http(s):// BASE (for the provision
|
||||
POST), but websockets.connect needs ws(s):// and the connector mounts its WS
|
||||
server at /relay. The transport must convert scheme AND ensure the /relay
|
||||
path. Regression for the live staging failures 'scheme isn't ws or wss' then
|
||||
'server rejected WebSocket connection: HTTP 400' (wrong path)."""
|
||||
t = WebSocketRelayTransport("https://connector.example", "discord", "b")
|
||||
assert t._url == "wss://connector.example/relay"
|
||||
t2 = WebSocketRelayTransport("http://connector.local:8080", "discord", "b")
|
||||
assert t2._url == "ws://connector.local:8080/relay"
|
||||
|
||||
|
||||
def test_ws_dial_url_idempotent_with_scheme_and_path():
|
||||
# Already ws(s):// and/or already ending in /relay -> unchanged (no double append).
|
||||
t = WebSocketRelayTransport("wss://connector.example/relay", "discord", "b")
|
||||
assert t._url == "wss://connector.example/relay"
|
||||
t2 = WebSocketRelayTransport("https://connector.example/relay/", "discord", "b")
|
||||
assert t2._url == "wss://connector.example/relay"
|
||||
t3 = WebSocketRelayTransport("ws://127.0.0.1:9", "discord", "b")
|
||||
assert t3._url == "ws://127.0.0.1:9/relay"
|
||||
|
|
|
|||
|
|
@ -337,6 +337,40 @@ class TestAdapterInit:
|
|||
assert isinstance(agent, FakeAgent)
|
||||
assert captured["reasoning_config"] == {"enabled": True, "effort": "xhigh"}
|
||||
|
||||
def test_create_agent_refreshes_max_iterations_from_runtime_config(self, monkeypatch):
|
||||
captured = {}
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
monkeypatch.setattr("run_agent.AIAgent", FakeAgent)
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: {
|
||||
"provider": "openai",
|
||||
"base_url": "https://example.test/v1",
|
||||
"api_mode": "chat_completions",
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr("gateway.run._resolve_gateway_model", lambda: "gpt-5")
|
||||
monkeypatch.setattr("gateway.run._load_gateway_config", lambda: {"agent": {"max_turns": 200}})
|
||||
monkeypatch.setattr(
|
||||
"gateway.run.GatewayRunner._load_reasoning_config",
|
||||
staticmethod(lambda: {}),
|
||||
)
|
||||
monkeypatch.setattr("gateway.run.GatewayRunner._load_fallback_model", staticmethod(lambda: None))
|
||||
monkeypatch.setattr("gateway.run._current_max_iterations", lambda: 200)
|
||||
monkeypatch.setattr("hermes_cli.tools_config._get_platform_tools", lambda *_: set())
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
|
||||
agent = adapter._create_agent(session_id="api-session")
|
||||
|
||||
assert isinstance(agent, FakeAgent)
|
||||
assert captured["max_iterations"] == 200
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Auth checking
|
||||
|
|
|
|||
92
tests/gateway/test_cached_agent_max_iterations.py
Normal file
92
tests/gateway/test_cached_agent_max_iterations.py
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
"""Regression tests for PR #48127: cached agent max_iterations refresh.
|
||||
|
||||
When a long-lived gateway reuses an agent from its cache, the agent must run
|
||||
the *current* configured iteration budget — not the budget it was constructed
|
||||
with on the first turn of that session. Two pieces make that true:
|
||||
|
||||
1. ``GatewayRunner._init_cached_agent_for_turn`` must NOT reset
|
||||
``max_iterations`` itself (the gateway refreshes it explicitly right after,
|
||||
from current config). If this helper ever started clobbering it, the
|
||||
gateway's refresh would be silently undone.
|
||||
2. The per-turn budget object is rebuilt from ``agent.max_iterations`` at the
|
||||
start of every turn (``agent/turn_context.py`` -> ``IterationBudget``), so
|
||||
refreshing ``max_iterations`` on the cached agent is sufficient to change
|
||||
the operative cap the agent loop checks.
|
||||
|
||||
These tests exercise the real code paths rather than asserting a plain
|
||||
assignment, so they fail if either contract regresses.
|
||||
"""
|
||||
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
|
||||
from agent.iteration_budget import IterationBudget
|
||||
|
||||
|
||||
def _make_cached_agent(max_iterations: int) -> SimpleNamespace:
|
||||
"""A minimal stand-in cached agent with the attributes the helpers touch."""
|
||||
# The turn loop checks both api_call_count >= max_iterations AND
|
||||
# iteration_budget.remaining <= 0 (turn_finalizer.py), so the budget must
|
||||
# also reflect the new cap. Seed it with the stale value to prove the
|
||||
# refresh propagates.
|
||||
return SimpleNamespace(
|
||||
_last_activity_ts=time.time() - 1000,
|
||||
_last_activity_desc="previous turn",
|
||||
_api_call_count=42,
|
||||
_last_flushed_db_idx=5,
|
||||
max_iterations=max_iterations,
|
||||
iteration_budget=IterationBudget(max_iterations),
|
||||
)
|
||||
|
||||
|
||||
def test_init_cached_agent_for_turn_does_not_touch_max_iterations():
|
||||
"""The per-turn reset helper must leave max_iterations untouched.
|
||||
|
||||
The gateway refreshes max_iterations explicitly right after calling this
|
||||
helper; if the helper ever reset it, that refresh would be undone.
|
||||
"""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
agent = _make_cached_agent(90)
|
||||
GatewayRunner._init_cached_agent_for_turn(agent, interrupt_depth=0)
|
||||
|
||||
# Per-turn state was reset...
|
||||
assert agent._api_call_count == 0
|
||||
assert agent._last_activity_desc == "starting new turn (cached)"
|
||||
assert agent._last_flushed_db_idx == 0
|
||||
# ...but the iteration budget was NOT changed by the helper itself.
|
||||
assert agent.max_iterations == 90
|
||||
|
||||
|
||||
def test_init_cached_agent_preserves_max_iterations_on_interrupt_depth():
|
||||
"""Interrupt-recursive turns must also leave max_iterations alone."""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
agent = _make_cached_agent(200)
|
||||
GatewayRunner._init_cached_agent_for_turn(agent, interrupt_depth=1)
|
||||
|
||||
# Activity timestamps preserved for the inactivity watchdog (#15654)...
|
||||
assert agent._last_activity_desc == "previous turn"
|
||||
# ...and max_iterations untouched.
|
||||
assert agent.max_iterations == 200
|
||||
|
||||
|
||||
def test_refreshed_max_iterations_propagates_to_turn_budget():
|
||||
"""Refreshing max_iterations on a cached agent changes the operative cap.
|
||||
|
||||
The gateway sets ``agent.max_iterations = max_iterations`` on cache reuse;
|
||||
the new turn's setup then rebuilds ``iteration_budget`` from it. This proves
|
||||
the refresh actually moves the budget the agent loop enforces — the cached
|
||||
agent started at 90 and ends a new turn capped at 200.
|
||||
"""
|
||||
agent = _make_cached_agent(90)
|
||||
assert agent.iteration_budget.max_total == 90
|
||||
|
||||
# Gateway refresh on cache reuse:
|
||||
agent.max_iterations = 200
|
||||
|
||||
# Start-of-turn budget rebuild (agent/turn_context.py:166):
|
||||
agent.iteration_budget = IterationBudget(agent.max_iterations)
|
||||
|
||||
assert agent.iteration_budget.max_total == 200
|
||||
assert agent.iteration_budget.remaining == 200
|
||||
|
|
@ -311,6 +311,55 @@ class TestLoadGatewayConfig:
|
|||
|
||||
assert config.quick_commands == {"limits": {"type": "exec", "command": "echo ok"}}
|
||||
|
||||
def test_relay_platform_enabled_from_env_url(self, tmp_path, monkeypatch):
|
||||
"""GATEWAY_RELAY_URL must enable Platform.RELAY in config.platforms so
|
||||
start_gateway()'s connect loop actually dials the connector. Registering
|
||||
the adapter in the platform_registry is NOT enough — the connect loop
|
||||
iterates config.platforms, so an un-enabled RELAY never connects (the
|
||||
'relay registered but no inbound' bug)."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setenv("GATEWAY_RELAY_URL", "https://connector.example/relay/")
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
assert Platform.RELAY in config.platforms
|
||||
relay = config.platforms[Platform.RELAY]
|
||||
assert relay.enabled is True
|
||||
# Trailing slash stripped; mirrored into extra for the connected-checker.
|
||||
assert relay.extra.get("relay_url") == "https://connector.example/relay"
|
||||
assert Platform.RELAY in config.get_connected_platforms()
|
||||
|
||||
def test_relay_platform_absent_when_url_unset(self, tmp_path, monkeypatch):
|
||||
"""No relay URL -> no RELAY platform, so direct/single-tenant gateways
|
||||
are unaffected."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("GATEWAY_RELAY_URL", raising=False)
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
assert Platform.RELAY not in config.platforms
|
||||
|
||||
def test_relay_platform_enabled_from_config_yaml(self, tmp_path, monkeypatch):
|
||||
"""gateway.relay_url in config.yaml also enables RELAY (env-less path)."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"gateway:\n platforms:\n relay:\n extra:\n relay_url: https://connector.example/relay\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("GATEWAY_RELAY_URL", raising=False)
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
assert Platform.RELAY in config.platforms
|
||||
assert config.platforms[Platform.RELAY].enabled is True
|
||||
|
||||
def test_bridges_group_sessions_per_user_from_config_yaml(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
|
|
|||
|
|
@ -122,13 +122,56 @@ class TestClarifyChoiceViewConstruction:
|
|||
clarify_id="cidZ",
|
||||
allowed_user_ids=set(),
|
||||
)
|
||||
# 75 chars + 3 ellipsis chars in the body, plus "1. " prefix
|
||||
# 78 chars + single-char ellipsis in the body, plus "1. " prefix.
|
||||
# Uses U+2026 (…) instead of "..." to fit the 80-char Discord cap.
|
||||
first_label = view.children[0].label
|
||||
assert first_label.startswith("1. ")
|
||||
assert first_label.endswith("...")
|
||||
assert first_label.endswith("\u2026")
|
||||
# Final label total <= 80 (Discord cap on button labels)
|
||||
assert len(first_label) <= 80
|
||||
|
||||
def test_truncates_long_choice_label_breaks_on_word_boundary(self):
|
||||
# Long choice with spaces — should cut at the last whole word so the
|
||||
# trailing text stays readable on Discord mobile.
|
||||
long_choice = (
|
||||
"Tight, well-illustrated, covers all 3 audiences "
|
||||
"(patients, families, curious general readers)"
|
||||
)
|
||||
view = ClarifyChoiceView(
|
||||
choices=[long_choice],
|
||||
clarify_id="cidW",
|
||||
allowed_user_ids=set(),
|
||||
)
|
||||
first_label = view.children[0].label
|
||||
assert first_label.startswith("1. ")
|
||||
assert first_label.endswith("\u2026")
|
||||
# No mid-word fragment before the ellipsis.
|
||||
assert not first_label.rstrip("\u2026").endswith("(")
|
||||
|
||||
def test_truncates_long_no_space_choice_on_soft_boundary(self):
|
||||
# A long choice with soft boundaries (commas, hyphens) but no spaces
|
||||
# should still cut on a soft boundary, not mid-word. We use an input
|
||||
# where position 76 is NOT a soft boundary — the test only passes
|
||||
# if the renderer actively searches backward for a soft char
|
||||
# rather than blindly cutting at the budget limit.
|
||||
long_choice = "a" * 30 + "-" + "b" * 30 + "-" + "c" * 30 + "-" + "d" * 30
|
||||
# 30a-30b-30c-30d = 30 + 1 + 30 + 1 + 30 + 1 + 30 = 123 chars
|
||||
# Position 76 is 'b' (a mid-word alpha). The renderer must look back
|
||||
# for a '-' to cut on.
|
||||
view = ClarifyChoiceView(
|
||||
choices=[long_choice],
|
||||
clarify_id="cidSB",
|
||||
allowed_user_ids=set(),
|
||||
)
|
||||
first_label = view.children[0].label
|
||||
assert first_label.endswith("\u2026")
|
||||
assert len(first_label) <= 80
|
||||
body = first_label[len("1. "):].rstrip("\u2026")
|
||||
last_char = body[-1]
|
||||
assert last_char in {"-", ",", ".", ")", " "}, (
|
||||
f"Label cuts mid-word at {last_char!r}: {first_label!r}"
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Choice callback → resolve_gateway_clarify
|
||||
|
|
@ -404,3 +447,134 @@ class TestDiscordSendClarify:
|
|||
# Only 1 real choice + 1 Other = 2 children
|
||||
assert len(view.children) == 2
|
||||
assert "real-choice" in view.children[0].label
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unwraps_dict_choices_to_description(self):
|
||||
# LLMs sometimes emit [{"description": "..."}] instead of bare strings
|
||||
# — the renderer must unwrap common dict shapes, not str() the whole
|
||||
# dict into a Python repr on the button label.
|
||||
adapter = _make_adapter()
|
||||
channel = MagicMock()
|
||||
sent_msg = MagicMock()
|
||||
sent_msg.id = 555
|
||||
channel.send = AsyncMock(return_value=sent_msg)
|
||||
adapter._client.get_channel = MagicMock(return_value=channel)
|
||||
|
||||
malformed = [
|
||||
{"description": "Tight, well-illustrated"},
|
||||
{"label": "Use label key"},
|
||||
{"text": "Use text key"},
|
||||
"normal-string", # strings still pass through
|
||||
]
|
||||
await adapter.send_clarify(
|
||||
chat_id="9001",
|
||||
question="?",
|
||||
choices=malformed,
|
||||
clarify_id="cidU",
|
||||
session_key="sk-U",
|
||||
)
|
||||
kwargs = channel.send.call_args.kwargs
|
||||
view = kwargs["view"]
|
||||
labels = [b.label for b in view.children[:-1]] # exclude Other
|
||||
# No raw Python repr should leak onto any label.
|
||||
for label in labels:
|
||||
assert "{'" not in label
|
||||
assert "':" not in label
|
||||
# Each dict unwrapped to its inner string.
|
||||
assert any("Tight, well-illustrated" in lbl for lbl in labels)
|
||||
assert any("Use label key" in lbl for lbl in labels)
|
||||
assert any("Use text key" in lbl for lbl in labels)
|
||||
assert any("normal-string" in lbl for lbl in labels)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unwrap_prefers_description_over_name_in_multi_key_dict(self):
|
||||
# When the LLM emits both 'name' (often a short identifier in
|
||||
# OpenAI-style tool calls) and 'description' (the user-facing text),
|
||||
# the renderer must surface 'description'. The user should never see
|
||||
# a 4-char model identifier on a button label.
|
||||
adapter = _make_adapter()
|
||||
channel = MagicMock()
|
||||
sent_msg = MagicMock()
|
||||
sent_msg.id = 666
|
||||
channel.send = AsyncMock(return_value=sent_msg)
|
||||
adapter._client.get_channel = MagicMock(return_value=channel)
|
||||
|
||||
await adapter.send_clarify(
|
||||
chat_id="9001",
|
||||
question="?",
|
||||
choices=[{"name": "tight", "description": "Tight, well-illustrated"}],
|
||||
clarify_id="cidN",
|
||||
session_key="sk-N",
|
||||
)
|
||||
kwargs = channel.send.call_args.kwargs
|
||||
view = kwargs["view"]
|
||||
choice_label = view.children[0].label
|
||||
assert "Tight, well-illustrated" in choice_label
|
||||
# The 'name' value (a short identifier) must NOT have leaked.
|
||||
body = choice_label.split("1. ", 1)[1].rstrip("\u2026")
|
||||
assert "tight" not in body, f"'name' leaked onto button: {choice_label!r}"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unwrap_prefers_label_over_description(self):
|
||||
# When both 'label' and 'description' are present, 'label' wins.
|
||||
# 'label' is the canonical short user-facing text in most LLM tool
|
||||
# conventions; 'description' is the longer explanation.
|
||||
adapter = _make_adapter()
|
||||
channel = MagicMock()
|
||||
sent_msg = MagicMock()
|
||||
sent_msg.id = 777
|
||||
channel.send = AsyncMock(return_value=sent_msg)
|
||||
adapter._client.get_channel = MagicMock(return_value=channel)
|
||||
|
||||
await adapter.send_clarify(
|
||||
chat_id="9001",
|
||||
question="?",
|
||||
choices=[{"label": "Short", "description": "Long verbose explanation"}],
|
||||
clarify_id="cidL",
|
||||
session_key="sk-L",
|
||||
)
|
||||
kwargs = channel.send.call_args.kwargs
|
||||
view = kwargs["view"]
|
||||
choice_label = view.children[0].label
|
||||
assert "Short" in choice_label
|
||||
# The longer description must NOT have leaked.
|
||||
assert "Long verbose" not in choice_label, (
|
||||
f"'description' leaked over 'label': {choice_label!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unwrap_does_not_pick_value_or_name_alone(self):
|
||||
# 'name' and 'value' are Discord-component-shaped fields that could
|
||||
# accidentally appear in dicts not intended as choices (e.g., a
|
||||
# developer-error in the gateway wiring). The renderer should not
|
||||
# surface them as button labels — only the well-known LLM tool-call
|
||||
# keys (label, description, text, title) should win.
|
||||
adapter = _make_adapter()
|
||||
channel = MagicMock()
|
||||
sent_msg = MagicMock()
|
||||
sent_msg.id = 888
|
||||
channel.send = AsyncMock(return_value=sent_msg)
|
||||
adapter._client.get_channel = MagicMock(return_value=channel)
|
||||
|
||||
await adapter.send_clarify(
|
||||
chat_id="9001",
|
||||
question="?",
|
||||
choices=[
|
||||
{"name": "only_name_here"}, # should be filtered out
|
||||
{"value": "only_value_here"}, # should be filtered out
|
||||
{"description": "real choice"},
|
||||
],
|
||||
clarify_id="cidNV",
|
||||
session_key="sk-NV",
|
||||
)
|
||||
kwargs = channel.send.call_args.kwargs
|
||||
view = kwargs["view"]
|
||||
choice_labels = [b.label for b in view.children[:-1]] # exclude Other
|
||||
# Only the well-formed dict survives.
|
||||
assert len(choice_labels) == 1, (
|
||||
f"Expected 1 choice, got {len(choice_labels)}: {choice_labels!r}"
|
||||
)
|
||||
assert "real choice" in choice_labels[0]
|
||||
for label in choice_labels:
|
||||
assert "only_name_here" not in label, f"name leaked: {label!r}"
|
||||
assert "only_value_here" not in label, f"value leaked: {label!r}"
|
||||
|
|
|
|||
|
|
@ -666,6 +666,70 @@ async def test_fetch_channel_context_stops_at_self_message_and_reverses_to_chron
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_skips_self_improvement_boundary_message(adapter, monkeypatch):
|
||||
"""Delayed harness status bumps must not hide messages after the real reply."""
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "all")
|
||||
adapter.config.extra["history_backfill_limit"] = 10
|
||||
|
||||
codex = SimpleNamespace(id=55, display_name="Codex", name="Codex", bot=True)
|
||||
human = SimpleNamespace(id=56, display_name="Alice", name="Alice", bot=False)
|
||||
|
||||
channel = FakeHistoryChannel(
|
||||
[
|
||||
make_history_message(
|
||||
author=adapter._client.user,
|
||||
content="arbitrary lifecycle text from a metadata-marked send",
|
||||
msg_id=9,
|
||||
),
|
||||
make_history_message(
|
||||
author=adapter._client.user,
|
||||
content="[Background process bg-123 finished with exit code 0~ Here's the final output:\nok]",
|
||||
msg_id=8,
|
||||
),
|
||||
make_history_message(
|
||||
author=codex,
|
||||
content="♻ Gateway restarted successfully. Your session continues.",
|
||||
msg_id=7,
|
||||
),
|
||||
make_history_message(
|
||||
author=codex,
|
||||
content="💾 Self-improvement review: Memory updated",
|
||||
msg_id=6,
|
||||
),
|
||||
make_history_message(author=human, content="question after reply", msg_id=5),
|
||||
make_history_message(
|
||||
author=adapter._client.user,
|
||||
content="💾 Self-improvement review: Skill 'hermes-gateway-display-config' patched",
|
||||
msg_id=4,
|
||||
),
|
||||
make_history_message(author=codex, content="Codex final answer", msg_id=3),
|
||||
make_history_message(author=human, content="prompt before reply", msg_id=2),
|
||||
make_history_message(author=adapter._client.user, content="our prior response", msg_id=1),
|
||||
],
|
||||
channel_id=123,
|
||||
)
|
||||
adapter._nonconversational_messages.mark_many(["9"])
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=make_message(channel=channel, content="trigger"))
|
||||
|
||||
assert result == (
|
||||
"[Recent channel messages]\n"
|
||||
"[Alice] prompt before reply\n"
|
||||
"[Codex [bot]] Codex final answer\n"
|
||||
"[Alice] question after reply"
|
||||
)
|
||||
|
||||
|
||||
def test_nonconversational_fallback_requires_self_improvement_emoji():
|
||||
assert discord_platform._looks_like_nonconversational_history_message(
|
||||
"💾 Self-improvement review: Memory updated"
|
||||
)
|
||||
assert not discord_platform._looks_like_nonconversational_history_message(
|
||||
"Self-improvement review: this is a normal assistant heading"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_skips_other_bots_when_allow_bots_none(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "none")
|
||||
|
|
@ -801,6 +865,33 @@ async def test_fetch_channel_context_ignores_stale_cache(adapter, monkeypatch):
|
|||
assert recorded_after["value"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discord_send_does_not_cache_nonconversational_status_as_history_boundary(adapter):
|
||||
"""Automated status notifications should not move the backfill boundary."""
|
||||
|
||||
class SendingChannel(FakeTextChannel):
|
||||
async def send(self, content, reference=None):
|
||||
return SimpleNamespace(id=222)
|
||||
|
||||
channel = SendingChannel(channel_id=777)
|
||||
adapter._client = SimpleNamespace(
|
||||
user=adapter._client.user,
|
||||
get_channel=lambda channel_id: channel if channel_id == 777 else None,
|
||||
fetch_channel=AsyncMock(return_value=channel),
|
||||
)
|
||||
adapter._last_self_message_id["777"] = "111"
|
||||
|
||||
result = await adapter.send(
|
||||
"777",
|
||||
"arbitrary lifecycle text from gateway",
|
||||
metadata={"non_conversational": True},
|
||||
)
|
||||
|
||||
assert result.success is True
|
||||
assert adapter._last_self_message_id["777"] == "111"
|
||||
assert "222" in adapter._nonconversational_messages
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discord_shared_channel_backfill_prepends_context(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true")
|
||||
|
|
@ -925,5 +1016,3 @@ async def test_discord_auto_thread_skips_backfill(adapter, monkeypatch):
|
|||
|
||||
adapter._auto_create_thread.assert_awaited_once()
|
||||
adapter._fetch_channel_context.assert_not_awaited()
|
||||
|
||||
|
||||
|
|
|
|||
60
tests/gateway/test_gateway_command_line_matcher.py
Normal file
60
tests/gateway/test_gateway_command_line_matcher.py
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
"""Tests for the strict gateway command-line matcher.
|
||||
|
||||
Regression guard for the Windows ``hermes gateway restart`` silent-outage bug:
|
||||
the previous loose substring match (``"... gateway" in cmdline``) false-matched
|
||||
``gateway status``/``dashboard`` siblings and unrelated processes such as
|
||||
``python -m tui_gateway``, which let ``restart()`` race a still-draining old
|
||||
process and ``status``/``start`` report false positives.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway.status import looks_like_gateway_command_line as matches
|
||||
|
||||
|
||||
ACCEPT = [
|
||||
"pythonw.exe -m hermes_cli.main gateway run",
|
||||
r"C:\Users\me\hermes\venv\Scripts\pythonw.exe -m hermes_cli.main gateway run",
|
||||
"python -m hermes_cli.main --profile work gateway run",
|
||||
"python -m hermes_cli.main gateway run --replace",
|
||||
"python -m hermes_cli/main.py gateway run",
|
||||
"python gateway/run.py",
|
||||
"hermes-gateway.exe",
|
||||
"hermes gateway", # bare `hermes gateway` defaults to run
|
||||
"hermes gateway run",
|
||||
# profile selector AFTER the `gateway` token (argv is profile-position
|
||||
# agnostic — _apply_profile_override strips --profile/-p anywhere)
|
||||
"hermes gateway --profile work run",
|
||||
"python -m hermes_cli.main gateway -p work run",
|
||||
"hermes gateway --profile=work run",
|
||||
# a profile literally NAMED "gateway"
|
||||
"hermes -p gateway gateway run",
|
||||
"python -m hermes_cli.main --profile gateway gateway run",
|
||||
# quoted Windows paths with spaces (shlex-aware tokenization)
|
||||
r'"C:\Program Files\Hermes\hermes-gateway.exe"',
|
||||
r'"C:\Program Files\Hermes\gateway\run.py" run',
|
||||
r'"C:\Program Files\Py\pythonw.exe" -m hermes_cli.main gateway run',
|
||||
]
|
||||
|
||||
REJECT = [
|
||||
"python -m tui_gateway", # unrelated module
|
||||
"python -m hermes_cli.main gateway status", # other subcommand
|
||||
"python -m hermes_cli.main gateway restart",
|
||||
"python -m hermes_cli.main gateway stop",
|
||||
"python -m hermes_cli.main --profile x dashboard", # non-gateway subcommand
|
||||
"some random python -m mygateway thing",
|
||||
"",
|
||||
None,
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cmd", ACCEPT)
|
||||
def test_accepts_real_gateway_run(cmd):
|
||||
assert matches(cmd) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cmd", REJECT)
|
||||
def test_rejects_non_gateway_run(cmd):
|
||||
assert matches(cmd) is False
|
||||
|
|
@ -43,3 +43,27 @@ def test_watcher_loops_are_coroutines():
|
|||
# The two long-running watchers are async loops.
|
||||
assert inspect.iscoroutinefunction(GatewayKanbanWatchersMixin._kanban_notifier_watcher)
|
||||
assert inspect.iscoroutinefunction(GatewayKanbanWatchersMixin._kanban_dispatcher_watcher)
|
||||
|
||||
|
||||
def test_singleton_dispatcher_lock_is_exclusive(tmp_path):
|
||||
"""Only one holder of the dispatcher lock at a time — the backstop that
|
||||
stops concurrent dispatchers double reclaiming and corrupting shared
|
||||
kanban SQLite index pages under wal_autocheckpoint=0."""
|
||||
import os
|
||||
|
||||
from gateway.kanban_watchers import _acquire_singleton_lock, _release_singleton_lock
|
||||
|
||||
lock = tmp_path / "kanban" / ".dispatcher.lock"
|
||||
|
||||
h1, st1 = _acquire_singleton_lock(lock)
|
||||
assert st1 == "held" and h1 is not None
|
||||
|
||||
# A second acquire while the first is held must be refused, not granted.
|
||||
h2, st2 = _acquire_singleton_lock(lock)
|
||||
assert st2 == "contended" and h2 is None
|
||||
|
||||
# Releasing the first lets a fresh acquire succeed (lock is reusable).
|
||||
_release_singleton_lock(h1)
|
||||
h3, st3 = _acquire_singleton_lock(lock)
|
||||
assert st3 == "held" and h3 is not None
|
||||
_release_singleton_lock(h3)
|
||||
|
|
|
|||
|
|
@ -156,3 +156,46 @@ async def test_model_global_persists_when_config_has_proper_dict_model(tmp_path,
|
|||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
assert written["model"]["provider"] == "openrouter"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_no_flag_persists_by_default(tmp_path, monkeypatch):
|
||||
"""A plain ``/model X`` (no --global) now persists to config.yaml.
|
||||
|
||||
This is the user-facing fix: switching models in one session survives
|
||||
into the next without re-typing the switch every time.
|
||||
"""
|
||||
cfg_path = _setup_isolated_home(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
{"default": "old-model", "provider": "openai-codex"},
|
||||
)
|
||||
|
||||
result = await _make_runner()._handle_model_command(
|
||||
_make_event("/model gpt-5.5")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert "gpt-5.5" in result
|
||||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
assert written["model"]["default"] == "gpt-5.5"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_model_session_flag_does_not_persist(tmp_path, monkeypatch):
|
||||
"""``/model X --session`` opts out of persistence even under the new default."""
|
||||
cfg_path = _setup_isolated_home(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
{"default": "old-model", "provider": "openai-codex"},
|
||||
)
|
||||
|
||||
result = await _make_runner()._handle_model_command(
|
||||
_make_event("/model gpt-5.5 --session")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert "gpt-5.5" in result
|
||||
written = yaml.safe_load(cfg_path.read_text(encoding="utf-8"))
|
||||
# Config untouched — the session override is in-memory only.
|
||||
assert written["model"]["default"] == "old-model"
|
||||
|
|
|
|||
136
tests/gateway/test_multiplex_adapter_registry.py
Normal file
136
tests/gateway/test_multiplex_adapter_registry.py
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
"""Phase 3: secondary-profile adapter registry + same-token conflict detection."""
|
||||
import pytest
|
||||
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
|
||||
class _FakeAdapter:
|
||||
def __init__(self, token=None):
|
||||
self.token = token
|
||||
|
||||
|
||||
class TestCredentialFingerprint:
|
||||
def test_none_without_token(self):
|
||||
assert GatewayRunner._adapter_credential_fingerprint(_FakeAdapter()) is None
|
||||
|
||||
def test_stable_and_log_safe(self):
|
||||
a = _FakeAdapter(token="secret-bot-token")
|
||||
fp1 = GatewayRunner._adapter_credential_fingerprint(a)
|
||||
fp2 = GatewayRunner._adapter_credential_fingerprint(_FakeAdapter(token="secret-bot-token"))
|
||||
assert fp1 == fp2 # stable
|
||||
assert "secret-bot-token" not in (fp1 or "") # never the raw token
|
||||
assert len(fp1) == 16
|
||||
|
||||
def test_distinct_tokens_distinct_fp(self):
|
||||
a = GatewayRunner._adapter_credential_fingerprint(_FakeAdapter(token="tok-A"))
|
||||
b = GatewayRunner._adapter_credential_fingerprint(_FakeAdapter(token="tok-B"))
|
||||
assert a != b
|
||||
|
||||
def test_reads_alt_attrs(self):
|
||||
class _AltAdapter:
|
||||
def __init__(self):
|
||||
self.bot_token = "alt-token"
|
||||
assert GatewayRunner._adapter_credential_fingerprint(_AltAdapter()) is not None
|
||||
|
||||
|
||||
class TestProfileMessageHandler:
|
||||
@pytest.mark.asyncio
|
||||
async def test_stamps_profile_on_unstamped_source(self):
|
||||
runner = GatewayRunner.__new__(GatewayRunner)
|
||||
seen = {}
|
||||
|
||||
async def _fake_handle(event):
|
||||
seen["profile"] = event.source.profile
|
||||
return "ok"
|
||||
|
||||
runner._handle_message = _fake_handle
|
||||
handler = runner._make_profile_message_handler("coder")
|
||||
|
||||
class _Src:
|
||||
profile = None
|
||||
|
||||
class _Evt:
|
||||
source = _Src()
|
||||
|
||||
result = await handler(_Evt())
|
||||
assert result == "ok"
|
||||
assert seen["profile"] == "coder"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_does_not_override_existing_profile(self):
|
||||
runner = GatewayRunner.__new__(GatewayRunner)
|
||||
seen = {}
|
||||
|
||||
async def _fake_handle(event):
|
||||
seen["profile"] = event.source.profile
|
||||
return "ok"
|
||||
|
||||
runner._handle_message = _fake_handle
|
||||
handler = runner._make_profile_message_handler("coder")
|
||||
|
||||
class _Src:
|
||||
profile = "writer" # already stamped (e.g. by URL prefix)
|
||||
|
||||
class _Evt:
|
||||
source = _Src()
|
||||
|
||||
await handler(_Evt())
|
||||
assert seen["profile"] == "writer"
|
||||
|
||||
|
||||
class TestPortBindingHardError:
|
||||
"""A secondary profile enabling a port-binding platform aborts startup."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_secondary_webhook_raises(self, monkeypatch):
|
||||
from gateway.run import MultiplexConfigError
|
||||
from gateway.config import GatewayConfig, Platform, PlatformConfig
|
||||
|
||||
runner = GatewayRunner.__new__(GatewayRunner)
|
||||
runner.config = GatewayConfig(multiplex_profiles=True)
|
||||
runner._profile_adapters = {}
|
||||
|
||||
# reviewer profile config enables webhook (a port-binding platform)
|
||||
reviewer_cfg = GatewayConfig(multiplex_profiles=True)
|
||||
reviewer_cfg.platforms = {
|
||||
Platform.WEBHOOK: PlatformConfig(enabled=True, extra={"port": 8644}),
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"gateway.config.load_gateway_config", lambda: reviewer_cfg
|
||||
)
|
||||
|
||||
with pytest.raises(MultiplexConfigError) as ei:
|
||||
await runner._start_one_profile_adapters("reviewer", "/tmp/x", {})
|
||||
assert "webhook" in str(ei.value)
|
||||
assert "reviewer" in str(ei.value)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_secondary_non_binding_platform_ok(self, monkeypatch):
|
||||
"""A non-port-binding platform (e.g. telegram) is NOT rejected."""
|
||||
from gateway.config import GatewayConfig, Platform, PlatformConfig
|
||||
|
||||
runner = GatewayRunner.__new__(GatewayRunner)
|
||||
runner.config = GatewayConfig(multiplex_profiles=True)
|
||||
runner._profile_adapters = {}
|
||||
|
||||
reviewer_cfg = GatewayConfig(multiplex_profiles=True)
|
||||
reviewer_cfg.platforms = {
|
||||
Platform.TELEGRAM: PlatformConfig(enabled=True, token="t"),
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"gateway.config.load_gateway_config", lambda: reviewer_cfg
|
||||
)
|
||||
# _create_adapter returns None here (no real telegram token wiring), so
|
||||
# the loop simply connects nothing — the key assertion is NO raise.
|
||||
monkeypatch.setattr(runner, "_create_adapter", lambda p, c: None)
|
||||
|
||||
connected = await runner._start_one_profile_adapters("reviewer", "/tmp/x", {})
|
||||
assert connected == 0 # nothing connected, but no MultiplexConfigError
|
||||
|
||||
def test_port_binding_set_covers_known_listeners(self):
|
||||
from gateway.run import _PORT_BINDING_PLATFORM_VALUES
|
||||
# Every adapter that binds a TCP port must be in the guard set.
|
||||
for p in ("webhook", "api_server", "msgraph_webhook", "feishu",
|
||||
"wecom_callback", "bluebubbles", "sms"):
|
||||
assert p in _PORT_BINDING_PLATFORM_VALUES
|
||||
|
||||
88
tests/gateway/test_multiplex_credential_isolation.py
Normal file
88
tests/gateway/test_multiplex_credential_isolation.py
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
"""End-to-end credential isolation proof for multiplex mode (Workstream A).
|
||||
|
||||
These exercise the REAL resolution path (runtime_provider, secret scope, MCP
|
||||
interpolation) rather than mocking it, proving the property that matters: two
|
||||
profiles with different keys never see each other's, and an unscoped read in
|
||||
multiplex mode fails closed instead of leaking.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope as ss
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset(monkeypatch):
|
||||
ss.set_multiplex_active(False)
|
||||
yield
|
||||
ss.set_multiplex_active(False)
|
||||
|
||||
|
||||
class TestRuntimeProviderUsesScope:
|
||||
"""hermes_cli.runtime_provider._getenv resolves through the secret scope."""
|
||||
|
||||
def test_getenv_reads_scope_under_multiplex(self, monkeypatch):
|
||||
from hermes_cli.runtime_provider import _getenv
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-global-leak")
|
||||
ss.set_multiplex_active(True)
|
||||
tok = ss.set_secret_scope({"ANTHROPIC_API_KEY": "sk-profileA"})
|
||||
try:
|
||||
assert _getenv("ANTHROPIC_API_KEY") == "sk-profileA"
|
||||
finally:
|
||||
ss.reset_secret_scope(tok)
|
||||
|
||||
def test_getenv_two_profiles_isolated(self, monkeypatch):
|
||||
from hermes_cli.runtime_provider import _getenv
|
||||
ss.set_multiplex_active(True)
|
||||
|
||||
tok_a = ss.set_secret_scope({"OPENAI_API_KEY": "sk-A"})
|
||||
try:
|
||||
assert _getenv("OPENAI_API_KEY") == "sk-A"
|
||||
finally:
|
||||
ss.reset_secret_scope(tok_a)
|
||||
|
||||
tok_b = ss.set_secret_scope({"OPENAI_API_KEY": "sk-B"})
|
||||
try:
|
||||
assert _getenv("OPENAI_API_KEY") == "sk-B"
|
||||
finally:
|
||||
ss.reset_secret_scope(tok_b)
|
||||
|
||||
def test_getenv_fails_closed_unscoped(self, monkeypatch):
|
||||
from hermes_cli.runtime_provider import _getenv
|
||||
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-leak")
|
||||
ss.set_multiplex_active(True)
|
||||
with pytest.raises(ss.UnscopedSecretError):
|
||||
_getenv("OPENROUTER_API_KEY")
|
||||
|
||||
def test_getenv_global_var_still_reads_environ(self, monkeypatch):
|
||||
from hermes_cli.runtime_provider import _getenv
|
||||
monkeypatch.setenv("HERMES_MAX_ITERATIONS", "42")
|
||||
ss.set_multiplex_active(True)
|
||||
# global var: no scope needed, no raise
|
||||
assert _getenv("HERMES_MAX_ITERATIONS") == "42"
|
||||
|
||||
|
||||
class TestMcpInterpolationUsesScope:
|
||||
"""MCP config ${VAR} interpolation resolves through the secret scope."""
|
||||
|
||||
def test_interpolation_reads_scope(self, monkeypatch):
|
||||
from tools.mcp_tool import _interpolate_env_vars
|
||||
monkeypatch.setenv("MY_MCP_TOKEN", "global-token")
|
||||
ss.set_multiplex_active(True)
|
||||
tok = ss.set_secret_scope({"MY_MCP_TOKEN": "profile-token"})
|
||||
try:
|
||||
cfg = {"env": {"TOKEN": "${MY_MCP_TOKEN}"}}
|
||||
assert _interpolate_env_vars(cfg) == {"env": {"TOKEN": "profile-token"}}
|
||||
finally:
|
||||
ss.reset_secret_scope(tok)
|
||||
|
||||
def test_interpolation_unset_keeps_placeholder(self, monkeypatch):
|
||||
from tools.mcp_tool import _interpolate_env_vars
|
||||
monkeypatch.delenv("UNSET_MCP_VAR", raising=False)
|
||||
# multiplex off: unset var keeps literal placeholder (legacy behavior)
|
||||
assert _interpolate_env_vars("${UNSET_MCP_VAR}") == "${UNSET_MCP_VAR}"
|
||||
|
||||
def test_interpolation_off_reads_environ(self, monkeypatch):
|
||||
from tools.mcp_tool import _interpolate_env_vars
|
||||
monkeypatch.setenv("MY_MCP_TOKEN", "env-token")
|
||||
# multiplex off: legacy os.environ resolution
|
||||
assert _interpolate_env_vars("${MY_MCP_TOKEN}") == "env-token"
|
||||
73
tests/gateway/test_multiplex_http_routing.py
Normal file
73
tests/gateway/test_multiplex_http_routing.py
Normal file
|
|
@ -0,0 +1,73 @@
|
|||
"""Phase 1: HTTP-inbound /p/<profile>/ routing for the webhook adapter."""
|
||||
import pytest
|
||||
|
||||
from gateway.config import GatewayConfig, Platform
|
||||
from gateway.session import SessionSource, build_session_key
|
||||
|
||||
|
||||
class TestSessionSourceProfileField:
|
||||
def test_profile_roundtrips(self):
|
||||
s = SessionSource(
|
||||
platform=Platform.WEBHOOK if hasattr(Platform, "WEBHOOK") else Platform.TELEGRAM,
|
||||
chat_id="c1",
|
||||
chat_type="webhook",
|
||||
profile="coder",
|
||||
)
|
||||
restored = SessionSource.from_dict(s.to_dict())
|
||||
assert restored.profile == "coder"
|
||||
|
||||
def test_profile_absent_not_serialized(self):
|
||||
s = SessionSource(platform=Platform.TELEGRAM, chat_id="c1", chat_type="dm")
|
||||
assert "profile" not in s.to_dict()
|
||||
|
||||
def test_source_profile_drives_session_key_namespace(self):
|
||||
s = SessionSource(platform=Platform.TELEGRAM, chat_id="99", chat_type="dm")
|
||||
# build_session_key takes profile explicitly; the adapter passes
|
||||
# source.profile through. Verify the namespace follows it.
|
||||
assert build_session_key(s, profile="coder") == "agent:coder:telegram:dm:99"
|
||||
|
||||
|
||||
class TestWebhookProfileResolution:
|
||||
"""_resolve_request_profile validates the /p/<profile>/ prefix."""
|
||||
|
||||
def _adapter(self, multiplex: bool, served=("default", "coder")):
|
||||
from gateway.platforms.webhook import WebhookAdapter, _PROFILE_REJECTED
|
||||
|
||||
class _FakeReq:
|
||||
def __init__(self, profile):
|
||||
self.match_info = {"profile": profile} if profile is not None else {}
|
||||
|
||||
cfg = GatewayConfig(multiplex_profiles=multiplex)
|
||||
|
||||
class _Runner:
|
||||
config = cfg
|
||||
|
||||
# Construct minimally; we only call _resolve_request_profile.
|
||||
adapter = WebhookAdapter.__new__(WebhookAdapter)
|
||||
adapter.gateway_runner = _Runner()
|
||||
return adapter, _FakeReq, _PROFILE_REJECTED, served
|
||||
|
||||
def test_no_prefix_returns_none(self):
|
||||
adapter, Req, _REJ, _ = self._adapter(multiplex=True)
|
||||
assert adapter._resolve_request_profile(Req(None)) is None
|
||||
|
||||
def test_prefix_ignored_when_multiplex_off(self):
|
||||
adapter, Req, _REJ, _ = self._adapter(multiplex=False)
|
||||
# Even a bogus profile is ignored (not 404'd) when multiplexing is off.
|
||||
assert adapter._resolve_request_profile(Req("anything")) is None
|
||||
|
||||
def test_known_profile_accepted(self, monkeypatch):
|
||||
adapter, Req, _REJ, served = self._adapter(multiplex=True)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.profiles.profiles_to_serve",
|
||||
lambda multiplex: [(n, None) for n in served],
|
||||
)
|
||||
assert adapter._resolve_request_profile(Req("coder")) == "coder"
|
||||
|
||||
def test_unknown_profile_rejected(self, monkeypatch):
|
||||
adapter, Req, REJ, served = self._adapter(multiplex=True)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.profiles.profiles_to_serve",
|
||||
lambda multiplex: [(n, None) for n in served],
|
||||
)
|
||||
assert adapter._resolve_request_profile(Req("ghost")) is REJ
|
||||
55
tests/gateway/test_multiplex_lifecycle.py
Normal file
55
tests/gateway/test_multiplex_lifecycle.py
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
"""Phase 4: lifecycle guard + per-profile observability."""
|
||||
import pytest
|
||||
|
||||
|
||||
class TestServedProfilesStatus:
|
||||
def test_write_and_read_served_profiles(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
import importlib
|
||||
import gateway.status as status
|
||||
importlib.reload(status)
|
||||
try:
|
||||
status.write_runtime_status(
|
||||
gateway_state="running", served_profiles=["default", "coder"]
|
||||
)
|
||||
rec = status.read_runtime_status()
|
||||
assert rec.get("served_profiles") == ["default", "coder"]
|
||||
finally:
|
||||
importlib.reload(status)
|
||||
|
||||
def test_served_profiles_absent_by_default(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
import importlib
|
||||
import gateway.status as status
|
||||
importlib.reload(status)
|
||||
try:
|
||||
status.write_runtime_status(gateway_state="running")
|
||||
rec = status.read_runtime_status()
|
||||
assert "served_profiles" not in rec
|
||||
finally:
|
||||
importlib.reload(status)
|
||||
|
||||
|
||||
class TestNamedProfileMultiplexerGuard:
|
||||
"""_guard_named_profile_under_multiplexer is inert unless all conditions hold."""
|
||||
|
||||
def test_inert_for_default_profile(self, monkeypatch):
|
||||
from hermes_cli import gateway as gw
|
||||
monkeypatch.setattr(gw, "_profile_suffix", lambda: "")
|
||||
# Should return without raising (default profile => guard N/A).
|
||||
gw._guard_named_profile_under_multiplexer(force=False)
|
||||
|
||||
def test_force_bypasses(self, monkeypatch):
|
||||
from hermes_cli import gateway as gw
|
||||
# Even if it looks like a named profile, force returns immediately.
|
||||
monkeypatch.setattr(gw, "_profile_suffix", lambda: "coder")
|
||||
gw._guard_named_profile_under_multiplexer(force=True)
|
||||
|
||||
def test_inert_when_no_default_gateway_running(self, monkeypatch, tmp_path):
|
||||
from hermes_cli import gateway as gw
|
||||
monkeypatch.setattr(gw, "_profile_suffix", lambda: "coder")
|
||||
monkeypatch.setattr(
|
||||
"hermes_constants.get_default_hermes_root", lambda: tmp_path
|
||||
)
|
||||
# No gateway.pid in tmp_path => no running default gateway => no raise.
|
||||
gw._guard_named_profile_under_multiplexer(force=False)
|
||||
165
tests/gateway/test_multiplex_phase0.py
Normal file
165
tests/gateway/test_multiplex_phase0.py
Normal file
|
|
@ -0,0 +1,165 @@
|
|||
"""Phase 0 foundations for multi-profile gateway multiplexing.
|
||||
|
||||
Covers the three Phase 0 deliverables:
|
||||
1. ``gateway.multiplex_profiles`` config flag (default False, round-trips).
|
||||
2. ``hermes_cli.profiles.profiles_to_serve`` enumeration.
|
||||
3. Profile-stamped ``build_session_key`` that is BYTE-IDENTICAL when the
|
||||
flag is off (the orphan-every-session guard) and namespace-segmented when
|
||||
on, without disturbing the positional key layout downstream parsers rely
|
||||
on.
|
||||
"""
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
|
||||
from gateway.config import GatewayConfig, Platform
|
||||
from gateway.session import SessionSource, SessionStore, build_session_key
|
||||
|
||||
|
||||
def _src(**kw) -> SessionSource:
|
||||
kw.setdefault("platform", Platform.TELEGRAM)
|
||||
kw.setdefault("chat_id", "99")
|
||||
kw.setdefault("chat_type", "dm")
|
||||
return SessionSource(**kw)
|
||||
|
||||
|
||||
class TestSessionKeyByteIdenticalWhenOff:
|
||||
"""The non-negotiable guard: with no profile (or 'default'), every key is
|
||||
byte-for-byte what it was before Phase 0. A diff here orphans every
|
||||
existing session on upgrade."""
|
||||
|
||||
@pytest.mark.parametrize("profile", [None, "default"])
|
||||
def test_dm_with_chat_id(self, profile):
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
assert build_session_key(s, profile=profile) == "agent:main:telegram:dm:99"
|
||||
|
||||
@pytest.mark.parametrize("profile", [None, "default"])
|
||||
def test_dm_with_thread(self, profile):
|
||||
s = _src(chat_id="99", chat_type="dm", thread_id="t1")
|
||||
assert build_session_key(s, profile=profile) == "agent:main:telegram:dm:99:t1"
|
||||
|
||||
@pytest.mark.parametrize("profile", [None, "default"])
|
||||
def test_dm_without_chat_id_falls_back_to_user(self, profile):
|
||||
s = _src(chat_id="", chat_type="dm", user_id="jordan")
|
||||
assert build_session_key(s, profile=profile) == "agent:main:telegram:dm:jordan"
|
||||
|
||||
@pytest.mark.parametrize("profile", [None, "default"])
|
||||
def test_group_per_user(self, profile):
|
||||
s = _src(platform=Platform.DISCORD, chat_id="g1", chat_type="group", user_id="alice")
|
||||
assert (
|
||||
build_session_key(s, profile=profile)
|
||||
== "agent:main:discord:group:g1:alice"
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("profile", [None, "default"])
|
||||
def test_group_shared_when_disabled(self, profile):
|
||||
s = _src(platform=Platform.DISCORD, chat_id="g1", chat_type="group", user_id="alice")
|
||||
assert (
|
||||
build_session_key(s, group_sessions_per_user=False, profile=profile)
|
||||
== "agent:main:discord:group:g1"
|
||||
)
|
||||
|
||||
|
||||
class TestSessionKeyNamespacedWhenOn:
|
||||
"""A named profile occupies the namespace slot, isolating its sessions."""
|
||||
|
||||
def test_named_profile_dm(self):
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
assert build_session_key(s, profile="coder") == "agent:coder:telegram:dm:99"
|
||||
|
||||
def test_named_profile_group_per_user(self):
|
||||
s = _src(platform=Platform.DISCORD, chat_id="g1", chat_type="group", user_id="alice")
|
||||
assert (
|
||||
build_session_key(s, profile="coder")
|
||||
== "agent:coder:discord:group:g1:alice"
|
||||
)
|
||||
|
||||
def test_two_profiles_same_chat_do_not_collide(self):
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
a = build_session_key(s, profile="default")
|
||||
b = build_session_key(s, profile="coder")
|
||||
c = build_session_key(s, profile="writer")
|
||||
assert a != b != c and a != c
|
||||
|
||||
def test_positional_layout_preserved_for_parsers(self):
|
||||
"""Downstream parsers split on ':' and read parts[2]=platform,
|
||||
parts[3]=chat_type, parts[4]=chat_id (see qqbot adapter
|
||||
_parse_gateway_session_key). The profile must occupy parts[1] only."""
|
||||
s = _src(platform=Platform.DISCORD, chat_id="g1", chat_type="group", user_id="alice")
|
||||
parts = build_session_key(s, profile="coder").split(":")
|
||||
assert parts[0] == "agent"
|
||||
assert parts[1] == "coder" # namespace slot (was always 'main')
|
||||
assert parts[2] == "discord" # platform — unchanged offset
|
||||
assert parts[3] == "group" # chat_type — unchanged offset
|
||||
assert parts[4] == "g1" # chat_id — unchanged offset
|
||||
|
||||
def test_default_namespace_layout_matches_named(self):
|
||||
"""Default and named keys differ ONLY in parts[1]."""
|
||||
s = _src(platform=Platform.SLACK, chat_id="c1", chat_type="channel", user_id="u1")
|
||||
d = build_session_key(s, profile="default").split(":")
|
||||
n = build_session_key(s, profile="coder").split(":")
|
||||
assert d[0] == n[0] == "agent"
|
||||
assert d[1] == "main" and n[1] == "coder"
|
||||
assert d[2:] == n[2:] # everything after the namespace is identical
|
||||
|
||||
|
||||
class TestMultiplexConfigFlag:
|
||||
"""gateway.multiplex_profiles defaults off and round-trips."""
|
||||
|
||||
def test_default_is_false(self):
|
||||
assert GatewayConfig().multiplex_profiles is False
|
||||
|
||||
def test_to_dict_includes_flag(self):
|
||||
assert GatewayConfig().to_dict()["multiplex_profiles"] is False
|
||||
|
||||
def test_from_dict_top_level(self):
|
||||
cfg = GatewayConfig.from_dict({"multiplex_profiles": True})
|
||||
assert cfg.multiplex_profiles is True
|
||||
|
||||
def test_from_dict_nested_gateway(self):
|
||||
cfg = GatewayConfig.from_dict({"gateway": {"multiplex_profiles": True}})
|
||||
assert cfg.multiplex_profiles is True
|
||||
|
||||
def test_from_dict_coerces_truthy_string(self):
|
||||
cfg = GatewayConfig.from_dict({"multiplex_profiles": "true"})
|
||||
assert cfg.multiplex_profiles is True
|
||||
|
||||
def test_roundtrip(self):
|
||||
cfg = GatewayConfig.from_dict(GatewayConfig(multiplex_profiles=True).to_dict())
|
||||
assert cfg.multiplex_profiles is True
|
||||
|
||||
|
||||
class TestSessionStoreProfileResolution:
|
||||
"""SessionStore._generate_session_key honors the flag: legacy namespace
|
||||
when off, active-profile namespace when on."""
|
||||
|
||||
def _store(self, tmp_path, **cfg_kw):
|
||||
config = GatewayConfig(**cfg_kw)
|
||||
with patch("gateway.session.SessionStore._ensure_loaded"):
|
||||
s = SessionStore(sessions_dir=tmp_path, config=config)
|
||||
s._db = None
|
||||
s._loaded = True
|
||||
return s
|
||||
|
||||
def test_flag_off_uses_legacy_namespace(self, tmp_path):
|
||||
store = self._store(tmp_path) # multiplex_profiles defaults False
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
assert store._generate_session_key(s) == "agent:main:telegram:dm:99"
|
||||
assert store._generate_session_key(s) == build_session_key(s)
|
||||
|
||||
def test_flag_off_resolve_profile_is_none(self, tmp_path):
|
||||
store = self._store(tmp_path)
|
||||
assert store._resolve_profile_for_key() is None
|
||||
|
||||
def test_flag_on_uses_active_profile_namespace(self, tmp_path):
|
||||
store = self._store(tmp_path, multiplex_profiles=True)
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
with patch("hermes_cli.profiles.get_active_profile_name", return_value="coder"):
|
||||
assert store._generate_session_key(s) == "agent:coder:telegram:dm:99"
|
||||
|
||||
def test_flag_on_default_profile_stays_legacy(self, tmp_path):
|
||||
store = self._store(tmp_path, multiplex_profiles=True)
|
||||
s = _src(chat_id="99", chat_type="dm")
|
||||
with patch("hermes_cli.profiles.get_active_profile_name", return_value="default"):
|
||||
assert store._generate_session_key(s) == "agent:main:telegram:dm:99"
|
||||
|
||||
|
||||
|
|
@ -51,3 +51,18 @@ def test_reload_runtime_env_keeps_env_max_iterations_when_config_omits_key(
|
|||
gateway_run._reload_runtime_env_preserving_config_authority()
|
||||
|
||||
assert os.environ["HERMES_MAX_ITERATIONS"] == "123"
|
||||
|
||||
|
||||
def test_current_max_iterations_reloads_before_reading(monkeypatch) -> None:
|
||||
monkeypatch.setenv("HERMES_MAX_ITERATIONS", "90")
|
||||
|
||||
def _fake_reload() -> None:
|
||||
os.environ["HERMES_MAX_ITERATIONS"] = "200"
|
||||
|
||||
monkeypatch.setattr(
|
||||
gateway_run,
|
||||
"_reload_runtime_env_preserving_config_authority",
|
||||
_fake_reload,
|
||||
)
|
||||
|
||||
assert gateway_run._current_max_iterations() == 200
|
||||
|
|
|
|||
|
|
@ -153,6 +153,39 @@ class TestShouldExclude:
|
|||
assert not _should_exclude(Path("skills/autonomous-ai-agents/hermes-agent/SKILL.md"))
|
||||
assert not _should_exclude(Path("skills/autonomous-ai-agents/hermes-agent/sub/item.txt"))
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"rel",
|
||||
[
|
||||
"plugins/my-plugin/.venv/lib/python3.12/site-packages/x/__init__.py",
|
||||
"plugins/my-plugin/venv/bin/python",
|
||||
"mcp/server/site-packages/pkg/mod.py",
|
||||
".cache/uv/wheels/abc.whl",
|
||||
"plugins/p/.cache/pip/http/deadbeef",
|
||||
".tox/py312/log.txt",
|
||||
".nox/tests/bin/pytest",
|
||||
"plugins/p/.pytest_cache/v/cache/lastfailed",
|
||||
".mypy_cache/3.12/agent.meta.json",
|
||||
".ruff_cache/0.4.0/abc",
|
||||
],
|
||||
)
|
||||
def test_excludes_regeneratable_dependency_and_cache_dirs(self, rel):
|
||||
"""Python dep trees and tool caches under HERMES_HOME must be skipped —
|
||||
these are what balloon a backup to hundreds of thousands of files."""
|
||||
from hermes_cli.backup import _should_exclude
|
||||
assert _should_exclude(Path(rel))
|
||||
|
||||
def test_does_not_exclude_curator_archive(self):
|
||||
"""skills/.archive/ holds restorable archived skills and MUST survive
|
||||
a backup — it is intentionally NOT in the exclusion set."""
|
||||
from hermes_cli.backup import _should_exclude
|
||||
assert not _should_exclude(Path("skills/.archive/old-skill/SKILL.md"))
|
||||
|
||||
def test_does_not_exclude_legit_files_resembling_cache_names(self):
|
||||
"""Only directory-component matches are excluded; a normal file is kept."""
|
||||
from hermes_cli.backup import _should_exclude
|
||||
assert not _should_exclude(Path("skills/my-skill/venv-notes.md"))
|
||||
assert not _should_exclude(Path("memories/cache.json"))
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Backup tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -272,6 +305,37 @@ class TestBackup:
|
|||
agent_files = [n for n in names if "hermes-agent" in n]
|
||||
assert agent_files == [], f"hermes-agent files leaked into backup: {agent_files}"
|
||||
|
||||
def test_excludes_dependency_and_cache_trees(self, tmp_path, monkeypatch):
|
||||
"""A plugin venv / site-packages / pip cache under HERMES_HOME must be
|
||||
pruned by the walk, while real data (skills, config) is preserved.
|
||||
This is the regression guard for the ballooning-backup bug."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
_make_hermes_tree(hermes_home)
|
||||
|
||||
# Simulate the heavy regeneratable trees that ballooned the backup.
|
||||
venv_pkg = hermes_home / "plugins" / "heavy" / ".venv" / "lib" / "site-packages" / "dep"
|
||||
venv_pkg.mkdir(parents=True)
|
||||
(venv_pkg / "__init__.py").write_text("# dep\n")
|
||||
pip_cache = hermes_home / ".cache" / "uv" / "wheels"
|
||||
pip_cache.mkdir(parents=True)
|
||||
(pip_cache / "abc.whl").write_bytes(b"\x00")
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
|
||||
out_zip = tmp_path / "backup.zip"
|
||||
from hermes_cli.backup import run_backup
|
||||
run_backup(Namespace(output=str(out_zip)))
|
||||
|
||||
with zipfile.ZipFile(out_zip, "r") as zf:
|
||||
names = zf.namelist()
|
||||
leaked = [n for n in names if ".venv" in n or "site-packages" in n or ".cache" in n]
|
||||
assert leaked == [], f"regeneratable trees leaked into backup: {leaked}"
|
||||
# Real data still present.
|
||||
assert "skills/my-skill/SKILL.md" in names
|
||||
assert "config.yaml" in names
|
||||
|
||||
def test_includes_nested_hermes_agent_in_skills(self, tmp_path, monkeypatch):
|
||||
"""Backup includes skills/.../hermes-agent/ but NOT root hermes-agent/."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
|
|
|
|||
|
|
@ -955,6 +955,17 @@ class TestInterimAssistantMessageConfig:
|
|||
assert raw["display"]["interim_assistant_messages"] is True
|
||||
|
||||
|
||||
class TestCliRefreshIntervalConfig:
|
||||
"""Test the CLI refresh_interval config default (#45592 / #48309)."""
|
||||
|
||||
def test_default_config_enables_cli_refresh_interval(self):
|
||||
"""cli_refresh_interval defaults to 1.0 so the idle status-bar
|
||||
clock keeps ticking and the bottom chrome stays alive during
|
||||
idle (#45592). Users on emulators where the periodic redraw
|
||||
fights auto-scroll can set it to 0 (#48309)."""
|
||||
assert DEFAULT_CONFIG["display"]["cli_refresh_interval"] == 1.0
|
||||
|
||||
|
||||
class TestDiscordChannelPromptsConfig:
|
||||
def test_default_config_includes_discord_channel_prompts(self):
|
||||
assert DEFAULT_CONFIG["discord"]["channel_prompts"] == {}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,9 @@ def hermes_home(tmp_path, monkeypatch):
|
|||
(logs_dir / "gateway.log").write_text(
|
||||
"2026-04-12 17:00:10 INFO gateway.run: started\n"
|
||||
)
|
||||
(logs_dir / "gui.log").write_text(
|
||||
"2026-04-12 17:00:12 INFO hermes_cli.web_server: dashboard request\n"
|
||||
)
|
||||
(logs_dir / "desktop.log").write_text(
|
||||
"2026-04-12 17:00:15 INFO desktop: backend spawned\n"
|
||||
)
|
||||
|
|
@ -454,6 +457,15 @@ class TestCollectDebugReport:
|
|||
|
||||
assert "--- gateway.log" in report
|
||||
|
||||
def test_report_includes_gui_log(self, hermes_home):
|
||||
from hermes_cli.debug import collect_debug_report
|
||||
|
||||
with patch("hermes_cli.dump.run_dump"):
|
||||
report = collect_debug_report(log_lines=50)
|
||||
|
||||
assert "--- gui.log" in report
|
||||
assert "dashboard request" in report
|
||||
|
||||
def test_report_includes_desktop_log(self, hermes_home):
|
||||
from hermes_cli.debug import collect_debug_report
|
||||
|
||||
|
|
@ -538,8 +550,8 @@ class TestRunDebugShare:
|
|||
assert "FULL agent.log" in out
|
||||
assert "FULL gateway.log" in out
|
||||
|
||||
def test_share_uploads_four_pastes(self, hermes_home, capsys):
|
||||
"""Successful share uploads report + agent.log + gateway.log + desktop.log."""
|
||||
def test_share_uploads_five_pastes(self, hermes_home, capsys):
|
||||
"""Successful share uploads report + agent.log + gateway.log + gui.log + desktop.log."""
|
||||
from hermes_cli.debug import run_debug_share
|
||||
|
||||
args = MagicMock()
|
||||
|
|
@ -561,15 +573,17 @@ class TestRunDebugShare:
|
|||
run_debug_share(args)
|
||||
|
||||
out = capsys.readouterr().out
|
||||
# Should have 4 uploads: report, agent.log, gateway.log, desktop.log
|
||||
assert call_count[0] == 4
|
||||
# Should have 5 uploads: report, agent.log, gateway.log, gui.log, desktop.log
|
||||
assert call_count[0] == 5
|
||||
assert "paste.rs/paste1" in out # Report
|
||||
assert "paste.rs/paste2" in out # agent.log
|
||||
assert "paste.rs/paste3" in out # gateway.log
|
||||
assert "paste.rs/paste4" in out # desktop.log
|
||||
assert "paste.rs/paste4" in out # gui.log
|
||||
assert "paste.rs/paste5" in out # desktop.log
|
||||
assert "Report" in out
|
||||
assert "agent.log" in out
|
||||
assert "gateway.log" in out
|
||||
assert "gui.log" in out
|
||||
assert "desktop.log" in out
|
||||
|
||||
# Each log paste should start with the dump header
|
||||
|
|
@ -579,7 +593,10 @@ class TestRunDebugShare:
|
|||
gateway_paste = uploaded_content[2]
|
||||
assert "--- hermes dump ---" in gateway_paste
|
||||
assert "--- full gateway.log ---" in gateway_paste
|
||||
desktop_paste = uploaded_content[3]
|
||||
gui_paste = uploaded_content[3]
|
||||
assert "--- hermes dump ---" in gui_paste
|
||||
assert "--- full gui.log ---" in gui_paste
|
||||
desktop_paste = uploaded_content[4]
|
||||
assert "--- hermes dump ---" in desktop_paste
|
||||
assert "--- full desktop.log ---" in desktop_paste
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ Covers:
|
|||
- _contains_gateway_lifecycle_command pattern matching
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
from argparse import Namespace
|
||||
|
||||
|
|
@ -250,3 +251,109 @@ class TestGatewaySelfTargetingGuard:
|
|||
args = Namespace(gateway_command="restart", all=False, system=False)
|
||||
with pytest.raises(_Reached):
|
||||
gw.gateway_command(args)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defense 3: terminal_tool hard-blocks gateway lifecycle commands inside gateway
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestTerminalToolGatewayLifecycleGuard:
|
||||
"""terminal_tool must refuse gateway lifecycle commands when _HERMES_GATEWAY=1.
|
||||
|
||||
Issue #37453: systemctl --user restart hermes-gateway runs as a child of the
|
||||
gateway process. When systemd delivers SIGTERM the gateway kills its own
|
||||
restart command mid-execution — the service may never restart. The guard
|
||||
must fire before execution, unconditionally (force=True cannot bypass it).
|
||||
"""
|
||||
|
||||
def _make_fake_env(self):
|
||||
class _FakeEnv:
|
||||
env = {}
|
||||
def execute(self, command, **kwargs): # pragma: no cover
|
||||
raise AssertionError("execute must not be reached")
|
||||
return _FakeEnv()
|
||||
|
||||
def _minimal_config(self):
|
||||
return {"env_type": "local", "cwd": "/tmp", "timeout": 60, "lifetime_seconds": 3600}
|
||||
|
||||
def _patch_env(self, monkeypatch, fake_env, *, inside_gateway: bool):
|
||||
import tools.terminal_tool as tt
|
||||
eid = "default"
|
||||
monkeypatch.setattr(tt, "_active_environments", {eid: fake_env})
|
||||
monkeypatch.setattr(tt, "_last_activity", {eid: 0.0})
|
||||
monkeypatch.setattr(tt, "_task_env_overrides", {})
|
||||
monkeypatch.setattr(tt, "_get_env_config", self._minimal_config)
|
||||
if inside_gateway:
|
||||
monkeypatch.setenv("_HERMES_GATEWAY", "1")
|
||||
else:
|
||||
monkeypatch.delenv("_HERMES_GATEWAY", raising=False)
|
||||
|
||||
@pytest.mark.parametrize("cmd", [
|
||||
"systemctl restart hermes-gateway",
|
||||
"systemctl --user restart hermes-gateway",
|
||||
"systemctl stop hermes-gateway.service",
|
||||
"hermes gateway restart",
|
||||
"launchctl kickstart gui/501/ai.hermes.gateway",
|
||||
"pkill -f hermes.*gateway",
|
||||
])
|
||||
def test_blocks_lifecycle_commands_inside_gateway(self, monkeypatch, cmd):
|
||||
import tools.terminal_tool as tt
|
||||
self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True)
|
||||
|
||||
result = json.loads(tt.terminal_tool(command=cmd))
|
||||
|
||||
assert result["exit_code"] == 1
|
||||
assert "Blocked" in result["error"]
|
||||
|
||||
def test_force_true_cannot_bypass_block(self, monkeypatch):
|
||||
import tools.terminal_tool as tt
|
||||
self._patch_env(monkeypatch, self._make_fake_env(), inside_gateway=True)
|
||||
|
||||
result = json.loads(tt.terminal_tool(
|
||||
command="systemctl restart hermes-gateway", force=True
|
||||
))
|
||||
|
||||
assert result["exit_code"] == 1
|
||||
assert "Blocked" in result["error"]
|
||||
|
||||
def test_safe_systemctl_commands_pass_through(self, monkeypatch):
|
||||
"""Non-hermes systemctl commands must not be blocked by this guard."""
|
||||
import tools.terminal_tool as tt
|
||||
|
||||
calls = []
|
||||
|
||||
class _FakeEnv:
|
||||
env = {}
|
||||
def execute(self, command, **kwargs):
|
||||
calls.append(command)
|
||||
return {"output": "Active: running", "returncode": 0}
|
||||
|
||||
self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=True)
|
||||
monkeypatch.setattr(tt, "_check_all_guards", lambda cmd, env: {"approved": True})
|
||||
|
||||
result = json.loads(tt.terminal_tool(command="systemctl status nginx"))
|
||||
|
||||
assert result["exit_code"] == 0
|
||||
assert calls == ["systemctl status nginx"]
|
||||
|
||||
def test_guard_inactive_outside_gateway(self, monkeypatch):
|
||||
"""Without _HERMES_GATEWAY=1 the lifecycle guard must not fire."""
|
||||
import tools.terminal_tool as tt
|
||||
|
||||
calls = []
|
||||
|
||||
class _FakeEnv:
|
||||
env = {}
|
||||
def execute(self, command, **kwargs):
|
||||
calls.append(command)
|
||||
return {"output": "restarting...", "returncode": 0}
|
||||
|
||||
self._patch_env(monkeypatch, _FakeEnv(), inside_gateway=False)
|
||||
monkeypatch.setattr(tt, "_check_all_guards", lambda cmd, env: {"approved": True})
|
||||
|
||||
result = json.loads(tt.terminal_tool(command="systemctl restart hermes-gateway"))
|
||||
|
||||
# Outside the gateway the lifecycle guard doesn't block — the normal
|
||||
# approval flow handles it (here mocked as approved).
|
||||
assert result["exit_code"] == 0
|
||||
assert calls == ["systemctl restart hermes-gateway"]
|
||||
|
|
|
|||
|
|
@ -505,6 +505,171 @@ def test_stale_claim_with_live_pid_uses_env_ttl_override(
|
|||
assert task.claim_expires > int(time.time()) + 3000
|
||||
|
||||
|
||||
def test_stale_claim_deferred_when_live_worker_survives_termination(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""A TTL-expired claim whose worker survives the kill must NOT be released.
|
||||
|
||||
Releasing would let the dispatcher spawn a duplicate beside the still-alive
|
||||
worker — the runaway seen when a cgroup memory.high throttle parks a worker
|
||||
in uninterruptible (D) state, where a pending SIGKILL cannot land. The claim
|
||||
is held (extended) and retried next tick instead.
|
||||
"""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
with kb.connect() as conn:
|
||||
t = kb.create_task(conn, title="x", assignee="a")
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
kb.claim_task(conn, t, claimer=f"{host}:worker")
|
||||
kb._set_worker_pid(conn, t, 12345)
|
||||
|
||||
old_expires = int(time.time()) - 60
|
||||
# Heartbeat stale by > 1h so the live-pid EXTEND branch is skipped and
|
||||
# the terminate path (the wedged-worker case) runs.
|
||||
conn.execute(
|
||||
"UPDATE tasks SET claim_expires = ?, last_heartbeat_at = ? "
|
||||
"WHERE id = ?",
|
||||
(old_expires, int(time.time()) - 7200, t),
|
||||
)
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: True)
|
||||
monkeypatch.setattr(
|
||||
_kb, "_terminate_reclaimed_worker",
|
||||
lambda *a, **k: {
|
||||
"termination_attempted": True,
|
||||
"host_local": True,
|
||||
"terminated": False,
|
||||
},
|
||||
)
|
||||
reclaimed = kb.release_stale_claims(conn, signal_fn=lambda _p, _s: None)
|
||||
assert reclaimed == 0
|
||||
|
||||
assert kb.get_task(conn, t).status == "running"
|
||||
worker_pid = conn.execute(
|
||||
"SELECT worker_pid FROM tasks WHERE id = ?", (t,),
|
||||
).fetchone()[0]
|
||||
assert worker_pid == 12345 # worker not orphaned
|
||||
claim_expires = conn.execute(
|
||||
"SELECT claim_expires FROM tasks WHERE id = ?", (t,),
|
||||
).fetchone()[0]
|
||||
assert claim_expires > old_expires # claim held, not released
|
||||
|
||||
kinds = [
|
||||
r["kind"] for r in conn.execute(
|
||||
"SELECT kind FROM task_events WHERE task_id = ?", (t,),
|
||||
).fetchall()
|
||||
]
|
||||
assert "reclaim_deferred" in kinds
|
||||
assert "reclaimed" not in kinds
|
||||
|
||||
|
||||
def test_stale_claim_reclaimed_when_termination_succeeds(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""When the worker is actually killed, the claim is released as before."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
with kb.connect() as conn:
|
||||
t = kb.create_task(conn, title="x", assignee="a")
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
kb.claim_task(conn, t, claimer=f"{host}:worker")
|
||||
kb._set_worker_pid(conn, t, 12345)
|
||||
conn.execute(
|
||||
"UPDATE tasks SET claim_expires = ?, last_heartbeat_at = ? "
|
||||
"WHERE id = ?",
|
||||
(int(time.time()) - 60, int(time.time()) - 7200, t),
|
||||
)
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: False)
|
||||
monkeypatch.setattr(
|
||||
_kb, "_terminate_reclaimed_worker",
|
||||
lambda *a, **k: {
|
||||
"termination_attempted": True,
|
||||
"host_local": True,
|
||||
"terminated": True,
|
||||
},
|
||||
)
|
||||
reclaimed = kb.release_stale_claims(conn, signal_fn=lambda _p, _s: None)
|
||||
assert reclaimed == 1
|
||||
assert kb.get_task(conn, t).status == "ready"
|
||||
|
||||
|
||||
def test_stale_claim_released_when_worker_not_host_local(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""The defer guard only holds OUR own surviving workers.
|
||||
|
||||
A claim we cannot manage (different host, or no kill attempted) must still
|
||||
be released, otherwise a foreign-host claim could strand a task forever.
|
||||
"""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
with kb.connect() as conn:
|
||||
t = kb.create_task(conn, title="x", assignee="a")
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
kb.claim_task(conn, t, claimer=f"{host}:worker")
|
||||
kb._set_worker_pid(conn, t, 12345)
|
||||
conn.execute(
|
||||
"UPDATE tasks SET claim_expires = ?, last_heartbeat_at = ? "
|
||||
"WHERE id = ?",
|
||||
(int(time.time()) - 60, int(time.time()) - 7200, t),
|
||||
)
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: True)
|
||||
monkeypatch.setattr(
|
||||
_kb, "_terminate_reclaimed_worker",
|
||||
lambda *a, **k: {
|
||||
"termination_attempted": False,
|
||||
"host_local": False,
|
||||
"terminated": False,
|
||||
},
|
||||
)
|
||||
reclaimed = kb.release_stale_claims(conn, signal_fn=lambda _p, _s: None)
|
||||
assert reclaimed == 1
|
||||
assert kb.get_task(conn, t).status == "ready"
|
||||
|
||||
|
||||
def test_detect_stale_defers_when_live_worker_survives(kanban_home, monkeypatch):
|
||||
"""detect_stale_running must also hold the claim when the worker survives."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
with kb.connect() as conn:
|
||||
t = kb.create_task(conn, title="wedged", assignee="worker")
|
||||
kb.claim_task(conn, t)
|
||||
kb._set_worker_pid(conn, t, os.getpid())
|
||||
|
||||
five_hours_ago = int(time.time()) - (5 * 3600)
|
||||
with kb.write_txn(conn):
|
||||
conn.execute(
|
||||
"UPDATE tasks SET started_at = ?, last_heartbeat_at = NULL "
|
||||
"WHERE id = ?",
|
||||
(five_hours_ago, t),
|
||||
)
|
||||
conn.execute(
|
||||
"UPDATE task_runs SET started_at = ? "
|
||||
"WHERE id = (SELECT current_run_id FROM tasks WHERE id = ?)",
|
||||
(five_hours_ago, t),
|
||||
)
|
||||
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: True)
|
||||
monkeypatch.setattr(
|
||||
_kb, "_terminate_reclaimed_worker",
|
||||
lambda *a, **k: {
|
||||
"termination_attempted": True,
|
||||
"host_local": True,
|
||||
"terminated": False,
|
||||
},
|
||||
)
|
||||
stale = kb.detect_stale_running(
|
||||
conn, stale_timeout_seconds=14400, signal_fn=lambda p, s: None,
|
||||
)
|
||||
assert stale == []
|
||||
assert kb.get_task(conn, t).status == "running"
|
||||
kinds = [
|
||||
r["kind"] for r in conn.execute(
|
||||
"SELECT kind FROM task_events WHERE task_id = ?", (t,),
|
||||
).fetchall()
|
||||
]
|
||||
assert "reclaim_deferred" in kinds
|
||||
|
||||
|
||||
def test_stale_claim_reclaim_event_records_diagnostic_payload(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
|
|
|
|||
|
|
@ -55,10 +55,12 @@ def test_prompt_toolkit_model_picker_defers_confirmation_off_key_handler(monkeyp
|
|||
lambda *_args: captured.setdefault("ran_inline", True)
|
||||
)
|
||||
|
||||
_bound(cli_mod.HermesCLI._handle_model_picker_selection, self_)()
|
||||
# The key handler now resolves persistence via resolve_persist_behavior,
|
||||
# which defaults to True (persist-by-default). Simulate that call.
|
||||
_bound(cli_mod.HermesCLI._handle_model_picker_selection, self_)(persist_global=True)
|
||||
|
||||
assert self_._model_picker_state is None
|
||||
assert captured["started"] is True
|
||||
assert captured["daemon"] is True
|
||||
assert captured["args"] == (result, False)
|
||||
assert captured["args"] == (result, True)
|
||||
assert "ran_inline" not in captured
|
||||
|
|
|
|||
122
tests/hermes_cli/test_model_switch_persist_default.py
Normal file
122
tests/hermes_cli/test_model_switch_persist_default.py
Normal file
|
|
@ -0,0 +1,122 @@
|
|||
"""Tests for persist-by-default model switching.
|
||||
|
||||
Covers:
|
||||
- ``parse_model_flags`` recognises ``--session`` (and keeps ``--global``).
|
||||
- ``resolve_persist_behavior`` applies the config-gated default and the
|
||||
``--session`` / ``--global`` overrides.
|
||||
- The default (no flags) persists, which is the user-facing fix: a plain
|
||||
``/model <name>`` survives across sessions.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.model_switch import parse_model_flags, resolve_persist_behavior
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# parse_model_flags
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestParseModelFlagsSession:
|
||||
def test_no_flags(self):
|
||||
assert parse_model_flags("sonnet") == ("sonnet", "", False, False, False)
|
||||
|
||||
def test_global_flag(self):
|
||||
assert parse_model_flags("sonnet --global") == ("sonnet", "", True, False, False)
|
||||
|
||||
def test_session_flag(self):
|
||||
assert parse_model_flags("sonnet --session") == (
|
||||
"sonnet",
|
||||
"",
|
||||
False,
|
||||
False,
|
||||
True,
|
||||
)
|
||||
|
||||
def test_session_with_provider(self):
|
||||
assert parse_model_flags("sonnet --provider anthropic --session") == (
|
||||
"sonnet",
|
||||
"anthropic",
|
||||
False,
|
||||
False,
|
||||
True,
|
||||
)
|
||||
|
||||
def test_refresh_flag_still_parsed(self):
|
||||
assert parse_model_flags("--refresh") == ("", "", False, True, False)
|
||||
|
||||
def test_unicode_dash_session_normalized(self):
|
||||
# Telegram/iOS auto-converts -- to en/em dashes.
|
||||
assert parse_model_flags("sonnet \u2013session") == (
|
||||
"sonnet",
|
||||
"",
|
||||
False,
|
||||
False,
|
||||
True,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# resolve_persist_behavior
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestResolvePersistBehavior:
|
||||
def test_session_flag_always_session_only(self):
|
||||
# --session opts out even if the config default is True.
|
||||
with _config({"model": {"persist_switch_by_default": True}}):
|
||||
assert resolve_persist_behavior(False, True) is False
|
||||
|
||||
def test_global_flag_always_persists(self):
|
||||
# --global forces persist even if the config default is False.
|
||||
with _config({"model": {"persist_switch_by_default": False}}):
|
||||
assert resolve_persist_behavior(True, False) is True
|
||||
|
||||
def test_default_persists_when_config_missing(self):
|
||||
# No model section at all → built-in default (True).
|
||||
with _config({}):
|
||||
assert resolve_persist_behavior(False, False) is True
|
||||
|
||||
def test_default_persists_when_key_true(self):
|
||||
with _config({"model": {"persist_switch_by_default": True}}):
|
||||
assert resolve_persist_behavior(False, False) is True
|
||||
|
||||
def test_default_session_only_when_key_false(self):
|
||||
with _config({"model": {"persist_switch_by_default": False}}):
|
||||
assert resolve_persist_behavior(False, False) is False
|
||||
|
||||
def test_default_when_model_is_flat_string(self):
|
||||
# Fresh install: ``model: ""`` (not a dict) → built-in default True.
|
||||
with _config({"model": ""}):
|
||||
assert resolve_persist_behavior(False, False) is True
|
||||
|
||||
def test_session_overrides_global_when_both_set(self):
|
||||
# --session is the explicit opt-out and wins over --global.
|
||||
with _config({"model": {"persist_switch_by_default": True}}):
|
||||
assert resolve_persist_behavior(True, True) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _config:
|
||||
"""Context manager that patches ``load_config`` to return a fixed dict."""
|
||||
|
||||
def __init__(self, cfg: dict):
|
||||
self.cfg = cfg
|
||||
|
||||
def __enter__(self):
|
||||
self._patch = patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value=self.cfg,
|
||||
)
|
||||
# resolve_persist_behavior imports load_config lazily inside the
|
||||
# function, so patching the source module is sufficient.
|
||||
self._patch.start()
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
self._patch.stop()
|
||||
|
|
@ -35,6 +35,7 @@ from hermes_cli.profiles import (
|
|||
has_bundled_skills_opt_out,
|
||||
NO_BUNDLED_SKILLS_MARKER,
|
||||
backfill_profile_envs,
|
||||
profiles_to_serve,
|
||||
)
|
||||
from hermes_cli.config import DEFAULT_CONFIG
|
||||
|
||||
|
|
@ -1487,3 +1488,48 @@ class TestEdgeCases:
|
|||
delete_profile("coder", yes=True)
|
||||
|
||||
assert get_active_profile() == "default"
|
||||
|
||||
|
||||
class TestProfilesToServe:
|
||||
"""profiles_to_serve(multiplex) — the gateway's profile-enumeration chokepoint."""
|
||||
|
||||
def test_off_returns_only_active_default(self, profile_env):
|
||||
serve = profiles_to_serve(multiplex=False)
|
||||
assert len(serve) == 1
|
||||
name, home = serve[0]
|
||||
assert name == "default"
|
||||
assert home == _get_default_hermes_home()
|
||||
|
||||
def test_off_returns_only_active_named(self, profile_env, monkeypatch):
|
||||
# A named profile's gateway runs with HERMES_HOME pointing at the
|
||||
# profile dir; get_active_profile_name() infers the name from there.
|
||||
create_profile("coder", no_alias=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(get_profile_dir("coder")))
|
||||
serve = profiles_to_serve(multiplex=False)
|
||||
assert len(serve) == 1
|
||||
assert serve[0][0] == "coder"
|
||||
assert serve[0][1] == get_profile_dir("coder")
|
||||
|
||||
def test_on_returns_default_plus_all_named(self, profile_env):
|
||||
create_profile("coder", no_alias=True)
|
||||
create_profile("writer", no_alias=True)
|
||||
serve = dict(profiles_to_serve(multiplex=True))
|
||||
assert set(serve) == {"default", "coder", "writer"}
|
||||
assert serve["default"] == _get_default_hermes_home()
|
||||
assert serve["coder"] == get_profile_dir("coder")
|
||||
|
||||
def test_on_default_always_first(self, profile_env):
|
||||
create_profile("coder", no_alias=True)
|
||||
serve = profiles_to_serve(multiplex=True)
|
||||
assert serve[0][0] == "default"
|
||||
|
||||
def test_on_active_profile_does_not_change_set(self, profile_env):
|
||||
"""Enumeration is independent of which profile is active."""
|
||||
create_profile("coder", no_alias=True)
|
||||
set_active_profile("coder")
|
||||
serve = dict(profiles_to_serve(multiplex=True))
|
||||
assert set(serve) == {"default", "coder"}
|
||||
|
||||
def test_on_no_named_profiles_returns_just_default(self, profile_env):
|
||||
serve = profiles_to_serve(multiplex=True)
|
||||
assert [n for n, _ in serve] == ["default"]
|
||||
|
|
|
|||
127
tests/hermes_cli/test_provider_catalog.py
Normal file
127
tests/hermes_cli/test_provider_catalog.py
Normal file
|
|
@ -0,0 +1,127 @@
|
|||
"""Tests for the unified provider catalog (hermes_cli.provider_catalog).
|
||||
|
||||
These are invariant tests, not snapshots: they assert the parity *contract*
|
||||
between what ``hermes model`` shows (``CANONICAL_PROVIDERS``) and what the
|
||||
catalog exposes, plus how each provider's ``auth_type`` maps to a desktop tab —
|
||||
never a specific provider count or a frozen vendor list (both change over time).
|
||||
"""
|
||||
|
||||
from hermes_cli.models import CANONICAL_PROVIDERS
|
||||
from hermes_cli.provider_catalog import (
|
||||
ProviderDescriptor,
|
||||
provider_catalog,
|
||||
provider_catalog_by_slug,
|
||||
tab_for_auth_type,
|
||||
)
|
||||
|
||||
|
||||
def test_catalog_covers_every_hermes_model_provider():
|
||||
"""PARITY CONTRACT: the catalog == the `hermes model` universe."""
|
||||
slugs = {d.slug for d in provider_catalog()}
|
||||
for entry in CANONICAL_PROVIDERS:
|
||||
assert entry.slug in slugs, (
|
||||
f"{entry.slug} is shown in `hermes model` but missing from provider_catalog()"
|
||||
)
|
||||
|
||||
|
||||
def test_catalog_has_no_providers_outside_hermes_model():
|
||||
"""The catalog must not invent providers `hermes model` doesn't show."""
|
||||
canonical = {e.slug for e in CANONICAL_PROVIDERS}
|
||||
for d in provider_catalog():
|
||||
assert d.slug in canonical, f"{d.slug} in catalog but not in CANONICAL_PROVIDERS"
|
||||
|
||||
|
||||
def test_every_descriptor_lands_on_exactly_one_known_tab():
|
||||
for d in provider_catalog():
|
||||
assert d.tab in {"keys", "accounts"}, f"{d.slug} has bad tab {d.tab!r}"
|
||||
|
||||
|
||||
def test_descriptor_count_matches_canonical():
|
||||
"""One descriptor per canonical entry (no dupes, no drops)."""
|
||||
cat = provider_catalog()
|
||||
assert len(cat) == len(CANONICAL_PROVIDERS)
|
||||
assert len({d.slug for d in cat}) == len(cat)
|
||||
|
||||
|
||||
def test_profileless_providers_still_present():
|
||||
"""Providers without a ProviderProfile must still resolve via fallbacks.
|
||||
|
||||
lmstudio / openai-api / tencent-tokenhub / xai-oauth have no profile on
|
||||
main; they exist only as registry + canonical entries. The catalog must
|
||||
not require a profile to include a provider.
|
||||
"""
|
||||
by = provider_catalog_by_slug()
|
||||
for slug in ("lmstudio", "openai-api", "tencent-tokenhub", "xai-oauth"):
|
||||
assert slug in by, f"{slug} dropped from catalog (profile-less provider)"
|
||||
assert by[slug].label, f"{slug} has empty label despite canonical fallback"
|
||||
assert by[slug].description, f"{slug} has empty description despite fallback"
|
||||
|
||||
|
||||
def test_api_key_providers_route_to_keys_oauth_to_accounts():
|
||||
by = provider_catalog_by_slug()
|
||||
# api_key → keys
|
||||
assert by["kilocode"].tab == "keys"
|
||||
assert by["openai-api"].tab == "keys"
|
||||
# account / sign-in flows → accounts
|
||||
assert by["google-gemini-cli"].tab == "accounts"
|
||||
assert by["copilot-acp"].tab == "accounts"
|
||||
|
||||
|
||||
def test_copilot_surfaces_as_a_provider_with_its_own_token_var():
|
||||
"""Regression for the reported bug: a GitHub Copilot login showed up under
|
||||
tools, never as a provider, because the shared GITHUB_TOKEN is tool-category.
|
||||
|
||||
Copilot authenticates via the `copilot`/api_key path, so it belongs on the
|
||||
keys tab — but its PRIMARY credential var must be the provider-owned
|
||||
COPILOT_GITHUB_TOKEN, not the shared tool-category GITHUB_TOKEN. That is what
|
||||
lets the desktop render Copilot as its own provider card.
|
||||
"""
|
||||
by = provider_catalog_by_slug()
|
||||
assert "copilot" in by
|
||||
d = by["copilot"]
|
||||
assert d.tab == "keys"
|
||||
assert d.api_key_env_vars, "Copilot must expose a credential env var"
|
||||
assert d.api_key_env_vars[0] == "COPILOT_GITHUB_TOKEN", (
|
||||
"Copilot's primary var must be the provider-owned token, not shared GITHUB_TOKEN"
|
||||
)
|
||||
|
||||
|
||||
def test_bedrock_routes_to_keys():
|
||||
"""Bedrock is aws_sdk (AWS_REGION/AWS_PROFILE), configured on the keys tab."""
|
||||
by = provider_catalog_by_slug()
|
||||
assert by["bedrock"].tab == "keys"
|
||||
|
||||
|
||||
def test_api_key_providers_expose_a_credential_env_var():
|
||||
"""Every keys-tab provider that authenticates via a pasted API key must
|
||||
surface at least one env var to write the key into (otherwise the GUI can't
|
||||
configure it).
|
||||
|
||||
Exemptions: ``aws_sdk`` (bedrock — uses AWS_REGION/AWS_PROFILE) and the
|
||||
``custom`` bring-your-own-endpoint pseudo-provider, which is configured
|
||||
inline via the local-endpoint flow rather than a fixed env var.
|
||||
"""
|
||||
exempt = {"custom"}
|
||||
for d in provider_catalog():
|
||||
if d.auth_type == "api_key" and d.slug not in exempt:
|
||||
assert d.api_key_env_vars, f"{d.slug} is api_key but exposes no env var"
|
||||
|
||||
|
||||
def test_order_mirrors_canonical_declaration():
|
||||
cat = provider_catalog()
|
||||
assert [d.order for d in cat] == list(range(len(cat)))
|
||||
assert [d.slug for d in cat] == [e.slug for e in CANONICAL_PROVIDERS]
|
||||
|
||||
|
||||
def test_descriptors_are_provider_descriptor_instances():
|
||||
for d in provider_catalog():
|
||||
assert isinstance(d, ProviderDescriptor)
|
||||
|
||||
|
||||
def test_tab_for_auth_type_helper():
|
||||
assert tab_for_auth_type("api_key") == "keys"
|
||||
assert tab_for_auth_type("aws_sdk") == "keys"
|
||||
assert tab_for_auth_type("oauth_external") == "accounts"
|
||||
assert tab_for_auth_type("oauth_device_code") == "accounts"
|
||||
assert tab_for_auth_type("copilot") == "accounts"
|
||||
assert tab_for_auth_type("external_process") == "accounts"
|
||||
90
tests/hermes_cli/test_provider_parity.py
Normal file
90
tests/hermes_cli/test_provider_parity.py
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
"""End-to-end provider parity contract: the desktop Providers tabs must show
|
||||
the SAME provider universe as ``hermes model`` (the CLI/TUI picker).
|
||||
|
||||
This is the single load-bearing invariant of the unified provider catalog:
|
||||
|
||||
keys(/api/env provider rows) ∪ ids(/api/providers/oauth) ⊇ CANONICAL_PROVIDERS
|
||||
|
||||
i.e. every provider the CLI picker offers is configurable from the desktop app,
|
||||
on one of the two Providers sub-tabs (API keys or Accounts). It is asserted as
|
||||
an invariant against the real FastAPI endpoints (not a snapshot / count), so it
|
||||
can never silently drift again when a provider plugin is added.
|
||||
"""
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli.models import CANONICAL_PROVIDERS
|
||||
from hermes_cli.provider_catalog import provider_catalog
|
||||
from hermes_cli.web_server import _SESSION_TOKEN, app
|
||||
|
||||
client = TestClient(app)
|
||||
HEADERS = {"X-Hermes-Session-Token": _SESSION_TOKEN}
|
||||
|
||||
# `custom` is the bring-your-own-endpoint pseudo-provider configured inline via
|
||||
# the model picker's local-endpoint flow, not a fixed credential card. It is in
|
||||
# the CLI picker's universe but intentionally has no dedicated Providers-tab
|
||||
# card. Exempt it from the union check.
|
||||
_EXEMPT = {"custom"}
|
||||
|
||||
# Providers that legitimately offer BOTH auth methods and so intentionally
|
||||
# appear on both desktop tabs (an API-key card AND an account sign-in card).
|
||||
# Anthropic supports a direct API key (Keys tab) and a subscription OAuth /
|
||||
# Claude Code login (Accounts tab); surfacing both is correct, not a bug.
|
||||
_DUAL_TAB = {"anthropic"}
|
||||
|
||||
|
||||
def _keys_tab_providers() -> set[str]:
|
||||
"""Provider slugs that have at least one card on the desktop API-keys tab."""
|
||||
data = client.get("/api/env", headers=HEADERS).json()
|
||||
return {
|
||||
info.get("provider")
|
||||
for info in data.values()
|
||||
if info.get("category") == "provider" and info.get("provider")
|
||||
}
|
||||
|
||||
|
||||
def _accounts_tab_providers() -> set[str]:
|
||||
"""Provider slugs offered on the desktop Accounts tab."""
|
||||
data = client.get("/api/providers/oauth", headers=HEADERS).json()
|
||||
return {p["id"] for p in data["providers"]}
|
||||
|
||||
|
||||
def test_every_hermes_model_provider_is_configurable_in_desktop():
|
||||
"""PARITY CONTRACT: GUI (keys ∪ accounts) ⊇ `hermes model` universe."""
|
||||
gui = _keys_tab_providers() | _accounts_tab_providers()
|
||||
missing = [
|
||||
e.slug
|
||||
for e in CANONICAL_PROVIDERS
|
||||
if e.slug not in _EXEMPT and e.slug not in gui
|
||||
]
|
||||
assert not missing, (
|
||||
"providers shown in `hermes model` but not configurable in the desktop "
|
||||
f"Providers tabs: {missing}"
|
||||
)
|
||||
|
||||
|
||||
def test_each_provider_lands_on_the_tab_its_auth_type_dictates():
|
||||
"""A keys-tab provider must surface under /api/env; an accounts-tab provider
|
||||
under /api/providers/oauth. Cross-checks the catalog's tab routing against
|
||||
where each provider actually renders.
|
||||
"""
|
||||
keys = _keys_tab_providers()
|
||||
accounts = _accounts_tab_providers()
|
||||
for d in provider_catalog():
|
||||
if d.slug in _EXEMPT:
|
||||
continue
|
||||
if d.tab == "keys" and d.api_key_env_vars:
|
||||
assert d.slug in keys, f"{d.slug} (keys tab) missing from /api/env"
|
||||
elif d.tab == "accounts":
|
||||
assert d.slug in accounts, f"{d.slug} (accounts tab) missing from /api/providers/oauth"
|
||||
|
||||
|
||||
def test_no_provider_appears_on_both_tabs():
|
||||
"""A provider should be configured exactly one way — not duplicated across
|
||||
both tabs (which would confuse users about where to put credentials).
|
||||
|
||||
Exception: genuinely dual-auth providers (see ``_DUAL_TAB``) intentionally
|
||||
appear on both tabs.
|
||||
"""
|
||||
overlap = (_keys_tab_providers() & _accounts_tab_providers()) - _EXEMPT - _DUAL_TAB
|
||||
assert not overlap, f"providers appearing on BOTH desktop tabs: {sorted(overlap)}"
|
||||
|
|
@ -470,6 +470,39 @@ def test_xai_oauth_listed_as_loopback_flow():
|
|||
assert "grok" in providers["xai-oauth"]["name"].lower()
|
||||
|
||||
|
||||
def test_accounts_offers_every_oauth_provider_from_catalog():
|
||||
"""PARITY CONTRACT: every accounts-tab provider in the unified catalog (the
|
||||
`hermes model` universe) must be offered by /api/providers/oauth. This keeps
|
||||
the desktop Accounts tab in lockstep with the CLI picker — no provider the
|
||||
CLI can sign into may be missing from the GUI.
|
||||
"""
|
||||
from hermes_cli.provider_catalog import provider_catalog
|
||||
|
||||
resp = client.get("/api/providers/oauth", headers=HEADERS)
|
||||
assert resp.status_code == 200, resp.text
|
||||
offered = {p["id"] for p in resp.json()["providers"]}
|
||||
for d in provider_catalog():
|
||||
if d.tab == "accounts":
|
||||
assert d.slug in offered, (
|
||||
f"{d.slug} is an accounts-tab provider in `hermes model` but is "
|
||||
f"missing from the desktop Accounts tab (/api/providers/oauth)"
|
||||
)
|
||||
|
||||
|
||||
def test_gemini_cli_and_copilot_acp_now_in_accounts():
|
||||
"""Regression: google-gemini-cli and copilot-acp were canonical providers the
|
||||
CLI could configure, but had no Accounts card (the reported GUI/CLI drift).
|
||||
"""
|
||||
resp = client.get("/api/providers/oauth", headers=HEADERS)
|
||||
assert resp.status_code == 200, resp.text
|
||||
providers = {p["id"]: p for p in resp.json()["providers"]}
|
||||
assert "google-gemini-cli" in providers
|
||||
assert "copilot-acp" in providers
|
||||
# copilot-acp is managed by an external CLI: read-only card, not auto-removable.
|
||||
assert providers["copilot-acp"]["flow"] == "external"
|
||||
assert providers["copilot-acp"]["disconnectable"] is False
|
||||
|
||||
|
||||
def test_oauth_catalog_marks_external_providers_not_disconnectable():
|
||||
"""External CLI credentials are visible in Accounts but cannot be removed by Hermes."""
|
||||
resp = client.get("/api/providers/oauth", headers=HEADERS)
|
||||
|
|
@ -804,3 +837,56 @@ def test_unknown_pkce_provider_rejected_cleanly():
|
|||
# 4xx — what we MUST NOT see is a 200 with claude.ai in the body.
|
||||
assert resp.status_code >= 400, resp.text
|
||||
assert "claude.ai" not in resp.text.lower()
|
||||
|
||||
|
||||
def test_status_falls_through_to_generic_dispatcher_for_catalog_only_provider():
|
||||
"""Accounts-tab providers with no hardcoded branch reflect REAL status.
|
||||
|
||||
Providers appended to the Accounts tab from the unified provider_catalog()
|
||||
carry status_fn=None and may have no explicit branch in
|
||||
_resolve_provider_status. Before the fallthrough they rendered permanently
|
||||
logged-out; now they dispatch to hermes_cli.auth.get_auth_status (the
|
||||
canonical slug dispatcher) so membership AND status both auto-extend.
|
||||
"""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
fake_status = {
|
||||
"logged_in": True,
|
||||
"provider": "some-future-oauth",
|
||||
"name": "Future OAuth Provider",
|
||||
"access_token": "sk-future-secret-token-xyz",
|
||||
"expires_at": "2026-12-01T00:00:00Z",
|
||||
"has_refresh_token": True,
|
||||
}
|
||||
with patch("hermes_cli.auth.get_auth_status", return_value=fake_status):
|
||||
out = ws._resolve_provider_status("some-future-oauth", None)
|
||||
|
||||
assert out["logged_in"] is True
|
||||
assert out["source"] == "some-future-oauth"
|
||||
assert out["source_label"] == "Future OAuth Provider"
|
||||
# Token is previewed, never returned whole.
|
||||
assert out["token_preview"] and "sk-future-secret-token-xyz" not in out["token_preview"]
|
||||
assert out["expires_at"] == "2026-12-01T00:00:00Z"
|
||||
assert out["has_refresh_token"] is True
|
||||
|
||||
|
||||
def test_status_hardcoded_branch_wins_over_generic_fallback():
|
||||
"""An existing hardcoded branch (nous) is unaffected by the fallthrough."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
with patch(
|
||||
"hermes_cli.auth.get_nous_auth_status",
|
||||
return_value={"logged_in": True, "portal_base_url": "https://portal.test"},
|
||||
):
|
||||
out = ws._resolve_provider_status("nous", None)
|
||||
assert out["source"] == "nous_portal"
|
||||
assert out["source_label"] == "https://portal.test"
|
||||
|
||||
|
||||
def test_status_unknown_provider_degrades_to_logged_out():
|
||||
"""A provider the generic dispatcher can't resolve stays logged-out cleanly."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
with patch("hermes_cli.auth.get_auth_status", return_value={"logged_in": False}):
|
||||
out = ws._resolve_provider_status("totally-unknown", None)
|
||||
assert out["logged_in"] is False
|
||||
|
|
|
|||
|
|
@ -1299,6 +1299,57 @@ class TestWebServerEndpoints:
|
|||
for key, info in data.items():
|
||||
assert info["channel_managed"] is (key in channel_keys)
|
||||
|
||||
def test_get_env_vars_surfaces_catalog_providers(self):
|
||||
"""Every keys-tab provider in the unified catalog must appear in /api/env
|
||||
as a provider card, even when it has no hand entry in OPTIONAL_ENV_VARS.
|
||||
|
||||
Regression for the GUI⇄CLI drift: openai-api, kilocode, novita,
|
||||
tencent-tokenhub, copilot were configurable via `hermes model` but
|
||||
invisible in the desktop Providers → API keys tab.
|
||||
"""
|
||||
from hermes_cli.provider_catalog import provider_catalog
|
||||
|
||||
data = self.client.get("/api/env").json()
|
||||
for d in provider_catalog():
|
||||
if d.tab != "keys" or not d.api_key_env_vars:
|
||||
continue
|
||||
# The PRIMARY credential var must surface as this provider's card.
|
||||
# (Shared aliases like GITHUB_TOKEN are intentionally left on their
|
||||
# existing tool category and not hijacked — see the copilot test.)
|
||||
primary = d.api_key_env_vars[0]
|
||||
assert primary in data, f"{primary} ({d.slug}) missing from /api/env"
|
||||
info = data[primary]
|
||||
assert info["category"] == "provider"
|
||||
assert info["provider"] == d.slug
|
||||
assert info["provider_label"] == d.label
|
||||
|
||||
def test_get_env_vars_provider_rows_carry_grouping_hints(self):
|
||||
"""Provider env rows expose the backend `provider`/`provider_label` the
|
||||
desktop Keys tab groups by (so it no longer relies on prefix guesses)."""
|
||||
data = self.client.get("/api/env").json()
|
||||
# OPENAI_API_KEY is a hand-listed protected var AND a catalog provider;
|
||||
# it must come back tagged to the openai-api provider.
|
||||
assert data["OPENAI_API_KEY"]["provider"] == "openai-api"
|
||||
assert data["OPENAI_API_KEY"]["category"] == "provider"
|
||||
|
||||
def test_get_env_vars_copilot_uses_provider_token_not_shared_github_token(self):
|
||||
"""Copilot surfaces as its own provider card via COPILOT_GITHUB_TOKEN;
|
||||
the shared GITHUB_TOKEN keeps its existing (tool) category."""
|
||||
data = self.client.get("/api/env").json()
|
||||
assert data["COPILOT_GITHUB_TOKEN"]["provider"] == "copilot"
|
||||
assert data["COPILOT_GITHUB_TOKEN"]["category"] == "provider"
|
||||
# Shared GITHUB_TOKEN must NOT be hijacked into the copilot provider card.
|
||||
assert data.get("GITHUB_TOKEN", {}).get("provider", "") != "copilot"
|
||||
|
||||
def test_get_env_vars_bedrock_aws_vars_tagged_to_provider(self):
|
||||
"""Bedrock (aws_sdk, no api-key) must still appear on the Keys tab: its
|
||||
AWS_REGION/AWS_PROFILE settings are tagged to the bedrock provider card.
|
||||
"""
|
||||
data = self.client.get("/api/env").json()
|
||||
assert data["AWS_REGION"]["provider"] == "bedrock"
|
||||
assert data["AWS_REGION"]["category"] == "provider"
|
||||
assert data["AWS_PROFILE"]["provider"] == "bedrock"
|
||||
|
||||
def test_platform_scoped_messaging_env_vars_are_channel_managed(self):
|
||||
from hermes_cli.web_server import (
|
||||
_MESSAGING_KEYS_PAGE_KEYS,
|
||||
|
|
@ -1552,6 +1603,27 @@ class TestWebServerEndpoints:
|
|||
assert telegram["enabled"] is False
|
||||
assert any(field["key"] == "TELEGRAM_BOT_TOKEN" and field["required"] for field in telegram["env_vars"])
|
||||
|
||||
def test_slack_messaging_platform_exposes_user_allowlist(self):
|
||||
resp = self.client.get("/api/messaging/platforms")
|
||||
|
||||
assert resp.status_code == 200
|
||||
platforms = resp.json()["platforms"]
|
||||
slack = next(platform for platform in platforms if platform["id"] == "slack")
|
||||
fields = {field["key"]: field for field in slack["env_vars"]}
|
||||
|
||||
assert "allowed Slack member IDs" in slack["description"]
|
||||
assert set(fields) >= {
|
||||
"SLACK_BOT_TOKEN",
|
||||
"SLACK_APP_TOKEN",
|
||||
"SLACK_ALLOWED_USERS",
|
||||
}
|
||||
assert fields["SLACK_ALLOWED_USERS"]["prompt"] == "Allowed Slack member IDs"
|
||||
assert fields["SLACK_ALLOWED_USERS"]["is_password"] is False
|
||||
assert "member IDs" in fields["SLACK_ALLOWED_USERS"]["description"]
|
||||
assert "Bot User OAuth Token" in fields["SLACK_BOT_TOKEN"]["help"]
|
||||
assert "App-Level Tokens" in fields["SLACK_APP_TOKEN"]["help"]
|
||||
assert "Copy member ID" in fields["SLACK_ALLOWED_USERS"]["help"]
|
||||
|
||||
def test_weixin_messaging_metadata_describes_personal_ilink_setup(self):
|
||||
resp = self.client.get("/api/messaging/platforms")
|
||||
|
||||
|
|
@ -1628,6 +1700,70 @@ class TestWebServerEndpoints:
|
|||
telegram = next(platform for platform in status if platform["id"] == "telegram")
|
||||
assert telegram["enabled"] is False
|
||||
|
||||
def test_update_messaging_platform_saves_slack_allowed_users(self):
|
||||
from hermes_cli.config import load_env
|
||||
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,U04XYZ5LMN6"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert load_env()["SLACK_ALLOWED_USERS"] == "U01ABC2DEF3,U04XYZ5LMN6"
|
||||
|
||||
def test_update_messaging_platform_rejects_swapped_slack_bot_token(self):
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_BOT_TOKEN": "xapp-wrong-token-type"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "xoxb-" in resp.json()["detail"]
|
||||
|
||||
def test_update_messaging_platform_rejects_swapped_slack_app_token(self):
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_APP_TOKEN": "xoxb-wrong-token-type"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "xapp-" in resp.json()["detail"]
|
||||
|
||||
def test_update_messaging_platform_rejects_invalid_slack_allowed_users(self):
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,not-a-user"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "member IDs" in resp.json()["detail"]
|
||||
|
||||
def test_update_messaging_platform_accepts_slack_allowed_users_wildcard(self):
|
||||
# "*" is the gateway's allow-all wildcard (gateway/platforms/slack.py),
|
||||
# so the dashboard must accept it rather than rejecting it as malformed.
|
||||
from hermes_cli.config import load_env
|
||||
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_ALLOWED_USERS": "*"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert load_env()["SLACK_ALLOWED_USERS"] == "*"
|
||||
|
||||
def test_update_messaging_platform_accepts_slack_allowed_users_trailing_comma(self):
|
||||
# The gateway drops empty entries (gateway/platforms/slack.py), so a
|
||||
# trailing/interior comma must not be rejected by the dashboard.
|
||||
from hermes_cli.config import load_env
|
||||
|
||||
resp = self.client.put(
|
||||
"/api/messaging/platforms/slack",
|
||||
json={"env": {"SLACK_ALLOWED_USERS": "U01ABC2DEF3,,W04XYZ5LMN6,"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert load_env()["SLACK_ALLOWED_USERS"] == "U01ABC2DEF3,,W04XYZ5LMN6,"
|
||||
|
||||
def test_messaging_platform_test_reports_missing_required_setup(self):
|
||||
resp = self.client.put("/api/messaging/platforms/discord", json={"enabled": True})
|
||||
assert resp.status_code == 200
|
||||
|
|
@ -5062,6 +5198,7 @@ class TestPtyWebSocket:
|
|||
|
||||
_argv, _cwd, env = self.ws_module._resolve_chat_argv()
|
||||
|
||||
assert env["HERMES_TUI_DASHBOARD"] == "1"
|
||||
assert env["HERMES_TUI_INLINE"] == "1"
|
||||
assert env["HERMES_TUI_DISABLE_MOUSE"] == "1"
|
||||
|
||||
|
|
|
|||
|
|
@ -436,3 +436,55 @@ def test_stream_upload_large_file_under_cap_succeeds(forced_files_client, monkey
|
|||
assert created.status_code == 200
|
||||
assert file_path.stat().st_size == len(payload)
|
||||
assert file_path.read_bytes() == payload
|
||||
|
||||
|
||||
def test_stream_upload_cleans_temp_on_cancellation(forced_files_client):
|
||||
"""A client disconnect mid-stream (asyncio.CancelledError) must not leak a temp file.
|
||||
|
||||
CancelledError is a BaseException, not an Exception, so it bypasses the
|
||||
endpoint's ``except`` clauses entirely. The cleanup therefore lives in a
|
||||
``finally`` keyed on a success flag — without it, every aborted large
|
||||
upload (the exact NS-501 scenario) would orphan a partial ``.upload`` temp
|
||||
file in the target directory. We invoke the endpoint coroutine directly so
|
||||
the BaseException propagates instead of being swallowed by the test client.
|
||||
"""
|
||||
import asyncio
|
||||
|
||||
_client, root = forced_files_client
|
||||
target = root / "out" / "aborted.bin"
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
class _AbortingUpload:
|
||||
"""UploadFile stand-in that yields one chunk then aborts like a dropped client."""
|
||||
|
||||
filename = "aborted.bin"
|
||||
|
||||
def __init__(self):
|
||||
self._calls = 0
|
||||
|
||||
async def read(self, _size):
|
||||
self._calls += 1
|
||||
if self._calls == 1:
|
||||
return b"partial chunk before the client vanished"
|
||||
raise asyncio.CancelledError()
|
||||
|
||||
async def close(self):
|
||||
return None
|
||||
|
||||
request = SimpleNamespace()
|
||||
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
asyncio.run(
|
||||
web_server.upload_managed_file_stream(
|
||||
request=request,
|
||||
file=_AbortingUpload(),
|
||||
path=str(target),
|
||||
overwrite=True,
|
||||
)
|
||||
)
|
||||
|
||||
# No partial data was promoted into place ...
|
||||
assert not target.exists()
|
||||
# ... and no .upload temp file was left behind.
|
||||
leftovers = [p.name for p in target.parent.iterdir() if ".upload" in p.name]
|
||||
assert leftovers == [], f"temp upload files leaked on cancellation: {leftovers}"
|
||||
|
|
|
|||
|
|
@ -265,6 +265,355 @@ class TestOpenVikingSkillQuerySafety:
|
|||
assert RecordingVikingClient.calls == []
|
||||
|
||||
|
||||
class TestOpenVikingTurnConversion:
|
||||
def test_extract_current_turn_anchors_on_latest_matching_user_and_assistant(self):
|
||||
messages = [
|
||||
{"role": "user", "content": "Please inspect the repository for assemble hooks."},
|
||||
{"role": "assistant", "content": "Earlier answer."},
|
||||
{"role": "user", "content": "Please inspect the repository for assemble hooks."},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "I will search the codebase.",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_rg_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "shell_command",
|
||||
"arguments": json.dumps({"command": "rg assemble"}),
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_rg_1",
|
||||
"name": "shell_command",
|
||||
"content": "agent/context_engine.py: no preassemble hook",
|
||||
},
|
||||
{"role": "assistant", "content": "The current main does not expose assemble."},
|
||||
]
|
||||
|
||||
turn = OpenVikingMemoryProvider._extract_current_turn_messages(
|
||||
messages,
|
||||
"Please inspect the repository for assemble hooks.",
|
||||
"The current main does not expose assemble.",
|
||||
)
|
||||
|
||||
assert turn == messages[2:]
|
||||
|
||||
def test_messages_to_openviking_batch_coalesces_tool_results(self):
|
||||
turn = [
|
||||
{"role": "user", "content": "Please inspect the repository for assemble hooks."},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "I will search the codebase.",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_rg_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "shell_command",
|
||||
"arguments": json.dumps({"command": "rg assemble"}),
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_rg_1",
|
||||
"name": "shell_command",
|
||||
"content": "agent/context_engine.py: no preassemble hook",
|
||||
},
|
||||
{"role": "assistant", "content": "The current main does not expose assemble."},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert [message["role"] for message in batch] == ["user", "assistant", "assistant", "assistant"]
|
||||
assert batch[0]["parts"] == [
|
||||
{"type": "text", "text": "Please inspect the repository for assemble hooks."}
|
||||
]
|
||||
assert batch[1]["parts"] == [
|
||||
{"type": "text", "text": "I will search the codebase."}
|
||||
]
|
||||
assert batch[2]["parts"] == [
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_rg_1",
|
||||
"tool_name": "shell_command",
|
||||
"tool_input": {"command": "rg assemble"},
|
||||
"tool_output": "agent/context_engine.py: no preassemble hook",
|
||||
"tool_status": "completed",
|
||||
}
|
||||
]
|
||||
assert batch[3]["parts"] == [
|
||||
{"type": "text", "text": "The current main does not expose assemble."}
|
||||
]
|
||||
|
||||
def test_messages_to_openviking_batch_marks_json_tool_error_results(self):
|
||||
turn = [
|
||||
{"role": "user", "content": "Check the file."},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_read_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "read_file",
|
||||
"arguments": json.dumps({"path": "missing.md"}),
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_read_1",
|
||||
"name": "read_file",
|
||||
"content": json.dumps({"error": "File not found", "exit_code": 1}),
|
||||
},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert batch[1]["role"] == "assistant"
|
||||
assert batch[1]["parts"] == [
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_read_1",
|
||||
"tool_name": "read_file",
|
||||
"tool_input": {"path": "missing.md"},
|
||||
"tool_output": json.dumps({"error": "File not found", "exit_code": 1}),
|
||||
"tool_status": "error",
|
||||
}
|
||||
]
|
||||
|
||||
def test_messages_to_openviking_batch_keeps_pending_tool_call_without_result(self):
|
||||
turn = [
|
||||
{"role": "user", "content": "Start a long running check."},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "Starting it now.",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_long_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "long_check",
|
||||
"arguments": json.dumps({"target": "repo"}),
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert batch[1]["parts"] == [
|
||||
{"type": "text", "text": "Starting it now."},
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_long_1",
|
||||
"tool_name": "long_check",
|
||||
"tool_input": {"target": "repo"},
|
||||
"tool_status": "pending",
|
||||
},
|
||||
]
|
||||
|
||||
def test_messages_to_openviking_batch_coalesces_adjacent_tool_results(self):
|
||||
turn = [
|
||||
{"role": "user", "content": "Run both tools."},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_a",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "first_tool",
|
||||
"arguments": json.dumps({"x": 1}),
|
||||
},
|
||||
},
|
||||
{
|
||||
"id": "call_b",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "second_tool",
|
||||
"arguments": json.dumps({"y": 2}),
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": "call_a", "name": "first_tool", "content": "a"},
|
||||
{"role": "tool", "tool_call_id": "call_b", "name": "second_tool", "content": "b"},
|
||||
{"role": "assistant", "content": "Done."},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert [message["role"] for message in batch] == ["user", "assistant", "assistant"]
|
||||
assert batch[1]["parts"] == [
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_a",
|
||||
"tool_name": "first_tool",
|
||||
"tool_input": {"x": 1},
|
||||
"tool_output": "a",
|
||||
"tool_status": "completed",
|
||||
},
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_b",
|
||||
"tool_name": "second_tool",
|
||||
"tool_input": {"y": 2},
|
||||
"tool_output": "b",
|
||||
"tool_status": "completed",
|
||||
},
|
||||
]
|
||||
|
||||
def test_messages_to_openviking_batch_skips_openviking_recall_tool_results(self):
|
||||
for recall_tool_name in ("viking_search", "viking_read", "viking_browse"):
|
||||
turn = [
|
||||
{"role": "user", "content": "What did we decide about context assembly?"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call_recall_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": recall_tool_name,
|
||||
"arguments": json.dumps({"query": "context assembly decision"}),
|
||||
},
|
||||
},
|
||||
{
|
||||
"id": "call_shell_1",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "shell_command",
|
||||
"arguments": json.dumps({"command": "rg preassemble"}),
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_recall_1",
|
||||
"name": recall_tool_name,
|
||||
"content": json.dumps({
|
||||
"results": [
|
||||
{
|
||||
"uri": "viking://user/hermes/memories/context",
|
||||
"abstract": "Old OpenViking memory content",
|
||||
}
|
||||
]
|
||||
}),
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call_shell_1",
|
||||
"name": "shell_command",
|
||||
"content": "plugins/memory/openviking/__init__.py",
|
||||
},
|
||||
{"role": "assistant", "content": "We decided to keep sync_turn scoped to ingestion."},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert [message["role"] for message in batch] == ["user", "assistant", "assistant"]
|
||||
assert batch[1]["parts"] == [
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call_shell_1",
|
||||
"tool_name": "shell_command",
|
||||
"tool_input": {"command": "rg preassemble"},
|
||||
"tool_output": "plugins/memory/openviking/__init__.py",
|
||||
"tool_status": "completed",
|
||||
}
|
||||
]
|
||||
batch_text = json.dumps(batch)
|
||||
assert recall_tool_name not in batch_text
|
||||
assert "Old OpenViking memory content" not in batch_text
|
||||
|
||||
def test_messages_to_openviking_batch_empty_tool_id_does_not_drop_other_results(self):
|
||||
# A recall tool result that arrives with an empty tool_call_id must not
|
||||
# poison the skip set with "" and silently drop unrelated tool results
|
||||
# that also lack an id. Empty tool_call_id is reachable in the canonical
|
||||
# transcript (agent_runtime_helpers defaults it to "").
|
||||
turn = [
|
||||
{"role": "user", "content": "What did we decide?"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "viking_search",
|
||||
"arguments": json.dumps({"query": "decision"}),
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "",
|
||||
"name": "viking_search",
|
||||
"content": json.dumps({"results": ["recall stuff"]}),
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "",
|
||||
"name": "shell_command",
|
||||
"content": "important shell output",
|
||||
},
|
||||
{"role": "assistant", "content": "done"},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
batch_text = json.dumps(batch)
|
||||
# The unrelated (empty-id) shell result must survive.
|
||||
assert "important shell output" in batch_text
|
||||
# The recall tool result must still be excluded.
|
||||
assert "recall stuff" not in batch_text
|
||||
assert "viking_search" not in batch_text
|
||||
|
||||
def test_messages_to_openviking_batch_preserves_responses_text_parts(self):
|
||||
turn = [
|
||||
{"role": "user", "content": [{"type": "input_text", "text": "hello"}]},
|
||||
{"role": "assistant", "content": [{"type": "output_text", "text": "answer"}]},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn)
|
||||
|
||||
assert batch == [
|
||||
{"role": "user", "parts": [{"type": "text", "text": "hello"}]},
|
||||
{"role": "assistant", "parts": [{"type": "text", "text": "answer"}]},
|
||||
]
|
||||
|
||||
def test_messages_to_openviking_batch_adds_assistant_peer_id_when_requested(self):
|
||||
turn = [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": "answer"},
|
||||
]
|
||||
|
||||
batch = OpenVikingMemoryProvider._messages_to_openviking_batch(
|
||||
turn,
|
||||
assistant_peer_id="hermes",
|
||||
)
|
||||
|
||||
assert batch == [
|
||||
{"role": "user", "parts": [{"type": "text", "text": "hello"}]},
|
||||
{"role": "assistant", "parts": [{"type": "text", "text": "answer"}], "peer_id": "hermes"},
|
||||
]
|
||||
|
||||
|
||||
class TestOpenVikingRead:
|
||||
def test_overview_read_normalizes_uri_and_unwraps_result(self):
|
||||
provider = OpenVikingMemoryProvider()
|
||||
|
|
|
|||
|
|
@ -83,6 +83,66 @@ def _make_mock_client():
|
|||
return client
|
||||
|
||||
|
||||
def _provider_for_mode(tmp_path, monkeypatch, mode: str):
|
||||
"""Create an initialized provider without pre-seeding its client."""
|
||||
config = {
|
||||
"mode": mode,
|
||||
"apiKey": "test-key",
|
||||
"api_url": "http://localhost:9999",
|
||||
"bank_id": "test-bank",
|
||||
"budget": "mid",
|
||||
"memory_mode": "hybrid",
|
||||
}
|
||||
config_path = tmp_path / "hindsight" / "config.json"
|
||||
config_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
config_path.write_text(json.dumps(config))
|
||||
|
||||
monkeypatch.setattr(
|
||||
"plugins.memory.hindsight.get_hermes_home", lambda: tmp_path
|
||||
)
|
||||
|
||||
provider = HindsightMemoryProvider()
|
||||
provider.initialize(session_id="test-session", hermes_home=str(tmp_path), platform="cli")
|
||||
return provider
|
||||
|
||||
|
||||
def _assert_cloud_client_lazy_installed_before_import(tmp_path, monkeypatch, mode: str):
|
||||
"""Cloud/local-external clients must ensure lazy deps before importing."""
|
||||
import builtins
|
||||
|
||||
provider = _provider_for_mode(tmp_path, monkeypatch, mode)
|
||||
ensure_calls = []
|
||||
|
||||
def fake_ensure(feature, prompt=True):
|
||||
ensure_calls.append((feature, prompt))
|
||||
|
||||
class FakeHindsight:
|
||||
def __init__(self, **kwargs):
|
||||
self.kwargs = kwargs
|
||||
|
||||
real_import = builtins.__import__
|
||||
|
||||
def guarded_import(name, globals=None, locals=None, fromlist=(), level=0):
|
||||
if name == "hindsight_client":
|
||||
if ensure_calls != [("memory.hindsight", False)]:
|
||||
raise ModuleNotFoundError("No module named 'hindsight_client'")
|
||||
return SimpleNamespace(Hindsight=FakeHindsight)
|
||||
return real_import(name, globals, locals, fromlist, level)
|
||||
|
||||
monkeypatch.setattr("tools.lazy_deps.ensure", fake_ensure)
|
||||
monkeypatch.setattr(builtins, "__import__", guarded_import)
|
||||
|
||||
client = provider._get_client()
|
||||
|
||||
assert ensure_calls == [("memory.hindsight", False)]
|
||||
assert isinstance(client, FakeHindsight)
|
||||
assert client.kwargs == {
|
||||
"base_url": "http://localhost:9999",
|
||||
"timeout": 120.0,
|
||||
"api_key": "test-key",
|
||||
}
|
||||
|
||||
|
||||
class _FakeSessionDB:
|
||||
def __init__(self, messages=None):
|
||||
self._messages = list(messages or [])
|
||||
|
|
@ -232,6 +292,14 @@ class TestSchemas:
|
|||
|
||||
|
||||
class TestConfig:
|
||||
def test_cloud_client_lazy_installs_dependency_before_import(self, tmp_path, monkeypatch):
|
||||
_assert_cloud_client_lazy_installed_before_import(tmp_path, monkeypatch, "cloud")
|
||||
|
||||
def test_local_external_client_lazy_installs_dependency_before_import(self, tmp_path, monkeypatch):
|
||||
_assert_cloud_client_lazy_installed_before_import(
|
||||
tmp_path, monkeypatch, "local_external"
|
||||
)
|
||||
|
||||
def test_default_values(self, provider):
|
||||
assert provider._auto_retain is True
|
||||
assert provider._auto_recall is True
|
||||
|
|
|
|||
|
|
@ -1975,7 +1975,10 @@ def test_on_session_switch_commits_old_session_and_rotates_id():
|
|||
|
||||
provider.on_session_switch("new-sid", parent_session_id="old-sid")
|
||||
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
assert provider._session_id == "new-sid"
|
||||
assert provider._turn_count == 0
|
||||
|
||||
|
|
@ -1998,7 +2001,10 @@ def test_on_session_switch_commits_pending_tokens_without_turn_count():
|
|||
provider.on_session_switch("new-sid")
|
||||
|
||||
provider._client.get.assert_called_once_with("/api/v1/sessions/old-sid")
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
assert provider._session_id == "new-sid"
|
||||
assert provider._turn_count == 0
|
||||
|
||||
|
|
@ -2051,7 +2057,10 @@ def test_on_session_switch_waits_for_inflight_sync_thread():
|
|||
provider.on_session_switch("new-sid")
|
||||
|
||||
assert join_calls, "expected on_session_switch to join the in-flight sync thread"
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
|
||||
|
||||
def test_on_session_switch_noop_on_empty_new_id():
|
||||
|
|
@ -2186,6 +2195,78 @@ def test_sync_turn_retries_batch_write_with_fresh_client():
|
|||
)]
|
||||
|
||||
|
||||
def test_sync_turn_structured_messages_include_assistant_peer_id():
|
||||
provider = OpenVikingMemoryProvider()
|
||||
provider._client = MagicMock()
|
||||
provider._endpoint = "http://test"
|
||||
provider._api_key = ""
|
||||
provider._account = "acct"
|
||||
provider._user = "usr"
|
||||
provider._agent = "hermes"
|
||||
provider._session_id = "sid-structured"
|
||||
|
||||
captured = []
|
||||
|
||||
class StubClient:
|
||||
def __init__(self, *a, **kw):
|
||||
pass
|
||||
|
||||
def post(self, path, payload=None, **kwargs):
|
||||
captured.append((path, payload))
|
||||
return {}
|
||||
|
||||
import plugins.memory.openviking as _mod
|
||||
|
||||
real_client_cls = _mod._VikingClient
|
||||
_mod._VikingClient = StubClient
|
||||
messages = [
|
||||
{"role": "user", "content": [{"type": "input_text", "text": "u"}]},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "Looking.",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "call-1",
|
||||
"type": "function",
|
||||
"function": {"name": "shell_command", "arguments": json.dumps({"cmd": "pwd"})},
|
||||
}
|
||||
],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": "call-1", "name": "shell_command", "content": "ok"},
|
||||
{"role": "assistant", "content": [{"type": "output_text", "text": "a"}]},
|
||||
]
|
||||
try:
|
||||
provider.sync_turn("u", "a", messages=messages)
|
||||
assert provider._drain_writers("sid-structured", timeout=2.0)
|
||||
finally:
|
||||
_mod._VikingClient = real_client_cls
|
||||
|
||||
assert captured == [(
|
||||
"/api/v1/sessions/sid-structured/messages/batch",
|
||||
{
|
||||
"messages": [
|
||||
{"role": "user", "parts": [{"type": "text", "text": "u"}]},
|
||||
{"role": "assistant", "parts": [{"type": "text", "text": "Looking."}], "peer_id": "hermes"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"parts": [
|
||||
{
|
||||
"type": "tool",
|
||||
"tool_id": "call-1",
|
||||
"tool_name": "shell_command",
|
||||
"tool_input": {"cmd": "pwd"},
|
||||
"tool_output": "ok",
|
||||
"tool_status": "completed",
|
||||
}
|
||||
],
|
||||
"peer_id": "hermes",
|
||||
},
|
||||
{"role": "assistant", "parts": [{"type": "text", "text": "a"}], "peer_id": "hermes"},
|
||||
]
|
||||
},
|
||||
)]
|
||||
|
||||
|
||||
def test_sync_turn_noop_when_session_id_blank():
|
||||
provider = OpenVikingMemoryProvider()
|
||||
provider._client = MagicMock()
|
||||
|
|
@ -2206,7 +2287,10 @@ def test_on_session_end_marks_session_clean_after_successful_commit():
|
|||
|
||||
provider.on_session_end([])
|
||||
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
assert provider._turn_count == 0
|
||||
|
||||
|
||||
|
|
@ -2228,7 +2312,10 @@ def test_on_session_end_commits_pending_tokens_without_turn_count():
|
|||
provider.on_session_end([])
|
||||
|
||||
provider._client.get.assert_called_once_with("/api/v1/sessions/old-sid")
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
|
||||
|
||||
def test_end_then_switch_does_not_double_commit():
|
||||
|
|
@ -2241,7 +2328,10 @@ def test_end_then_switch_does_not_double_commit():
|
|||
provider.on_session_switch("new-sid", parent_session_id="old-sid")
|
||||
|
||||
# Exactly one commit call, on the OLD session, fired by on_session_end.
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
assert provider._session_id == "new-sid"
|
||||
assert provider._turn_count == 0
|
||||
|
||||
|
|
@ -2253,7 +2343,10 @@ def test_end_then_switch_with_pending_tokens_does_not_double_commit():
|
|||
provider.on_session_end([])
|
||||
provider.on_session_switch("new-sid", parent_session_id="old-sid")
|
||||
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
assert provider._session_id == "new-sid"
|
||||
assert provider._turn_count == 0
|
||||
|
||||
|
|
@ -2400,7 +2493,10 @@ def test_on_session_switch_does_not_block_caller_on_slow_drain():
|
|||
# Let the finalizer finish so it doesn't leak past the test.
|
||||
release_drain.set()
|
||||
assert provider._drain_finalizers(timeout=5.0)
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
|
||||
|
||||
def test_on_session_switch_defers_old_commit_to_finalizer_thread():
|
||||
|
|
@ -2415,7 +2511,7 @@ def test_on_session_switch_defers_old_commit_to_finalizer_thread():
|
|||
committed = threading.Event()
|
||||
drain_timeouts = []
|
||||
|
||||
def fake_post(path):
|
||||
def fake_post(path, payload=None):
|
||||
committed.set()
|
||||
return {}
|
||||
|
||||
|
|
@ -2433,7 +2529,10 @@ def test_on_session_switch_defers_old_commit_to_finalizer_thread():
|
|||
assert provider._turn_count == 0
|
||||
# The old-session commit lands on the finalizer thread, not inline.
|
||||
assert committed.wait(timeout=5.0), "old session was not finalized off-thread"
|
||||
provider._client.post.assert_called_once_with("/api/v1/sessions/old-sid/commit")
|
||||
provider._client.post.assert_called_once_with(
|
||||
"/api/v1/sessions/old-sid/commit",
|
||||
{"keep_recent_count": 0},
|
||||
)
|
||||
# The finalizer drains with the deferred (longer) budget, not inline 10s.
|
||||
assert drain_timeouts == [_DEFERRED_COMMIT_TIMEOUT]
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ Verifies that:
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
|
@ -148,6 +148,17 @@ class TestRunConversationCodexPath:
|
|||
and m.get("content") == "echo: hello"]
|
||||
assert final, f"expected final assistant message in {msgs}"
|
||||
|
||||
def test_projected_messages_are_synced_to_external_memory(self, fake_session):
|
||||
agent = _make_codex_agent()
|
||||
agent._memory_manager = MagicMock()
|
||||
agent._memory_manager.build_system_prompt.return_value = ""
|
||||
|
||||
with patch.object(agent, "_spawn_background_review", return_value=None):
|
||||
result = agent.run_conversation("hello")
|
||||
|
||||
agent._memory_manager.sync_all.assert_called_once()
|
||||
assert agent._memory_manager.sync_all.call_args.kwargs["messages"] == result["messages"]
|
||||
|
||||
def test_nudge_counters_tick(self, fake_session):
|
||||
"""The skill nudge counter must accumulate tool_iterations across
|
||||
turns. The memory nudge counter is gated on memory being configured
|
||||
|
|
|
|||
130
tests/run_agent/test_nonretryable_error_html_summary.py
Normal file
130
tests/run_agent/test_nonretryable_error_html_summary.py
Normal file
|
|
@ -0,0 +1,130 @@
|
|||
"""Regression: non-retryable API failures must not leak raw HTML pages.
|
||||
|
||||
A scheduled cron job fell back to the Codex (``chatgpt.com``) provider, which
|
||||
returned a Cloudflare *challenge* page (HTTP 403) instead of a normal API
|
||||
response. The conversation loop classified this as a non-retryable client
|
||||
error and returned the failure dict — but the ``error`` field carried
|
||||
``str(api_error)``, i.e. the entire ~60 KB Cloudflare HTML page. The cron
|
||||
scheduler then delivered that verbatim to Discord, where it was split into
|
||||
~31 messages (the reporter's "31 part discord message which is cloudflares
|
||||
challenge page").
|
||||
|
||||
The sibling "max retries exhausted" path already summarized the error via
|
||||
``_summarize_api_error`` (which collapses HTML pages to a one-liner); the
|
||||
non-retryable path did not. These tests lock the contract: whichever
|
||||
terminal path is taken, ``result['error']`` is a short, HTML-free summary.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import run_agent
|
||||
from run_agent import AIAgent
|
||||
|
||||
|
||||
# A representative Cloudflare "managed challenge" body, matching the shape the
|
||||
# Codex backend returned in the field report (no <title>, large inline
|
||||
# ``_cf_chl_opt`` script). Padded so length-based assertions are meaningful.
|
||||
_CLOUDFLARE_CHALLENGE_HTML = (
|
||||
"<!DOCTYPE html>\n<html>\n <head>\n"
|
||||
' <meta http-equiv="refresh" content="360"></head>\n'
|
||||
" <body>\n <div class=\"data\"><noscript>"
|
||||
"Enable JavaScript and cookies to continue</noscript>"
|
||||
"<script>(function(){window._cf_chl_opt = {cRay: 'a0ca002c4f91769c',"
|
||||
"cZone: 'chatgpt.com', cType: 'managed', "
|
||||
+ ("md: '" + "x" * 4000 + "',")
|
||||
+ "};})();</script></div>\n </body>\n</html>\n"
|
||||
)
|
||||
|
||||
|
||||
def _make_403_html_error() -> Exception:
|
||||
"""An exception mimicking a Codex 403 whose body is a Cloudflare page."""
|
||||
err = Exception(_CLOUDFLARE_CHALLENGE_HTML)
|
||||
err.status_code = 403
|
||||
return err
|
||||
|
||||
|
||||
def _make_agent() -> AIAgent:
|
||||
# Drive the standard chat-completions path with a concrete model so the
|
||||
# turn actually reaches ``client.chat.completions.create`` — that is where
|
||||
# the mocked 403 is raised. The non-retryable abort being exercised lives
|
||||
# in the shared conversation loop and is provider-agnostic; a Cloudflare
|
||||
# "managed challenge" 403 can surface on any provider sitting behind
|
||||
# Cloudflare (it was first reported on the Codex backend). Pinning
|
||||
# ``api_mode`` + ``model`` here avoids the earlier abort the previous
|
||||
# revision hit: an empty model on the Codex Responses path raised a
|
||||
# validation ``ValueError`` *before* any API call, so the test passed
|
||||
# without ever touching the 403 summarization path.
|
||||
with (
|
||||
patch("run_agent.get_tool_definitions", return_value=[]),
|
||||
patch("run_agent.check_toolset_requirements", return_value={}),
|
||||
patch("run_agent.OpenAI"),
|
||||
):
|
||||
a = AIAgent(
|
||||
api_key="test-key-1234567890",
|
||||
base_url="https://api.openai.com/v1",
|
||||
provider="openai",
|
||||
api_mode="chat_completions",
|
||||
model="gpt-5.5",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
a.client = MagicMock()
|
||||
a._cached_system_prompt = "You are helpful."
|
||||
a._use_prompt_caching = False
|
||||
a.tool_delay = 0
|
||||
a.compression_enabled = False
|
||||
a.save_trajectories = False
|
||||
return a
|
||||
|
||||
|
||||
def test_summarize_collapses_cloudflare_challenge_page():
|
||||
"""``_summarize_api_error`` must never echo the raw HTML body."""
|
||||
summary = AIAgent._summarize_api_error(_make_403_html_error())
|
||||
|
||||
assert "<html" not in summary.lower()
|
||||
assert "<!doctype" not in summary.lower()
|
||||
assert "_cf_chl_opt" not in summary
|
||||
# A one-liner, not a multi-kilobyte page.
|
||||
assert len(summary) < 200
|
||||
# Still informative: the HTTP status survives.
|
||||
assert "403" in summary
|
||||
|
||||
|
||||
def test_non_retryable_failure_error_is_summarized_not_raw_html():
|
||||
"""The terminal non-retryable dict must carry a short, HTML-free error.
|
||||
|
||||
This is the exact field path: a 403 Cloudflare challenge with no fallback
|
||||
configured aborts as a non-retryable client error. Before the fix the
|
||||
returned ``error`` was the full ~60 KB page.
|
||||
|
||||
The mocked 403 is the *only* failure the turn can hit — the agent reaches
|
||||
``client.chat.completions.create`` (asserted below), so the test cannot
|
||||
pass vacuously by aborting on some earlier, unrelated error.
|
||||
"""
|
||||
agent = _make_agent()
|
||||
agent.client.chat.completions.create.side_effect = _make_403_html_error()
|
||||
|
||||
with (
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = agent.run_conversation("daily briefing please")
|
||||
|
||||
# Guard against a vacuous pass: the mocked 403 must actually be the
|
||||
# failure that aborted the turn. (The previous revision never reached
|
||||
# this call and still "passed".)
|
||||
assert agent.client.chat.completions.create.called
|
||||
assert result.get("failed") is True
|
||||
error = result.get("error") or ""
|
||||
# The whole point of the fix: no raw HTML / Cloudflare markup leaks.
|
||||
assert "<html" not in error.lower()
|
||||
assert "<!doctype" not in error.lower()
|
||||
assert "_cf_chl_opt" not in error
|
||||
# Still informative: the summarized 403 status survives into the field
|
||||
# delivered downstream.
|
||||
assert "403" in error
|
||||
# The original page was tens of kilobytes; a summary is short.
|
||||
assert len(error) < 500
|
||||
assert len(error) < len(_CLOUDFLARE_CHALLENGE_HTML)
|
||||
|
|
@ -5813,12 +5813,126 @@ class TestAnthropicCredentialRefresh:
|
|||
|
||||
response = SimpleNamespace(content=[])
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._anthropic_client.messages.create.return_value = response
|
||||
stream_cm = MagicMock()
|
||||
stream_cm.__enter__.return_value.get_final_message.return_value = response
|
||||
agent._anthropic_client.messages.stream.return_value = stream_cm
|
||||
|
||||
with patch.object(agent, "_try_refresh_anthropic_client_credentials", return_value=True) as refresh:
|
||||
result = agent._anthropic_messages_create({"model": "claude-sonnet-4-20250514"})
|
||||
|
||||
refresh.assert_called_once_with()
|
||||
agent._anthropic_client.messages.stream.assert_called_once_with(model="claude-sonnet-4-20250514")
|
||||
agent._anthropic_client.messages.create.assert_not_called()
|
||||
assert result is response
|
||||
|
||||
def test_anthropic_messages_create_falls_back_when_stream_unavailable(self):
|
||||
with (
|
||||
patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search")),
|
||||
patch("run_agent.check_toolset_requirements", return_value={}),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()),
|
||||
):
|
||||
agent = AIAgent(
|
||||
api_key="sk-ant-oat01-current-token",
|
||||
base_url="https://openrouter.ai/api/v1",
|
||||
api_mode="anthropic_messages",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
|
||||
response = SimpleNamespace(content=[])
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._anthropic_client.messages.stream.side_effect = RuntimeError(
|
||||
"stream is not supported by this provider"
|
||||
)
|
||||
agent._anthropic_client.messages.create.return_value = response
|
||||
|
||||
with patch.object(agent, "_try_refresh_anthropic_client_credentials", return_value=False):
|
||||
result = agent._anthropic_messages_create({"model": "claude-sonnet-4-20250514"})
|
||||
|
||||
agent._anthropic_client.messages.stream.assert_called_once_with(model="claude-sonnet-4-20250514")
|
||||
agent._anthropic_client.messages.create.assert_called_once_with(model="claude-sonnet-4-20250514")
|
||||
assert result is response
|
||||
|
||||
def test_anthropic_messages_create_honors_disable_streaming(self):
|
||||
with (
|
||||
patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search")),
|
||||
patch("run_agent.check_toolset_requirements", return_value={}),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()),
|
||||
):
|
||||
agent = AIAgent(
|
||||
api_key="sk-ant-oat01-current-token",
|
||||
base_url="https://openrouter.ai/api/v1",
|
||||
api_mode="anthropic_messages",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
|
||||
response = SimpleNamespace(content=[])
|
||||
agent._disable_streaming = True
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._anthropic_client.messages.create.return_value = response
|
||||
|
||||
with patch.object(agent, "_try_refresh_anthropic_client_credentials", return_value=False):
|
||||
result = agent._anthropic_messages_create({"model": "claude-sonnet-4-20250514"})
|
||||
|
||||
agent._anthropic_client.messages.stream.assert_not_called()
|
||||
agent._anthropic_client.messages.create.assert_called_once_with(model="claude-sonnet-4-20250514")
|
||||
assert result is response
|
||||
|
||||
def test_anthropic_messages_create_does_not_mask_bedrock_stream_validation_errors(self):
|
||||
with (
|
||||
patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search")),
|
||||
patch("run_agent.check_toolset_requirements", return_value={}),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()),
|
||||
):
|
||||
agent = AIAgent(
|
||||
api_key="sk-ant-oat01-current-token",
|
||||
base_url="https://bedrock-runtime.us-east-1.amazonaws.com",
|
||||
api_mode="anthropic_messages",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
|
||||
exc = RuntimeError("ValidationException: InvokeModelWithResponseStream input malformed")
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._anthropic_client.messages.stream.side_effect = exc
|
||||
|
||||
with (
|
||||
patch.object(agent, "_try_refresh_anthropic_client_credentials", return_value=False),
|
||||
pytest.raises(RuntimeError, match="input malformed"),
|
||||
):
|
||||
agent._anthropic_messages_create({"model": "claude-sonnet-4-20250514"})
|
||||
|
||||
agent._anthropic_client.messages.create.assert_not_called()
|
||||
|
||||
def test_anthropic_messages_create_falls_back_for_bedrock_stream_access_denied(self):
|
||||
with (
|
||||
patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search")),
|
||||
patch("run_agent.check_toolset_requirements", return_value={}),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()),
|
||||
):
|
||||
agent = AIAgent(
|
||||
api_key="sk-ant-oat01-current-token",
|
||||
base_url="https://bedrock-runtime.us-east-1.amazonaws.com",
|
||||
api_mode="anthropic_messages",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
|
||||
response = SimpleNamespace(content=[])
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._anthropic_client.messages.stream.side_effect = RuntimeError(
|
||||
"User is not authorized to perform: bedrock:InvokeModelWithResponseStream"
|
||||
)
|
||||
agent._anthropic_client.messages.create.return_value = response
|
||||
|
||||
with patch.object(agent, "_try_refresh_anthropic_client_credentials", return_value=False):
|
||||
result = agent._anthropic_messages_create({"model": "claude-sonnet-4-20250514"})
|
||||
|
||||
agent._anthropic_client.messages.create.assert_called_once_with(model="claude-sonnet-4-20250514")
|
||||
assert result is response
|
||||
|
||||
|
|
|
|||
|
|
@ -2065,6 +2065,89 @@ class TestSessionTitle:
|
|||
assert session["ended_at"] is not None
|
||||
|
||||
|
||||
class TestSessionTitleLineage:
|
||||
"""Renaming a compression continuation back to its base title must succeed
|
||||
by transferring the title off the ended, hidden predecessor.
|
||||
|
||||
After a context compaction the original session is ended and projected
|
||||
behind its live tip in the session list (list_sessions_rich), so the user
|
||||
cannot see or free it. Without lineage-aware handling, renaming the visible
|
||||
tip back to the base name dead-ends with "already in use by <session they
|
||||
can't find>".
|
||||
"""
|
||||
|
||||
def _make_compression_chain(self, db, t0, *, root="root", tip="tip"):
|
||||
db.create_session(root, "cli")
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0, root))
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET ended_at=?, end_reason='compression' WHERE id=?",
|
||||
(t0 + 100, root),
|
||||
)
|
||||
db.create_session(tip, "cli", parent_session_id=root)
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0 + 200, tip))
|
||||
db._conn.commit()
|
||||
|
||||
def test_rename_continuation_back_to_base_transfers_title(self, db):
|
||||
import time as _time
|
||||
self._make_compression_chain(db, _time.time() - 3600)
|
||||
db.set_session_title("root", "fingerprint-scanner")
|
||||
db.set_session_title("tip", "fingerprint-scanner #2")
|
||||
|
||||
# User renames the visible tip back to the base name — must succeed.
|
||||
assert db.set_session_title("tip", "fingerprint-scanner") is True
|
||||
assert db.get_session("tip")["title"] == "fingerprint-scanner"
|
||||
# Title transferred off the hidden ancestor — no duplicate titles.
|
||||
assert db.get_session("root")["title"] is None
|
||||
|
||||
def test_transfer_walks_multi_level_chain(self, db):
|
||||
import time as _time
|
||||
t0 = _time.time() - 7200
|
||||
# root (compression) -> mid (compression) -> tip
|
||||
self._make_compression_chain(db, t0, root="root", tip="mid")
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET ended_at=?, end_reason='compression' WHERE id=?",
|
||||
(t0 + 300, "mid"),
|
||||
)
|
||||
db.create_session("tip", "cli", parent_session_id="mid")
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0 + 400, "tip"))
|
||||
db._conn.commit()
|
||||
|
||||
db.set_session_title("root", "deep-dive")
|
||||
assert db.set_session_title("tip", "deep-dive") is True
|
||||
assert db.get_session("tip")["title"] == "deep-dive"
|
||||
assert db.get_session("root")["title"] is None
|
||||
|
||||
def test_unrelated_session_still_conflicts(self, db):
|
||||
db.create_session("a", "cli")
|
||||
db.create_session("b", "cli")
|
||||
db.set_session_title("a", "shared")
|
||||
with pytest.raises(ValueError, match="already in use"):
|
||||
db.set_session_title("b", "shared")
|
||||
# The unrelated holder keeps its title.
|
||||
assert db.get_session("a")["title"] == "shared"
|
||||
|
||||
def test_non_compression_child_still_conflicts(self, db):
|
||||
"""A child whose parent did NOT end via compression (delegate/branch
|
||||
spawned while the parent was live) is not a continuation, so renaming it
|
||||
to the parent's title must still raise."""
|
||||
import time as _time
|
||||
t0 = _time.time() - 3600
|
||||
db.create_session("parent", "cli")
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0, "parent"))
|
||||
db.create_session("child", "cli", parent_session_id="parent")
|
||||
# Child started BEFORE parent ended, and parent ended for a non-
|
||||
# compression reason — not a continuation edge.
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0 + 10, "child"))
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET ended_at=?, end_reason='user_exit' WHERE id=?",
|
||||
(t0 + 100, "parent"),
|
||||
)
|
||||
db._conn.commit()
|
||||
db.set_session_title("parent", "shared")
|
||||
with pytest.raises(ValueError, match="already in use"):
|
||||
db.set_session_title("child", "shared")
|
||||
|
||||
|
||||
class TestSanitizeTitle:
|
||||
"""Tests for SessionDB.sanitize_title() validation and cleaning."""
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from tools.clarify_tool import (
|
|||
check_clarify_requirements,
|
||||
MAX_CHOICES,
|
||||
CLARIFY_SCHEMA,
|
||||
_flatten_choice,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -164,6 +165,70 @@ class TestCheckClarifyRequirements:
|
|||
assert check_clarify_requirements() is True
|
||||
|
||||
|
||||
class TestClarifyDictChoices:
|
||||
"""Dict-shaped choices must be unwrapped to user-facing text at the source.
|
||||
|
||||
LLMs sometimes emit [{"description": "..."}] instead of bare strings. The
|
||||
naive str(c) coercion leaked the Python dict repr onto every surface (CLI
|
||||
panel, Discord buttons, Telegram list) AND returned it verbatim as the
|
||||
user's answer. _flatten_choice normalises at the one platform-agnostic
|
||||
entry point so the whole class is fixed in one place.
|
||||
"""
|
||||
|
||||
def test_flatten_unwraps_label_first(self):
|
||||
assert _flatten_choice({"label": "Short", "description": "Long"}) == "Short"
|
||||
|
||||
def test_flatten_unwraps_description_when_no_label(self):
|
||||
assert _flatten_choice({"description": "A loose layout"}) == "A loose layout"
|
||||
|
||||
def test_flatten_unwrap_order_label_over_description(self):
|
||||
assert _flatten_choice({"description": "verbose", "label": "tight"}) == "tight"
|
||||
|
||||
def test_flatten_drops_name_value_only_dict(self):
|
||||
# name/value are component-shaped fields, not user-facing labels —
|
||||
# picking them would leak raw enum values / short model ids.
|
||||
assert _flatten_choice({"name": "tight", "value": "x"}) == ""
|
||||
|
||||
def test_flatten_prefers_canonical_key_over_name(self):
|
||||
assert _flatten_choice({"name": "tight", "description": "Tight desc"}) == "Tight desc"
|
||||
|
||||
def test_flatten_drops_keyless_dict(self):
|
||||
assert _flatten_choice({"foo": "bar", "n": 1}) == ""
|
||||
|
||||
def test_flatten_passthrough_string_and_scalar(self):
|
||||
assert _flatten_choice("plain") == "plain"
|
||||
assert _flatten_choice(7) == "7"
|
||||
assert _flatten_choice(None) == ""
|
||||
|
||||
def test_dict_choices_reach_callback_as_clean_text(self):
|
||||
"""The whole point: the UI callback never sees a dict repr."""
|
||||
seen = []
|
||||
|
||||
def cb(question, choices):
|
||||
seen.extend(choices or [])
|
||||
return choices[0]
|
||||
|
||||
result = json.loads(clarify_tool(
|
||||
"Pick a layout",
|
||||
choices=[
|
||||
{"choice": "Tight", "description": "Tight, covers all 3 points"},
|
||||
{"description": "Loose layout"},
|
||||
{"name": "modelid", "value": "abc"}, # dropped, not leaked
|
||||
"A plain string choice",
|
||||
],
|
||||
callback=cb,
|
||||
)) # type: ignore
|
||||
assert seen == [
|
||||
"Tight, covers all 3 points",
|
||||
"Loose layout",
|
||||
"A plain string choice",
|
||||
]
|
||||
# and the resolved answer is clean text, not a dict repr
|
||||
assert result["user_response"] == "Tight, covers all 3 points"
|
||||
assert "{" not in result["user_response"]
|
||||
assert all("{" not in c for c in result["choices_offered"])
|
||||
|
||||
|
||||
class TestClarifySchema:
|
||||
"""Tests for the OpenAI function-calling schema."""
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ without requiring the ``piper-tts`` package to actually be installed
|
|||
|
||||
import json
|
||||
import sys
|
||||
import types
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
|
@ -219,7 +220,7 @@ class TestGeneratePiperTts:
|
|||
|
||||
# The SynthesisConfig import happens inline inside _generate_piper_tts
|
||||
# via ``from piper import SynthesisConfig``. Inject a fake piper
|
||||
# module so that import resolves.
|
||||
# module so that that import resolves.
|
||||
monkeypatch.setitem(sys.modules, "piper", FakePiperModule)
|
||||
|
||||
config = {
|
||||
|
|
@ -239,6 +240,96 @@ class TestGeneratePiperTts:
|
|||
assert kwargs["length_scale"] == 2.0
|
||||
assert kwargs["volume"] == 0.8
|
||||
|
||||
def test_speaker_id_passed_through_to_synconfig(self, tmp_path, monkeypatch):
|
||||
"""speaker_id flows from config to SynthesisConfig when set."""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
fake_syn_cls = MagicMock()
|
||||
monkeypatch.setitem(sys.modules, "piper", types.SimpleNamespace(SynthesisConfig=fake_syn_cls))
|
||||
|
||||
config = {"piper": {"voice": str(model), "speaker_id": 2}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / "out.wav"), config)
|
||||
|
||||
fake_syn_cls.assert_called_once()
|
||||
assert fake_syn_cls.call_args.kwargs["speaker_id"] == 2
|
||||
|
||||
def test_speaker_id_alone_triggers_synconfig(self, tmp_path, monkeypatch):
|
||||
"""Setting ONLY speaker_id (no other advanced knobs) still constructs SynthesisConfig.
|
||||
|
||||
Regression guard: has_advanced must include speaker_id, otherwise
|
||||
this knob gets silently dropped on the simplest configuration.
|
||||
"""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
fake_syn_cls = MagicMock()
|
||||
monkeypatch.setitem(sys.modules, "piper", types.SimpleNamespace(SynthesisConfig=fake_syn_cls))
|
||||
|
||||
config = {"piper": {"voice": str(model), "speaker_id": 1}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / "out.wav"), config)
|
||||
|
||||
fake_syn_cls.assert_called_once()
|
||||
|
||||
def test_speaker_id_default_zero_when_unset(self, tmp_path, monkeypatch):
|
||||
"""No speaker_id in config → SynthesisConfig.speaker_id == 0 (Piper's default)."""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
fake_syn_cls = MagicMock()
|
||||
monkeypatch.setitem(sys.modules, "piper", types.SimpleNamespace(SynthesisConfig=fake_syn_cls))
|
||||
|
||||
config = {"piper": {"voice": str(model), "length_scale": 1.5}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / "out.wav"), config)
|
||||
|
||||
assert fake_syn_cls.call_args.kwargs["speaker_id"] == 0
|
||||
|
||||
def test_speaker_id_bool_rejected_to_zero(self, tmp_path, monkeypatch):
|
||||
"""True/False would coerce to 1/0 and hide a config mistake — reject outright."""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
fake_syn_cls = MagicMock()
|
||||
monkeypatch.setitem(sys.modules, "piper", types.SimpleNamespace(SynthesisConfig=fake_syn_cls))
|
||||
|
||||
for bad in (True, False):
|
||||
fake_syn_cls.reset_mock()
|
||||
config = {"piper": {"voice": str(model), "speaker_id": bad}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / f"out-{bad}.wav"), config)
|
||||
assert fake_syn_cls.call_args.kwargs["speaker_id"] == 0
|
||||
|
||||
def test_speaker_id_non_int_dropped_to_zero(self, tmp_path, monkeypatch):
|
||||
"""Unparseable config (string, list, dict) drops to 0 instead of raising."""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
fake_syn_cls = MagicMock()
|
||||
monkeypatch.setitem(sys.modules, "piper", types.SimpleNamespace(SynthesisConfig=fake_syn_cls))
|
||||
|
||||
for bad in ("two", [1, 2], {"k": 1}, None):
|
||||
fake_syn_cls.reset_mock()
|
||||
config = {"piper": {"voice": str(model), "speaker_id": bad}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / f"out-{type(bad).__name__}.wav"), config)
|
||||
assert fake_syn_cls.call_args.kwargs["speaker_id"] == 0
|
||||
|
||||
def test_speaker_id_does_not_invalidate_voice_cache(self, tmp_path, monkeypatch):
|
||||
"""Switching speaker_id between calls must NOT trigger a model reload.
|
||||
|
||||
PiperVoice is bound to a model, not a speaker — speaker is applied
|
||||
per-call via syn_config.speaker_id. The voice cache should serve the
|
||||
same PiperVoice instance for the same (model, cuda) regardless of
|
||||
how many distinct speaker_ids the user cycles through.
|
||||
"""
|
||||
model = self._prepare_voice_files(tmp_path)
|
||||
monkeypatch.setattr(tts_tool, "_import_piper", lambda: _StubPiperVoice)
|
||||
|
||||
for speaker in (0, 1, 2, 3):
|
||||
config = {"piper": {"voice": str(model), "speaker_id": speaker}}
|
||||
tts_tool._generate_piper_tts("hi", str(tmp_path / f"out-{speaker}.wav"), config)
|
||||
|
||||
# Only one PiperVoice.load() call across four calls with different speakers.
|
||||
assert _StubPiperVoice.loaded == [str(model)]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# text_to_speech_tool end-to-end (provider == "piper")
|
||||
|
|
|
|||
|
|
@ -1,8 +1,16 @@
|
|||
"""Tests for xAI TTS speech-tag handling."""
|
||||
|
||||
from unittest.mock import Mock
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
from tools.tts_tool import _apply_xai_auto_speech_tags, _generate_xai_tts
|
||||
import pytest
|
||||
|
||||
from tools.tts_tool import (
|
||||
_XAI_INLINE_SPEECH_TAGS,
|
||||
_XAI_WRAPPING_SPEECH_TAGS,
|
||||
_apply_xai_auto_speech_tags,
|
||||
_generate_xai_tts,
|
||||
)
|
||||
|
||||
|
||||
def test_apply_xai_auto_speech_tags_adds_light_pause_after_first_sentence():
|
||||
|
|
@ -72,8 +80,20 @@ def test_apply_xai_auto_speech_tags_single_newline_still_gets_first_sentence_pau
|
|||
)
|
||||
|
||||
|
||||
def test_generate_xai_tts_sends_auto_speech_tags_when_enabled(tmp_path, monkeypatch):
|
||||
def test_generate_xai_tts_sends_auxiliary_rewriter_output_to_api(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""auto_speech_tags=True should send the auxiliary rewriter's tagged
|
||||
output (not the conservative local pause fallback) to the xAI TTS API.
|
||||
|
||||
The previous version of this test asserted on the local pause-tagged
|
||||
text — which only happened to match because ``call_llm`` returns
|
||||
``None`` in the test environment and the function silently fell
|
||||
back. With the new auxiliary-rewrite path the user-visible contract
|
||||
is "what the LLM said wins", so this test pins that down.
|
||||
"""
|
||||
captured = {}
|
||||
rewriter_output = "Bonjour Monsieur Talbot. [warmly] Ceci est un test. [soft laugh]"
|
||||
|
||||
class FakeResponse:
|
||||
content = b"mp3"
|
||||
|
|
@ -88,8 +108,15 @@ def test_generate_xai_tts_sends_auto_speech_tags_when_enabled(tmp_path, monkeypa
|
|||
captured["timeout"] = timeout
|
||||
return FakeResponse()
|
||||
|
||||
fake_response = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content=rewriter_output))]
|
||||
)
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
monkeypatch.setattr(
|
||||
"agent.auxiliary_client.call_llm", lambda *a, **kw: fake_response
|
||||
)
|
||||
|
||||
out = tmp_path / "out.mp3"
|
||||
_generate_xai_tts(
|
||||
|
|
@ -102,7 +129,178 @@ def test_generate_xai_tts_sends_auto_speech_tags_when_enabled(tmp_path, monkeypa
|
|||
assert captured["url"] == "https://api.x.ai/v1/tts"
|
||||
assert captured["json"]["voice_id"] == "ara"
|
||||
assert captured["json"]["language"] == "fr"
|
||||
assert captured["json"]["text"] == "Bonjour Monsieur Talbot. [pause] Ceci est un test."
|
||||
assert captured["json"]["text"] == rewriter_output
|
||||
|
||||
|
||||
def test_auto_speech_tags_calls_auxiliary_rewriter_with_tts_audio_tags_task():
|
||||
"""When input has no explicit speech tags, the function must call the
|
||||
auxiliary rewriter with task='tts_audio_tags' and a system prompt
|
||||
that documents the xAI inline + wrapping tag vocabulary.
|
||||
"""
|
||||
response = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content="[warmly] Hi."))]
|
||||
)
|
||||
|
||||
with patch("agent.auxiliary_client.call_llm", return_value=response) as mock_call:
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
assert result == "[warmly] Hi."
|
||||
mock_call.assert_called_once()
|
||||
call_kwargs = mock_call.call_args.kwargs
|
||||
assert call_kwargs["task"] == "tts_audio_tags"
|
||||
assert call_kwargs["temperature"] == 0.7
|
||||
|
||||
messages = call_kwargs["messages"]
|
||||
assert messages[0]["role"] == "system"
|
||||
assert messages[1]["role"] == "user"
|
||||
|
||||
system_prompt = messages[0]["content"]
|
||||
# All documented inline + wrapping tag names must appear in the prompt
|
||||
# so the auxiliary model knows what's valid. The prompt lists them
|
||||
# comma-separated in two example lines ("Valid inline tags (use as
|
||||
# `[tag]`): pause, long-pause, ..." and a similar line for wrapping).
|
||||
for tag in _XAI_INLINE_SPEECH_TAGS:
|
||||
assert tag in system_prompt, (
|
||||
f"inline tag {tag!r} missing from system prompt"
|
||||
)
|
||||
for tag in _XAI_WRAPPING_SPEECH_TAGS:
|
||||
assert tag in system_prompt, (
|
||||
f"wrapping tag {tag!r} missing from system prompt"
|
||||
)
|
||||
# The prompt must explicitly show the BBCode-style closing syntax so
|
||||
# the rewriter uses [/tag] and not <tag>...</tag>.
|
||||
assert "[/tag]" in system_prompt
|
||||
|
||||
# The user message carries the locally pause-tagged transcript (the
|
||||
# conservative fallback the rewriter is asked to enrich).
|
||||
assert "TRANSCRIPT TO TAG" in messages[1]["content"]
|
||||
assert "[pause]" in messages[1]["content"]
|
||||
|
||||
|
||||
def test_auto_speech_tags_strips_markdown_fences_from_rewriter_output():
|
||||
"""If the auxiliary model wraps its reply in ```...``` fences the
|
||||
function must strip them before returning.
|
||||
"""
|
||||
fenced = "```\n[warmly] Bonjour. [soft laugh]\n```"
|
||||
response = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content=fenced))]
|
||||
)
|
||||
|
||||
with patch("agent.auxiliary_client.call_llm", return_value=response):
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
assert result == "[warmly] Bonjour. [soft laugh]"
|
||||
|
||||
|
||||
def test_auto_speech_tags_strips_markdown_fence_with_language_hint():
|
||||
"""The fence regex accepts an optional language tag like ```text ...```."""
|
||||
fenced = "```text\n[warmly] Bonjour.\n```"
|
||||
response = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content=fenced))]
|
||||
)
|
||||
|
||||
with patch("agent.auxiliary_client.call_llm", return_value=response):
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
assert result == "[warmly] Bonjour."
|
||||
|
||||
|
||||
def test_auto_speech_tags_falls_back_to_local_on_auxiliary_exception(caplog):
|
||||
"""If the auxiliary rewriter raises (timeout, network, provider error,
|
||||
anything) the function must silently fall back to the local
|
||||
pause-tagged text so the user still gets audio.
|
||||
"""
|
||||
import logging
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="tools.tts_tool"), patch(
|
||||
"agent.auxiliary_client.call_llm",
|
||||
side_effect=RuntimeError("upstream provider timed out"),
|
||||
):
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
# Local fallback: first sentence gets a [pause] inserted, single
|
||||
# paragraph, no other rewriter activity.
|
||||
assert result == (
|
||||
"Bonjour Monsieur Talbot. [pause] Ceci est un test de réponse vocale."
|
||||
)
|
||||
assert "xAI TTS audio tag rewrite failed" in caplog.text
|
||||
|
||||
|
||||
def test_auto_speech_tags_falls_back_to_local_when_rewriter_returns_empty():
|
||||
"""An empty / None rewriter response must also fall back to local."""
|
||||
empty_response = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content=""))]
|
||||
)
|
||||
|
||||
with patch(
|
||||
"agent.auxiliary_client.call_llm", return_value=empty_response
|
||||
):
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
assert result == (
|
||||
"Bonjour Monsieur Talbot. [pause] Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
|
||||
def test_auto_speech_tags_skips_auxiliary_when_input_has_explicit_tags():
|
||||
"""If the user/model already supplied explicit speech tags we trust
|
||||
them and never call the rewriter — that would risk the rewriter
|
||||
overwriting intentional markup.
|
||||
"""
|
||||
tagged = "Bonjour. [pause] <whisper>Déjà balisé.</whisper>"
|
||||
|
||||
with patch("agent.auxiliary_client.call_llm") as mock_call:
|
||||
result = _apply_xai_auto_speech_tags(tagged)
|
||||
|
||||
mock_call.assert_not_called()
|
||||
# The local pass is a no-op for already-tagged text (no double
|
||||
# paragraph normalization, no first-sentence pause injection).
|
||||
assert result == tagged
|
||||
|
||||
|
||||
def test_auto_speech_tags_skips_auxiliary_for_empty_input():
|
||||
with patch("agent.auxiliary_client.call_llm") as mock_call:
|
||||
assert _apply_xai_auto_speech_tags("") == ""
|
||||
assert _apply_xai_auto_speech_tags(" \n ") == " \n "
|
||||
|
||||
mock_call.assert_not_called()
|
||||
|
||||
|
||||
def test_auto_speech_tags_skips_auxiliary_for_whitespace_only_input():
|
||||
"""Whitespace-only input short-circuits before the rewriter runs."""
|
||||
with patch("agent.auxiliary_client.call_llm") as mock_call:
|
||||
assert _apply_xai_auto_speech_tags(" ") == " "
|
||||
|
||||
mock_call.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad_response", [None, SimpleNamespace(choices=[])])
|
||||
def test_auto_speech_tags_falls_back_to_local_on_malformed_rewriter_response(
|
||||
bad_response,
|
||||
):
|
||||
"""Both ``None`` and a response with no choices must fall back to the
|
||||
conservative local pass rather than crash.
|
||||
"""
|
||||
with patch(
|
||||
"agent.auxiliary_client.call_llm", return_value=bad_response
|
||||
):
|
||||
result = _apply_xai_auto_speech_tags(
|
||||
"Bonjour Monsieur Talbot. Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
assert result == (
|
||||
"Bonjour Monsieur Talbot. [pause] Ceci est un test de réponse vocale."
|
||||
)
|
||||
|
||||
|
||||
def test_generate_xai_tts_leaves_text_plain_by_default(tmp_path, monkeypatch):
|
||||
|
|
@ -126,3 +324,207 @@ def test_generate_xai_tts_leaves_text_plain_by_default(tmp_path, monkeypatch):
|
|||
)
|
||||
|
||||
assert captured["json"]["text"] == "Bonjour Monsieur Talbot. Ceci est un test."
|
||||
|
||||
|
||||
def test_generate_xai_tts_omits_speed_and_latency_by_default(tmp_path, monkeypatch):
|
||||
"""No speed / optimize_streaming_latency in the request body unless
|
||||
the user explicitly sets them. Keeps the existing minimal-payload
|
||||
contract for default configs.
|
||||
"""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello world.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "ara", "language": "en"}},
|
||||
)
|
||||
|
||||
assert "speed" not in captured["json"]
|
||||
assert "optimize_streaming_latency" not in captured["json"]
|
||||
|
||||
|
||||
def test_generate_xai_tts_sends_speed_when_set(tmp_path, monkeypatch):
|
||||
"""tts.xai.speed flows into the POST body."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello world.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "ara", "language": "en", "speed": 1.5}},
|
||||
)
|
||||
|
||||
assert captured["json"]["speed"] == 1.5
|
||||
|
||||
|
||||
def test_generate_xai_tts_speed_clamped_to_valid_range(tmp_path, monkeypatch):
|
||||
"""speed values outside xAI's 0.7..1.5 band are clamped, not sent raw."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
# Below 0.7 -> 0.7
|
||||
_generate_xai_tts(
|
||||
"Hello.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "eve", "language": "en", "speed": 0.1}},
|
||||
)
|
||||
assert captured["json"]["speed"] == 0.7
|
||||
|
||||
# Above 1.5 -> 1.5
|
||||
_generate_xai_tts(
|
||||
"Hello.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "eve", "language": "en", "speed": 3.0}},
|
||||
)
|
||||
assert captured["json"]["speed"] == 1.5
|
||||
|
||||
|
||||
def test_generate_xai_tts_omits_speed_when_exactly_default(tmp_path, monkeypatch):
|
||||
"""speed == 1.0 is the API default; the field stays out of the payload."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "eve", "language": "en", "speed": 1.0}},
|
||||
)
|
||||
|
||||
assert "speed" not in captured["json"]
|
||||
|
||||
|
||||
def test_generate_xai_tts_sends_optimize_streaming_latency_when_set(tmp_path, monkeypatch):
|
||||
"""tts.xai.optimize_streaming_latency flows into the POST body."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello world.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "ara", "language": "en", "optimize_streaming_latency": 2}},
|
||||
)
|
||||
|
||||
assert captured["json"]["optimize_streaming_latency"] == 2
|
||||
|
||||
|
||||
def test_generate_xai_tts_optimize_streaming_latency_omitted_at_default(tmp_path, monkeypatch):
|
||||
"""optimize_streaming_latency == 0 is the API default; field is not sent."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello world.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"xai": {"voice_id": "ara", "language": "en", "optimize_streaming_latency": 0}},
|
||||
)
|
||||
|
||||
assert "optimize_streaming_latency" not in captured["json"]
|
||||
|
||||
|
||||
def test_generate_xai_tts_global_speed_used_as_fallback(tmp_path, monkeypatch):
|
||||
"""Global tts.speed is the fallback when tts.xai.speed is unset."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"speed": 0.8, "xai": {"voice_id": "ara", "language": "en"}},
|
||||
)
|
||||
|
||||
assert captured["json"]["speed"] == 0.8
|
||||
|
||||
|
||||
def test_generate_xai_tts_provider_speed_overrides_global(tmp_path, monkeypatch):
|
||||
"""tts.xai.speed wins over the global tts.speed fallback."""
|
||||
captured = {}
|
||||
|
||||
fake_response = Mock()
|
||||
fake_response.content = b"mp3"
|
||||
fake_response.raise_for_status.return_value = None
|
||||
|
||||
def fake_post(url, headers, json, timeout):
|
||||
captured["json"] = json
|
||||
return fake_response
|
||||
|
||||
monkeypatch.setenv("XAI_API_KEY", "test-xai-key")
|
||||
monkeypatch.setattr("requests.post", fake_post)
|
||||
|
||||
_generate_xai_tts(
|
||||
"Hello.",
|
||||
str(tmp_path / "out.mp3"),
|
||||
{"speed": 1.5, "xai": {"voice_id": "ara", "language": "en", "speed": 0.7}},
|
||||
)
|
||||
|
||||
assert captured["json"]["speed"] == 0.7
|
||||
|
|
|
|||
|
|
@ -185,15 +185,17 @@ def test_goal_requires_session(server):
|
|||
# ── slash.exec /goal routing ──────────────────────────────────────────
|
||||
|
||||
|
||||
def test_slash_exec_rejects_goal_routes_to_command_dispatch(server, session):
|
||||
"""slash.exec must reject /goal with 4018 so the TUI client falls through
|
||||
to command.dispatch. Without this, the HermesCLI slash-worker subprocess
|
||||
would set the goal but silently drop the kickoff — the queue is in-proc."""
|
||||
def test_slash_exec_routes_goal_to_command_dispatch(server, session):
|
||||
"""slash.exec must route /goal directly to command.dispatch internally
|
||||
instead of returning an error. Previously the 4018 error required the
|
||||
TUI client to retry via command.dispatch, but some clients failed the
|
||||
fallback, leaving the command empty ("empty command")."""
|
||||
sid, _, _ = session
|
||||
r = _call(server, "slash.exec", command="goal status", session_id=sid)
|
||||
assert "error" in r
|
||||
assert r["error"]["code"] == 4018
|
||||
assert "command.dispatch" in r["error"]["message"]
|
||||
# Should succeed by routing to command.dispatch internally
|
||||
assert "result" in r
|
||||
assert r["result"]["type"] == "exec"
|
||||
assert "No active goal" in r["result"]["output"]
|
||||
|
||||
|
||||
def test_pending_input_commands_includes_goal(server):
|
||||
|
|
|
|||
|
|
@ -443,7 +443,9 @@ def test_apply_model_switch_does_not_leak_process_env():
|
|||
|
||||
with (
|
||||
patch("hermes_cli.model_switch.parse_model_flags",
|
||||
return_value=("glm-5.1", None, False, False)),
|
||||
return_value=("glm-5.1", None, False, False, True)),
|
||||
patch("hermes_cli.model_switch.resolve_persist_behavior",
|
||||
return_value=False),
|
||||
patch("hermes_cli.model_switch.switch_model", return_value=_FakeResult()),
|
||||
patch("tui_gateway.server._emit"),
|
||||
patch("tui_gateway.server._restart_slash_worker"),
|
||||
|
|
|
|||
|
|
@ -1121,20 +1121,45 @@ def test_slash_exec_plugin_handler_error_returns_output(server):
|
|||
|
||||
|
||||
@pytest.mark.parametrize("cmd", ["retry", "queue hello", "q hello", "steer fix the test", "plan"])
|
||||
def test_slash_exec_rejects_pending_input_commands(server, cmd):
|
||||
"""slash.exec must reject commands that use _pending_input in the CLI."""
|
||||
sid = "test-session"
|
||||
server._sessions[sid] = {"session_key": sid, "agent": None}
|
||||
def test_slash_exec_routes_pending_input_commands_to_dispatch(server, cmd):
|
||||
"""slash.exec must route _pending_input commands to command.dispatch
|
||||
internally instead of returning the old 4018 "use command.dispatch"
|
||||
fallback error (#48848). Some TUI clients failed that client-side
|
||||
fallback, dropping the input and surfacing "empty command".
|
||||
|
||||
resp = server.handle_request({
|
||||
The contract is that slash.exec produces exactly the response
|
||||
command.dispatch would for the same command — no fragile retry hop.
|
||||
"""
|
||||
base, _, arg = cmd.partition(" ")
|
||||
|
||||
def fresh_session():
|
||||
return {"session_key": "test-session", "agent": None}
|
||||
|
||||
sid = "test-session"
|
||||
|
||||
# Response from the (new) internal routing in slash.exec.
|
||||
server._sessions[sid] = fresh_session()
|
||||
routed = server.handle_request({
|
||||
"id": "r1",
|
||||
"method": "slash.exec",
|
||||
"params": {"command": cmd, "session_id": sid},
|
||||
})
|
||||
|
||||
assert "error" in resp
|
||||
assert resp["error"]["code"] == 4018
|
||||
assert "pending-input command" in resp["error"]["message"]
|
||||
# Response from calling command.dispatch directly with the parsed parts.
|
||||
server._sessions[sid] = fresh_session()
|
||||
direct = server.handle_request({
|
||||
"id": "r1",
|
||||
"method": "command.dispatch",
|
||||
"params": {"name": base, "arg": arg, "session_id": sid},
|
||||
})
|
||||
|
||||
# slash.exec must no longer emit the old client-fallback rejection.
|
||||
if "error" in routed:
|
||||
assert "pending-input command" not in routed["error"]["message"]
|
||||
|
||||
# Internal routing must yield the same payload as command.dispatch.
|
||||
assert routed.get("result") == direct.get("result")
|
||||
assert routed.get("error") == direct.get("error")
|
||||
|
||||
|
||||
def test_command_dispatch_queue_sends_message(server):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue