mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-09-04 02:01:48 +03:00
refactor(webui): isolate websocket application orchestration (#5548)
* refactor(webui): extract session attach projection * refactor(webui): isolate websocket application orchestration * refactor(webui): tighten websocket application boundary * test(webui): assert module logger for fork failures
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
"""Executable architecture constraints for the WebSocket transport adapter."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
from nanobot.channels.websocket import runtime
|
||||
|
||||
_REPOSITORY_ROOT = Path(__file__).resolve().parents[2]
|
||||
_RUNTIME_PATH = _REPOSITORY_ROOT / "nanobot" / "channels" / "websocket" / "runtime.py"
|
||||
_SESSION_IDENTITY_PATH = _REPOSITORY_ROOT / "nanobot" / "webui" / "session_identity.py"
|
||||
_FORBIDDEN_RUNTIME_IMPORTS = (
|
||||
"nanobot.bus.outbound_events",
|
||||
"nanobot.command",
|
||||
"nanobot.runtime_context",
|
||||
"nanobot.security.workspace_access",
|
||||
"nanobot.session.goal_state",
|
||||
"nanobot.webui.cli_apps_api",
|
||||
"nanobot.webui.forking",
|
||||
"nanobot.webui.mcp_presets_api",
|
||||
"nanobot.webui.sidebar_state",
|
||||
"nanobot.webui.transcription_ws",
|
||||
)
|
||||
|
||||
|
||||
def _channel_method(name: str) -> ast.AsyncFunctionDef:
|
||||
tree = ast.parse(_RUNTIME_PATH.read_text(encoding="utf-8"))
|
||||
channel = next(
|
||||
node
|
||||
for node in tree.body
|
||||
if isinstance(node, ast.ClassDef) and node.name == "WebSocketChannel"
|
||||
)
|
||||
return next(
|
||||
node
|
||||
for node in channel.body
|
||||
if isinstance(node, ast.AsyncFunctionDef) and node.name == name
|
||||
)
|
||||
|
||||
|
||||
def _statements_without_docstring(node: ast.AsyncFunctionDef) -> list[ast.stmt]:
|
||||
body = list(node.body)
|
||||
if (
|
||||
body
|
||||
and isinstance(body[0], ast.Expr)
|
||||
and isinstance(body[0].value, ast.Constant)
|
||||
and isinstance(body[0].value.value, str)
|
||||
):
|
||||
body.pop(0)
|
||||
return body
|
||||
|
||||
|
||||
def test_websocket_runtime_does_not_import_application_command_trees() -> None:
|
||||
tree = ast.parse(_RUNTIME_PATH.read_text(encoding="utf-8"))
|
||||
imported = {
|
||||
node.module
|
||||
for node in tree.body
|
||||
if isinstance(node, ast.ImportFrom) and node.module is not None
|
||||
}
|
||||
violations = sorted(
|
||||
module
|
||||
for module in imported
|
||||
if module.startswith(_FORBIDDEN_RUNTIME_IMPORTS)
|
||||
)
|
||||
assert violations == []
|
||||
|
||||
|
||||
def test_business_entrypoints_are_thin_transport_delegations() -> None:
|
||||
for method_name in ("_dispatch_envelope", "_hydrate_after_subscribe", "send"):
|
||||
statements = _statements_without_docstring(_channel_method(method_name))
|
||||
assert len(statements) == 1, method_name
|
||||
assert isinstance(statements[0], ast.Expr), method_name
|
||||
assert isinstance(statements[0].value, ast.Await), method_name
|
||||
|
||||
statements = _statements_without_docstring(_channel_method("_dispatch_http"))
|
||||
assert len(statements) == 1
|
||||
assert isinstance(statements[0], ast.Return)
|
||||
assert isinstance(statements[0].value, ast.Await)
|
||||
|
||||
|
||||
def test_persisted_webui_session_prefix_has_one_production_owner() -> None:
|
||||
owners = []
|
||||
for path in (_REPOSITORY_ROOT / "nanobot").rglob("*.py"):
|
||||
if "tests" in path.parts or path == _SESSION_IDENTITY_PATH:
|
||||
continue
|
||||
if "websocket:" in path.read_text(encoding="utf-8"):
|
||||
owners.append(path.relative_to(_REPOSITORY_ROOT).as_posix())
|
||||
assert owners == []
|
||||
assert 'WEBUI_SESSION_STORAGE_PREFIX = "websocket:"' in _SESSION_IDENTITY_PATH.read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
|
||||
def test_runtime_exports_compatibility_protocol_helpers() -> None:
|
||||
assert runtime._is_valid_chat_id("unified:default") # pyright: ignore[reportPrivateUsage]
|
||||
assert not runtime._is_valid_chat_id("../escape") # pyright: ignore[reportPrivateUsage]
|
||||
@@ -139,8 +139,9 @@ async def test_fork_handler_maps_invalid_source_and_internal_failure_to_stable_e
|
||||
channel = SimpleNamespace(
|
||||
send_webui_protocol_error=AsyncMock(),
|
||||
gateway=SimpleNamespace(session_manager=MagicMock()),
|
||||
logger=SimpleNamespace(warning=MagicMock()),
|
||||
)
|
||||
warning = MagicMock()
|
||||
monkeypatch.setattr(forking, "logger", SimpleNamespace(warning=warning))
|
||||
envelope = {"source_chat_id": "source", "before_user_index": 0}
|
||||
monkeypatch.setattr(forking, "create_webui_chat_fork", lambda *_args, **_kwargs: None)
|
||||
|
||||
@@ -158,5 +159,5 @@ async def test_fork_handler_maps_invalid_source_and_internal_failure_to_stable_e
|
||||
)
|
||||
await forking.handle_webui_fork_chat(channel, connection, envelope)
|
||||
|
||||
channel.logger.warning.assert_called_once_with("fork_chat failed: {}", ANY)
|
||||
warning.assert_called_once_with("fork_chat failed: {}", ANY)
|
||||
channel.send_webui_protocol_error.assert_awaited_once_with(connection, "fork_chat_failed")
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
from nanobot.webui.session_identity import (
|
||||
WEBUI_SESSION_STORAGE_PREFIX,
|
||||
is_valid_webui_chat_id,
|
||||
is_webui_session_key,
|
||||
webui_chat_id,
|
||||
webui_session_key,
|
||||
)
|
||||
|
||||
|
||||
def test_webui_session_identity_preserves_persisted_wire_compatibility() -> None:
|
||||
assert WEBUI_SESSION_STORAGE_PREFIX == "websocket:"
|
||||
assert webui_session_key("chat-1") == "websocket:chat-1"
|
||||
assert is_webui_session_key("websocket:chat-1")
|
||||
assert webui_chat_id("websocket:chat-1") == "chat-1"
|
||||
assert webui_chat_id("websocket: chat-1") == " chat-1"
|
||||
assert webui_chat_id("websocket:") is None
|
||||
assert webui_chat_id("telegram:chat-1") is None
|
||||
|
||||
|
||||
def test_webui_chat_id_validation_is_protocol_scoped() -> None:
|
||||
assert is_valid_webui_chat_id("unified:default")
|
||||
assert is_valid_webui_chat_id("x" * 64)
|
||||
assert not is_valid_webui_chat_id("x" * 65)
|
||||
assert not is_valid_webui_chat_id("../escape")
|
||||
@@ -0,0 +1,114 @@
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.providers.base import LLMUsage
|
||||
from nanobot.session.model_selection import SESSION_MODEL_PRESET_METADATA_KEY
|
||||
from nanobot.session.recovery import RECOVERY_METADATA_KEY
|
||||
from nanobot.webui.session_projection import WebUISessionProjection
|
||||
|
||||
|
||||
def test_attach_fields_restore_session_runtime_metadata() -> None:
|
||||
usage = LLMUsage.reported(input_tokens=120, output_tokens=8, total_tokens=175)
|
||||
sessions = MagicMock()
|
||||
sessions.read_session_metadata.return_value = {
|
||||
"metadata": {
|
||||
SESSION_MODEL_PRESET_METADATA_KEY: "Deep Research",
|
||||
RECOVERY_METADATA_KEY: {
|
||||
"status": "recovered",
|
||||
"recovery_id": "recovery-1",
|
||||
"reason": "answer_restored",
|
||||
},
|
||||
"_last_usage": usage.to_dict(),
|
||||
}
|
||||
}
|
||||
|
||||
projection = WebUISessionProjection(sessions)
|
||||
|
||||
assert projection.attach_fields("websocket:chat-1") == {
|
||||
"model_preset": "Deep Research",
|
||||
"recovery_state": {
|
||||
"status": "recovered",
|
||||
"recovery_id": "recovery-1",
|
||||
"reason": "answer_restored",
|
||||
},
|
||||
"usage": usage.to_turn_dict(),
|
||||
}
|
||||
sessions.read_session_metadata.assert_called_once_with("websocket:chat-1")
|
||||
|
||||
|
||||
def test_attach_fields_tolerate_missing_or_invalid_session_metadata() -> None:
|
||||
sessions = MagicMock()
|
||||
sessions.read_session_metadata.return_value = {
|
||||
"metadata": {SESSION_MODEL_PRESET_METADATA_KEY: 42}
|
||||
}
|
||||
log = MagicMock()
|
||||
projection = WebUISessionProjection(sessions, log=log)
|
||||
|
||||
assert projection.attach_fields("websocket:invalid") == {"model_preset": None}
|
||||
log.warning.assert_called_once()
|
||||
assert WebUISessionProjection(None).attach_fields("websocket:missing") == {}
|
||||
|
||||
|
||||
def test_hydration_events_restore_goal_and_running_turn(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
sessions = MagicMock()
|
||||
sessions.read_session_metadata.return_value = {
|
||||
"metadata": {
|
||||
"goal_state": {
|
||||
"status": "active",
|
||||
"objective": "finish boundary split",
|
||||
"ui_summary": "Refactoring",
|
||||
}
|
||||
}
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"nanobot.webui.session_projection.websocket_turn_wall_started_at",
|
||||
lambda _chat_id: 42.5,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.webui.session_projection.websocket_turn_id",
|
||||
lambda _chat_id: "turn-1",
|
||||
)
|
||||
|
||||
events = WebUISessionProjection(sessions).hydration_events(
|
||||
"websocket:chat-1",
|
||||
"chat-1",
|
||||
)
|
||||
|
||||
assert events == (
|
||||
{
|
||||
"event": "goal_state",
|
||||
"chat_id": "chat-1",
|
||||
"goal_state": {
|
||||
"active": True,
|
||||
"status": "active",
|
||||
"ui_summary": "Refactoring",
|
||||
"objective": "finish boundary split",
|
||||
},
|
||||
},
|
||||
{
|
||||
"event": "goal_status",
|
||||
"chat_id": "chat-1",
|
||||
"status": "running",
|
||||
"started_at": 42.5,
|
||||
"turn_id": "turn-1",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def test_hydration_events_are_quiet_without_actionable_state(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
sessions = MagicMock()
|
||||
sessions.read_session_metadata.return_value = {"metadata": {}}
|
||||
monkeypatch.setattr(
|
||||
"nanobot.webui.session_projection.websocket_turn_wall_started_at",
|
||||
lambda _chat_id: None,
|
||||
)
|
||||
|
||||
assert WebUISessionProjection(sessions).hydration_events(
|
||||
"websocket:chat-1",
|
||||
"chat-1",
|
||||
) == ()
|
||||
Reference in New Issue
Block a user