Fix Hermes adapter integration and recovery
This commit is contained in:
Codex
+239
-13
@@ -13,13 +13,12 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
import pytest
|
||||
from aiohttp import web
|
||||
|
||||
# Make the plugin package importable regardless of install layout.
|
||||
import sys
|
||||
@@ -156,7 +155,6 @@ def make_server(tmp_path: Path, **overrides) -> PhebyServer:
|
||||
**overrides,
|
||||
})
|
||||
cfg.secret = overrides.get("secret", cfg.secret or "test-secret-abc123")
|
||||
from hermes_constants import get_hermes_home # conftest redirects home
|
||||
root = Path(tmp_path) / "attachments"
|
||||
server = PhebyServer(cfg, adapter=FakeAdapter())
|
||||
server.store = AttachmentStore(root=root, retention_days=7)
|
||||
@@ -248,6 +246,19 @@ class TestAuth:
|
||||
ok = await server._authenticate(client, "peer4")
|
||||
assert not ok and not client.authenticated
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_malformed_version_is_structured_error(self, tmp_path):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.ws.inbox.put_nowait(proto.encode_message({
|
||||
"type": proto.C_HELLO, "secret": "test-secret-abc123",
|
||||
"protocol_version": {"not": "an integer"}}))
|
||||
ok = await server._authenticate(client, "peer5")
|
||||
assert not ok and not client.authenticated
|
||||
error = client.ws.events()[-1]
|
||||
assert error["type"] == proto.S_ERROR
|
||||
assert error["error"]["code"] == proto.ERR_VERSION_MISMATCH
|
||||
|
||||
def test_constant_time_equals(self):
|
||||
assert constant_time_equals("abc", "abc")
|
||||
assert not constant_time_equals("abc", "abd")
|
||||
@@ -414,7 +425,8 @@ class TestAttachments:
|
||||
# ═══════════════════════════════════════════════════════════════════════════
|
||||
class TestBridge:
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_start_event_is_structured_not_text(self, tmp_path):
|
||||
async def test_tool_start_event_is_structured_not_text(self, tmp_path,
|
||||
monkeypatch):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
@@ -428,8 +440,10 @@ class TestBridge:
|
||||
enabled=True, extra={"secret": "test-secret-abc123"}))
|
||||
real._pcfg = server.config
|
||||
real._server = server
|
||||
real._loop = asyncio.get_running_loop()
|
||||
real._active_sessions = {"agent:main:pheby:dm:conv1": asyncio.Event()}
|
||||
# Tool-call dedup state is created lazily via _tool_state()
|
||||
monkeypatch.setattr(real, "_conversation_for_session_id",
|
||||
lambda _sid: "conv1")
|
||||
|
||||
from gateway.stream_events import ToolCallChunk
|
||||
marker = real.format_tool_event(
|
||||
@@ -437,21 +451,26 @@ class TestBridge:
|
||||
args={"query": "cats"}, index=0),
|
||||
mode="all")
|
||||
assert marker is None # never rendered as chat text
|
||||
await asyncio.sleep(0) # let ensure_future broadcast run
|
||||
# The display event is intentionally eaten. The authoritative hook
|
||||
# carries the real Hermes session and tool-call IDs.
|
||||
real.on_pre_tool_call(
|
||||
session_id="session-1", tool_name="web_search",
|
||||
tool_call_id="call-1", args={"query": "cats"})
|
||||
await asyncio.sleep(0)
|
||||
events = client.ws.events()
|
||||
tool_events = [e for e in events if e["type"] == proto.S_TOOL_EVENT]
|
||||
assert len(tool_events) == 1
|
||||
ev = tool_events[0]
|
||||
assert ev["tool_name"] == "web_search"
|
||||
assert ev["status"] == "running"
|
||||
assert ev["tool_call_id"]
|
||||
assert ev["tool_call_id"] == "call-1"
|
||||
assert ev["conversation_id"] == "conv1"
|
||||
# No fake prose leaked into a message event
|
||||
assert not any(e.get("type") == proto.S_MESSAGE_COMPLETE
|
||||
for e in events)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_tool_call_completion(self, tmp_path):
|
||||
async def test_post_tool_call_completion(self, tmp_path, monkeypatch):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
@@ -462,10 +481,13 @@ class TestBridge:
|
||||
real = PhebyAdapter(PlatformConfig(
|
||||
enabled=True, extra={"secret": "test-secret-abc123"}))
|
||||
real._server = server
|
||||
real._loop = asyncio.get_running_loop()
|
||||
real._active_sessions = {"agent:main:pheby:dm:conv1": asyncio.Event()}
|
||||
monkeypatch.setattr(real, "_conversation_for_session_id",
|
||||
lambda _sid: "conv1")
|
||||
|
||||
real.on_post_tool_call(
|
||||
tool_name="terminal", tool_call_id="call_9",
|
||||
session_id="session-1", tool_name="terminal", tool_call_id="call_9",
|
||||
status="ok", duration_ms=1234)
|
||||
await asyncio.sleep(0) # let ensure_future run
|
||||
ev = [e for e in client.ws.events()
|
||||
@@ -475,7 +497,8 @@ class TestBridge:
|
||||
assert ev["duration_ms"] == 1234
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approval_push_and_resolve_roundtrip(self, tmp_path):
|
||||
async def test_approval_push_and_resolve_roundtrip(self, tmp_path,
|
||||
monkeypatch):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
@@ -491,11 +514,12 @@ class TestBridge:
|
||||
assert req["choices"] == ["once", "session", "always", "deny"]
|
||||
assert req["description"] == "Destructive command"
|
||||
|
||||
# Resolve: with no real Hermes queue the resolve call fails-open to
|
||||
# accepted=False but the pending entry must be consumed either way.
|
||||
from pheby import hermes_bridge as hb
|
||||
import tools.approval
|
||||
monkeypatch.setattr(tools.approval, "resolve_gateway_approval",
|
||||
lambda *args, **kwargs: 1)
|
||||
ok = await hb.resolve_approval(req["approval_id"], "deny", None)
|
||||
assert ok is True # pending entry existed; resolution attempted
|
||||
assert ok is True
|
||||
# Double resolve → not found
|
||||
ok2 = await hb.resolve_approval(req["approval_id"], "once", None)
|
||||
assert ok2 is False
|
||||
@@ -552,6 +576,117 @@ class TestBridge:
|
||||
assert events[0]["type"] == proto.S_RUN_ACCEPTED
|
||||
assert events[1]["type"] == proto.S_MESSAGE_START
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_chat_handler_passes_server_to_bridge(self, tmp_path):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
cid = "a" * 32
|
||||
|
||||
await server._handle_chat_send(client, {
|
||||
"type": proto.C_CHAT_SEND,
|
||||
"conversation_id": cid,
|
||||
"text": "hello through WebSocket",
|
||||
}, "request-handler")
|
||||
|
||||
assert server.adapter.handled[-1].text == "hello through WebSocket"
|
||||
assert server.adapter.handled[-1].source.chat_id == cid
|
||||
assert client.ws.events()[0]["type"] == proto.S_RUN_ACCEPTED
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_chat_dispatch_failure_closes_run(self, tmp_path,
|
||||
monkeypatch):
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
server._clients["t"] = client
|
||||
cid = "b" * 32
|
||||
|
||||
async def reject(_event):
|
||||
raise RuntimeError("gateway unavailable")
|
||||
|
||||
monkeypatch.setattr(server.adapter, "handle_message", reject)
|
||||
from pheby import hermes_bridge as hb
|
||||
with pytest.raises(RuntimeError, match="gateway unavailable"):
|
||||
await hb.send_chat(server, cid, "hello", client, "request")
|
||||
|
||||
assert hb.active_run(cid) is None
|
||||
assert client.ws.events()[-1]["type"] == proto.S_RUN_FINISHED
|
||||
assert client.ws.events()[-1]["status"] == "failed"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_removes_route_instead_of_resetting(self, tmp_path,
|
||||
monkeypatch):
|
||||
server = make_server(tmp_path)
|
||||
cid = await server.router.new_conversation("Delete me")
|
||||
session_key = f"agent:main:pheby:dm:{cid}"
|
||||
|
||||
class Entry:
|
||||
display_name = "Delete me"
|
||||
|
||||
class Store:
|
||||
def __init__(self):
|
||||
self._lock = threading.Lock()
|
||||
self._entries = {session_key: Entry()}
|
||||
self.saved = False
|
||||
|
||||
def _ensure_loaded_locked(self):
|
||||
return None
|
||||
|
||||
def _save(self):
|
||||
self.saved = True
|
||||
|
||||
def peek_session_id(self, key):
|
||||
return "session-delete" if key == session_key else None
|
||||
|
||||
class DB:
|
||||
deleted = None
|
||||
|
||||
def delete_session(self, session_id):
|
||||
self.deleted = session_id
|
||||
return True
|
||||
|
||||
store, db = Store(), DB()
|
||||
from pheby import hermes_bridge as hb
|
||||
monkeypatch.setattr(hb, "_session_store", lambda: store)
|
||||
monkeypatch.setattr(hb, "_session_db", lambda: db)
|
||||
|
||||
assert await hb.delete_conversation(cid) is True
|
||||
assert db.deleted == "session-delete"
|
||||
assert session_key not in store._entries
|
||||
assert store.saved is True
|
||||
assert await server.router.get_name(cid) is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_models_snapshot_uses_current_hermes_signature(
|
||||
self, tmp_path, monkeypatch):
|
||||
make_server(tmp_path)
|
||||
captured = {}
|
||||
|
||||
def fake_list_picker_providers(**kwargs):
|
||||
captured.update(kwargs)
|
||||
return [{"slug": "test", "models": ["m1"]}]
|
||||
|
||||
from pheby import hermes_bridge as hb
|
||||
import hermes_cli.config as hermes_config
|
||||
import hermes_cli.model_switch as model_switch
|
||||
monkeypatch.setattr(hb, "_load_cfg", lambda: {
|
||||
"model": {"default": "m1", "provider": "test"},
|
||||
"providers": {"test": {"base_url": "http://example"}},
|
||||
"model_catalog": {"excluded_providers": ["hidden"]},
|
||||
})
|
||||
monkeypatch.setattr(
|
||||
hermes_config, "get_compatible_custom_providers",
|
||||
lambda _cfg: [{"name": "test", "base_url": "http://example"}])
|
||||
monkeypatch.setattr(
|
||||
model_switch, "list_picker_providers", fake_list_picker_providers)
|
||||
|
||||
snapshot = await hb.models_snapshot()
|
||||
assert snapshot["providers"][0]["slug"] == "test"
|
||||
assert captured["current_model"] == "m1"
|
||||
assert captured["excluded_providers"] == ["hidden"]
|
||||
assert captured["custom_providers"][0]["name"] == "test"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_run_interrupts_agent(self, tmp_path):
|
||||
server = make_server(tmp_path)
|
||||
@@ -565,6 +700,7 @@ class TestBridge:
|
||||
server.adapter.gateway_runner = runner
|
||||
|
||||
from pheby import hermes_bridge as hb
|
||||
hb._ACTIVE_RUNS["convX"] = {"run_id": "run-x", "started": 0}
|
||||
ok = await hb.cancel_run("convX", None)
|
||||
assert ok is True
|
||||
assert agent.interrupts # agent.interrupt called, not thread-kill
|
||||
@@ -612,6 +748,47 @@ class TestRecovery:
|
||||
assert h1["messages"] == h2["messages"]
|
||||
assert h1["conversation_id"] == h2["conversation_id"] == cid
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_open_recovers_runtime_and_attachment_state(self, tmp_path):
|
||||
server = make_server(tmp_path)
|
||||
cid = await server.router.new_conversation("Recover")
|
||||
source = Path(tmp_path) / "recovery.png"
|
||||
source.write_bytes(b"\x89PNG recovery")
|
||||
attachment = await server.store.register_file(
|
||||
str(source), conversation_id=cid)
|
||||
|
||||
from pheby import hermes_bridge as hb
|
||||
hb._ACTIVE_RUNS[cid] = {"run_id": "run-recover", "started": 1}
|
||||
hb.record_tool_event(cid, {
|
||||
"type": proto.S_TOOL_EVENT,
|
||||
"conversation_id": cid,
|
||||
"run_id": "run-recover",
|
||||
"tool_call_id": "tool-recover",
|
||||
"tool_name": "terminal",
|
||||
"status": "running",
|
||||
})
|
||||
await hb.push_approval(
|
||||
{"command": "echo hi", "description": "Run command"},
|
||||
f"agent:main:pheby:dm:{cid}")
|
||||
await hb.push_clarify(
|
||||
"clarify-recover", f"agent:main:pheby:dm:{cid}",
|
||||
"Continue?", ["yes", "no"])
|
||||
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
await server._handle_conversation_open(client, {
|
||||
"type": proto.C_CONVERSATION_OPEN,
|
||||
"conversation_id": cid,
|
||||
}, "recover")
|
||||
snapshot = client.ws.events()[-1]
|
||||
assert snapshot["attachments"][0]["attachment_id"] == \
|
||||
attachment["attachment_id"]
|
||||
assert snapshot["run"]["run_id"] == "run-recover"
|
||||
assert snapshot["tools"][0]["tool_call_id"] == "tool-recover"
|
||||
assert snapshot["approvals"][0]["conversation_id"] == cid
|
||||
assert snapshot["clarifications"][0]["clarify_id"] == \
|
||||
"clarify-recover"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broadcast_reaches_multiple_clients(self, tmp_path):
|
||||
server = make_server(tmp_path)
|
||||
@@ -756,6 +933,45 @@ class TestLiveServer:
|
||||
# Adapter unit checks
|
||||
# ═══════════════════════════════════════════════════════════════════════════
|
||||
class TestAdapterUnits:
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_preview_keeps_run_open_until_finalize(self, tmp_path):
|
||||
from pheby.adapter import PhebyAdapter
|
||||
from gateway.config import PlatformConfig
|
||||
from pheby import hermes_bridge as hb
|
||||
|
||||
server = make_server(tmp_path)
|
||||
client = FakeClientConnection()
|
||||
client.authenticated = True
|
||||
server._clients["t"] = client
|
||||
adapter = PhebyAdapter(PlatformConfig(
|
||||
enabled=True, extra={"secret": "test-secret-abc123"}))
|
||||
adapter._server = server
|
||||
adapter._loop = asyncio.get_running_loop()
|
||||
adapter._drafts["conv"] = {"message_id": "draft-run", "text": ""}
|
||||
hb.set_adapter(adapter)
|
||||
hb.set_server(server)
|
||||
hb._ACTIVE_RUNS["conv"] = {"run_id": "run-1", "started": 0}
|
||||
|
||||
first = await adapter.send("conv", "hel", metadata={"expect_edits": True})
|
||||
assert first.message_id == "draft-run"
|
||||
assert hb.active_run_id("conv") == "run-1"
|
||||
assert client.ws.events()[-1]["type"] == proto.S_MESSAGE_DELTA
|
||||
|
||||
final = await adapter.edit_message(
|
||||
"conv", "draft-run", "hello", finalize=True)
|
||||
await asyncio.sleep(0)
|
||||
assert final.message_id == "draft-run"
|
||||
assert hb.active_run_id("conv") is None
|
||||
assert [e["type"] for e in client.ws.events()][-2:] == [
|
||||
proto.S_MESSAGE_COMPLETE, proto.S_RUN_FINISHED]
|
||||
|
||||
def test_transport_auth_is_gateway_authorization(self):
|
||||
from pheby.adapter import PhebyAdapter
|
||||
from gateway.config import PlatformConfig
|
||||
adapter = PhebyAdapter(PlatformConfig(
|
||||
enabled=True, extra={"secret": "test-secret-abc123"}))
|
||||
assert adapter.authorization_is_upstream is True
|
||||
|
||||
def test_redact_args(self):
|
||||
from pheby.adapter import _redact_args
|
||||
out = _redact_args({"query": "cats", "api_key": "sk-123",
|
||||
@@ -765,6 +981,16 @@ class TestAdapterUnits:
|
||||
assert out["query"] == "cats"
|
||||
assert out["long"].endswith("…")
|
||||
|
||||
def test_redact_args_recursively(self):
|
||||
from pheby.adapter import _redact_args
|
||||
out = _redact_args({
|
||||
"headers": {"Authorization": "Bearer secret"},
|
||||
"steps": [{"password": "hunter2", "value": "safe"}],
|
||||
})
|
||||
assert out["headers"]["Authorization"] == "[redacted]"
|
||||
assert out["steps"][0]["password"] == "[redacted]"
|
||||
assert out["steps"][0]["value"] == "safe"
|
||||
|
||||
def test_sanitize_filename(self):
|
||||
from pheby.attachments import AttachmentStore
|
||||
assert AttachmentStore._sanitize_filename("../../etc/passwd") == "passwd"
|
||||
|
||||
Reference in New Issue
Block a user