mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-07 09:58:34 +00:00
refactor(webui): remove legacy session messages route
This commit is contained in:
parent
ff6deda178
commit
cdb2a474f9
@ -19,11 +19,6 @@ from nanobot.channels.websocket.runtime import WebSocketChannel, WebSocketConfig
|
||||
from nanobot.cron.service import CronService
|
||||
from nanobot.cron.types import CronJob, CronPayload, CronSchedule
|
||||
from nanobot.optional_features import InstallResult
|
||||
from nanobot.runtime_context import (
|
||||
RUNTIME_CONTEXT_HISTORY_META,
|
||||
RuntimeContextBlock,
|
||||
append_runtime_context,
|
||||
)
|
||||
from nanobot.security.workspace_access import WORKSPACE_SCOPE_METADATA_KEY
|
||||
from nanobot.session.keys import UNIFIED_SESSION_KEY
|
||||
from nanobot.session.manager import Session, SessionManager
|
||||
@ -256,7 +251,7 @@ async def test_bootstrap_returns_token_for_localhost(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sessions_routes_require_bearer_token(
|
||||
async def test_sessions_list_requires_bearer_token(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = _seed_session(tmp_path, key="websocket:abc")
|
||||
@ -278,14 +273,26 @@ async def test_sessions_routes_require_bearer_token(
|
||||
# Server stays an opaque source: filesystem paths must not leak to the wire.
|
||||
assert all("path" not in s for s in listing.json()["sessions"])
|
||||
|
||||
msgs = await _http_get(
|
||||
"http://127.0.0.1:29902/api/sessions/websocket:abc/messages",
|
||||
headers=auth,
|
||||
finally:
|
||||
await channel.stop()
|
||||
await server_task
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_legacy_session_messages_route_is_not_exposed(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = _seed_session(tmp_path, key="websocket:legacy")
|
||||
channel = _ch(bus, session_manager=sm, port=29919)
|
||||
server_task = asyncio.create_task(channel.start())
|
||||
try:
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
response = await _http_get(
|
||||
"http://127.0.0.1:29919/api/sessions/websocket:legacy/messages",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
assert msgs.status_code == 200
|
||||
body = msgs.json()
|
||||
assert body["key"] == "websocket:abc"
|
||||
assert [m["role"] for m in body["messages"]] == ["user", "assistant"]
|
||||
|
||||
assert response.status_code == 404
|
||||
finally:
|
||||
await channel.stop()
|
||||
await server_task
|
||||
@ -2847,7 +2854,7 @@ async def test_session_delete_blocks_origin_automation_when_unified_enabled(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_routes_accept_percent_encoded_websocket_keys(
|
||||
async def test_session_delete_accepts_percent_encoded_websocket_keys(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = _seed_session(tmp_path, key="websocket:encoded-key")
|
||||
@ -2857,13 +2864,6 @@ async def test_session_routes_accept_percent_encoded_websocket_keys(
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
auth = {"Authorization": f"Bearer {token}"}
|
||||
|
||||
msgs = await _http_get(
|
||||
"http://127.0.0.1:29910/api/sessions/websocket%3Aencoded-key/messages",
|
||||
headers=auth,
|
||||
)
|
||||
assert msgs.status_code == 200
|
||||
assert msgs.json()["key"] == "websocket:encoded-key"
|
||||
|
||||
path = sm._get_session_path("websocket:encoded-key")
|
||||
assert path.exists()
|
||||
deleted = await _http_get(
|
||||
@ -2878,41 +2878,6 @@ async def test_session_routes_accept_percent_encoded_websocket_keys(
|
||||
await server_task
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_messages_hide_persisted_runtime_context(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = SessionManager(tmp_path)
|
||||
session = sm.get_or_create("websocket:runtime-context")
|
||||
content, marker = append_runtime_context(
|
||||
"visible user text",
|
||||
[RuntimeContextBlock(source="goal", content="private goal context")],
|
||||
)
|
||||
session.add_message(
|
||||
"user",
|
||||
content,
|
||||
**{RUNTIME_CONTEXT_HISTORY_META: marker},
|
||||
)
|
||||
sm.save(session)
|
||||
channel = _ch(bus, session_manager=sm, port=29919)
|
||||
server_task = asyncio.create_task(channel.start())
|
||||
try:
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
response = await _http_get(
|
||||
"http://127.0.0.1:29919/api/sessions/websocket:runtime-context/messages",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
message = response.json()["messages"][0]
|
||||
assert message["content"] == "visible user text"
|
||||
assert RUNTIME_CONTEXT_HISTORY_META not in message
|
||||
assert "private goal context" not in response.text
|
||||
finally:
|
||||
await channel.stop()
|
||||
await server_task
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webui_thread_resigns_assistant_media_urls(
|
||||
bus: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
@ -3116,7 +3081,7 @@ async def test_webui_thread_negotiates_gzip_for_large_payloads(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_routes_reject_non_websocket_keys(
|
||||
async def test_session_delete_rejects_non_websocket_keys(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = _seed_many(
|
||||
@ -3133,14 +3098,6 @@ async def test_session_routes_reject_non_websocket_keys(
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
auth = {"Authorization": f"Bearer {token}"}
|
||||
|
||||
# The webui list already hides non-websocket sessions; handcrafted URLs
|
||||
# should hit the same boundary rather than exposing or deleting them.
|
||||
msgs = await _http_get(
|
||||
"http://127.0.0.1:29909/api/sessions/cli:direct/messages",
|
||||
headers=auth,
|
||||
)
|
||||
assert msgs.status_code == 404
|
||||
|
||||
doomed = sm._get_session_path("slack:C123")
|
||||
assert doomed.exists()
|
||||
deny_delete = await _http_get(
|
||||
@ -3155,7 +3112,7 @@ async def test_session_routes_reject_non_websocket_keys(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_routes_reject_invalid_key(
|
||||
async def test_session_delete_rejects_invalid_key(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
sm = _seed_session(tmp_path)
|
||||
@ -3168,7 +3125,7 @@ async def test_session_routes_reject_invalid_key(
|
||||
# Invalid characters in the key -> regex match fails -> 404
|
||||
# (route doesn't match, falls through to channel 404).
|
||||
resp = await _http_get(
|
||||
"http://127.0.0.1:29904/api/sessions/bad%20key/messages",
|
||||
"http://127.0.0.1:29904/api/sessions/bad%20key/delete",
|
||||
headers=auth,
|
||||
)
|
||||
assert resp.status_code in {400, 404}
|
||||
|
||||
@ -1,11 +1,8 @@
|
||||
"""Tests for the signed ``/api/media/<sig>/<payload>`` route and its replay
|
||||
integration on ``/api/sessions/<key>/messages``.
|
||||
"""Tests for the signed ``/api/media/<sig>/<payload>`` route and WebUI replay.
|
||||
|
||||
The route is the return path for images attached to persisted user turns:
|
||||
:meth:`WebSocketChannel.gateway.media.sign_media_path` mints URLs during session reads,
|
||||
and :meth:`GatewayHTTPHandler._handle_media_fetch` serves the bytes back.
|
||||
These tests cover the two halves end-to-end plus the adversarial edges
|
||||
(bad signatures, ``..`` traversal, non-existent files, non-image types).
|
||||
The route is the return path for local media rendered by the WebUI. These tests
|
||||
cover URL signing and serving end-to-end plus the adversarial edges (bad
|
||||
signatures, ``..`` traversal, non-existent files, non-image types).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@ -20,7 +17,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
import pytest
|
||||
|
||||
from nanobot.channels.websocket.runtime import WebSocketChannel, WebSocketConfig
|
||||
from nanobot.session.manager import Session, SessionManager
|
||||
from nanobot.session.manager import SessionManager
|
||||
from nanobot.webui.gateway_services import build_gateway_services
|
||||
from nanobot.webui.media_api import (
|
||||
b64url_decode,
|
||||
@ -497,91 +494,3 @@ async def test_media_route_serves_svg_with_strict_csp(
|
||||
assert resp.headers.get("x-content-type-options") == "nosniff"
|
||||
assert "default-src 'none'" in resp.headers.get("content-security-policy", "")
|
||||
assert "sandbox" in resp.headers.get("content-security-policy", "")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /api/sessions/<key>/messages: media_urls hydration on session read
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_messages_exposes_signed_media_urls(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
"""The read path must map persisted ``media`` paths onto signed URLs
|
||||
and strip the raw path — the client never learns the server's layout."""
|
||||
media = tmp_path / "media"
|
||||
media.mkdir()
|
||||
img = media / "u.png"
|
||||
img.write_bytes(_PNG_BYTES)
|
||||
|
||||
sm = SessionManager(tmp_path / "ws_state")
|
||||
sess = Session(key="websocket:media-hydrate")
|
||||
sess.add_message("user", "look at this", media=[str(img)])
|
||||
sess.add_message("assistant", "nice")
|
||||
sm.save(sess)
|
||||
|
||||
channel = _ch(bus, session_manager=sm, port=29925)
|
||||
with patch("nanobot.webui.media_gateway.get_media_dir", return_value=media):
|
||||
server_task = asyncio.create_task(channel.start())
|
||||
try:
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
auth = {"Authorization": f"Bearer {token}"}
|
||||
resp = await _http_get(
|
||||
"http://127.0.0.1:29925/api/sessions/websocket:media-hydrate/messages",
|
||||
headers=auth,
|
||||
)
|
||||
body = resp.json()
|
||||
# The signed URL round-trips end-to-end: fetching it yields the same bytes.
|
||||
user_msg = next(m for m in body["messages"] if m["role"] == "user")
|
||||
urls = user_msg["media_urls"]
|
||||
assert isinstance(urls, list) and len(urls) == 1
|
||||
assert urls[0]["name"] == "u.png"
|
||||
assert urls[0]["url"].startswith("/api/media/")
|
||||
# Raw paths must not leak to the wire.
|
||||
assert "media" not in user_msg
|
||||
|
||||
# And the URL actually works.
|
||||
fetched = await _http_get(f"http://127.0.0.1:29925{urls[0]['url']}")
|
||||
assert fetched.status_code == 200
|
||||
assert fetched.content == _PNG_BYTES
|
||||
finally:
|
||||
await channel.stop()
|
||||
await server_task
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_messages_skips_vanished_media(
|
||||
bus: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
"""Paths that no longer resolve inside the media root produce no URL —
|
||||
the message is still delivered, just without the preview."""
|
||||
media = tmp_path / "media"
|
||||
media.mkdir()
|
||||
|
||||
sm = SessionManager(tmp_path / "ws_state")
|
||||
sess = Session(key="websocket:vanished")
|
||||
sess.add_message("user", "missing pic", media=[str(media / "absent.png")])
|
||||
sm.save(sess)
|
||||
|
||||
channel = _ch(bus, session_manager=sm, port=29926)
|
||||
with patch("nanobot.webui.media_gateway.get_media_dir", return_value=media):
|
||||
server_task = asyncio.create_task(channel.start())
|
||||
try:
|
||||
token = channel.gateway.tokens.issue_api_token(300)
|
||||
resp = await _http_get(
|
||||
"http://127.0.0.1:29926/api/sessions/websocket:vanished/messages",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
user_msg = next(m for m in resp.json()["messages"] if m["role"] == "user")
|
||||
# absent.png lives inside the media root so it *does* get a signed
|
||||
# URL (we don't stat the file at signing time — that would slow
|
||||
# the listing). Fetching the URL is where the 404 surfaces.
|
||||
urls = user_msg.get("media_urls") or []
|
||||
assert len(urls) == 1
|
||||
fetched = await _http_get(f"http://127.0.0.1:29926{urls[0]['url']}")
|
||||
assert fetched.status_code == 404
|
||||
assert "media" not in user_msg
|
||||
finally:
|
||||
await channel.stop()
|
||||
await server_task
|
||||
|
||||
@ -3,14 +3,13 @@
|
||||
Persisted subagent announcements mirror ``agent/subagent_announce.md``: header,
|
||||
full ``Task:`` assignment (model context), ``Result:``, and a trailing model-only
|
||||
``Summarize…`` instruction. External channels (embedded WebUI, session previews)
|
||||
should show only the header plus a truncated result body."""
|
||||
should show only the header plus a truncated result body.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, cast
|
||||
|
||||
# Cap Result section length so WebSocket session replay stays readable; full text
|
||||
# remains on disk for LLM replay (we only mutate outgoing API copies in websocket).
|
||||
# Cap the Result section so session previews stay readable; full text remains on
|
||||
# disk for LLM replay.
|
||||
_SUBAGENT_CHANNEL_RESULT_MAX_CHARS = 800
|
||||
|
||||
|
||||
@ -44,16 +43,3 @@ def scrub_subagent_announce_body(content: str) -> str:
|
||||
if header and body:
|
||||
return f"{header}\n\n{body}"
|
||||
return header or body or stripped
|
||||
|
||||
|
||||
def scrub_subagent_messages_for_channel(messages: list[dict[str, Any]]) -> None:
|
||||
"""Mutate message dicts in place when they carry ``subagent_result`` inject."""
|
||||
for msg in messages:
|
||||
if not isinstance(cast(object, msg), dict):
|
||||
continue
|
||||
if msg.get("injected_event") != "subagent_result":
|
||||
continue
|
||||
raw = msg.get("content")
|
||||
if not isinstance(raw, str) or not raw.strip():
|
||||
continue
|
||||
msg["content"] = scrub_subagent_announce_body(raw)
|
||||
|
||||
@ -13,7 +13,7 @@ import shutil
|
||||
import uuid
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
from typing import Any, cast
|
||||
from typing import Any
|
||||
|
||||
from websockets.http11 import Request as WsRequest
|
||||
from websockets.http11 import Response
|
||||
@ -32,7 +32,6 @@ from nanobot.webui.http_utils import (
|
||||
|
||||
MediaDirProvider = Callable[[str | None], Path]
|
||||
SignedMediaPath = Callable[[Path], dict[str, str] | None]
|
||||
SignedMediaUrl = Callable[[Path], str | None]
|
||||
|
||||
|
||||
def b64url_encode(data: bytes) -> str:
|
||||
@ -190,37 +189,6 @@ def signed_media_attachments(
|
||||
return out
|
||||
|
||||
|
||||
def attach_signed_media_urls(
|
||||
payload: dict[str, Any],
|
||||
*,
|
||||
sign_path: SignedMediaUrl,
|
||||
) -> None:
|
||||
"""Replace raw media path lists in a WebUI session payload with signed URLs."""
|
||||
messages = payload.get("messages")
|
||||
if not isinstance(messages, list):
|
||||
return
|
||||
raw_messages = cast(list[Any], messages)
|
||||
for msg in raw_messages:
|
||||
if not isinstance(msg, dict):
|
||||
continue
|
||||
message = cast(dict[str, Any], msg)
|
||||
media = message.get("media")
|
||||
if not isinstance(media, list) or not media:
|
||||
continue
|
||||
media_entries = cast(list[Any], media)
|
||||
urls: list[dict[str, str]] = []
|
||||
for entry in media_entries:
|
||||
if not isinstance(entry, str) or not entry:
|
||||
continue
|
||||
signed = sign_path(Path(entry))
|
||||
if signed is None:
|
||||
continue
|
||||
urls.append({"url": signed, "name": Path(entry).name})
|
||||
if urls:
|
||||
message["media_urls"] = urls
|
||||
message.pop("media", None)
|
||||
|
||||
|
||||
def serve_signed_media(
|
||||
sig: str,
|
||||
payload: str,
|
||||
|
||||
@ -17,7 +17,6 @@ from nanobot.webui.attachment_ingress import (
|
||||
)
|
||||
from nanobot.webui.ingress_policy import AttachmentIngressLimits
|
||||
from nanobot.webui.media_api import (
|
||||
attach_signed_media_urls,
|
||||
serve_signed_media,
|
||||
sign_media_path,
|
||||
sign_or_stage_media_path,
|
||||
@ -99,9 +98,6 @@ class WebUIMediaGateway:
|
||||
sign_path=self.sign_or_stage_media_path,
|
||||
)
|
||||
|
||||
def augment_media_urls(self, payload: dict[str, Any]) -> None:
|
||||
attach_signed_media_urls(payload, sign_path=self.sign_media_path)
|
||||
|
||||
def augment_transcript_media(self, paths: list[str]) -> list[dict[str, Any]]:
|
||||
return signed_media_attachments(
|
||||
paths,
|
||||
|
||||
@ -26,10 +26,8 @@ from websockets.http11 import Response
|
||||
from nanobot.command.builtin import builtin_command_palette
|
||||
from nanobot.cron.session_turns import is_bound_cron_job
|
||||
from nanobot.cron.types import CronJob, CronSchedule
|
||||
from nanobot.runtime_context import public_history_messages
|
||||
from nanobot.security.workspace_access import WorkspaceScope
|
||||
from nanobot.triggers.local_types import LocalTrigger
|
||||
from nanobot.utils.subagent_channel_display import scrub_subagent_messages_for_channel
|
||||
from nanobot.webui.file_preview import (
|
||||
WebUIFilePreviewError,
|
||||
file_preview_availability_payload,
|
||||
@ -462,10 +460,6 @@ class GatewayHTTPHandler:
|
||||
# -- Session routes -----------------------------------------------------
|
||||
|
||||
async def _dispatch_session_routes(self, request: WsRequest, got: str) -> Response | None:
|
||||
m = re.match(r"^/api/sessions/([^/]+)/messages$", got)
|
||||
if m:
|
||||
return self._handle_session_messages(request, m.group(1))
|
||||
|
||||
m = re.match(r"^/api/sessions/([^/]+)/webui-thread$", got)
|
||||
if m:
|
||||
return self._handle_webui_thread_get(request, m.group(1))
|
||||
@ -527,34 +521,6 @@ class GatewayHTTPHandler:
|
||||
cleaned.append(row)
|
||||
return {"sessions": cleaned}
|
||||
|
||||
def _handle_session_messages(self, request: WsRequest, key: str) -> Response:
|
||||
if not self.check_api_token(request):
|
||||
return _http_error(401, "Unauthorized")
|
||||
if self.session_manager is None:
|
||||
return _http_error(503, "session manager unavailable")
|
||||
decoded_key = _decode_api_key(key)
|
||||
if decoded_key is None:
|
||||
return _http_error(400, "invalid session key")
|
||||
if not _is_websocket_channel_session_key(decoded_key):
|
||||
return _http_error(404, "session not found")
|
||||
data = self.session_manager.read_session_file(decoded_key)
|
||||
if data is None:
|
||||
return _http_error(404, "session not found")
|
||||
messages = data.get("messages")
|
||||
if isinstance(messages, list):
|
||||
session_messages = cast(list[dict[str, Any]], messages)
|
||||
scrub_subagent_messages_for_channel(session_messages)
|
||||
raw_session_messages = cast(list[Any], messages)
|
||||
data["messages"] = public_history_messages(
|
||||
[
|
||||
cast(dict[str, Any], message)
|
||||
for message in raw_session_messages
|
||||
if isinstance(message, dict)
|
||||
]
|
||||
)
|
||||
self.media.augment_media_urls(data)
|
||||
return _http_json_response(data)
|
||||
|
||||
def _handle_webui_thread_get(self, request: WsRequest, key: str) -> Response:
|
||||
if not self.check_api_token(request):
|
||||
return _http_error(401, "Unauthorized")
|
||||
|
||||
@ -1074,9 +1074,8 @@ async def test_process_message_persists_media_paths_on_user_turn(tmp_path: Path)
|
||||
"""User turns that attach images must record the media paths alongside
|
||||
the text so the webui can rehydrate previews on session replay.
|
||||
|
||||
This is the producer half of the signed-media-URL round-trip: paths are
|
||||
stored here, then :meth:`WebSocketChannel._augment_media_urls` maps them
|
||||
onto signed URLs on the way out.
|
||||
The WebUI transcript replay can use these paths to restore attachment
|
||||
previews when it backfills from canonical session history.
|
||||
"""
|
||||
img_a = tmp_path / "uuid-1.png"
|
||||
img_a.write_bytes(_PNG_1X1)
|
||||
|
||||
@ -1,9 +1,6 @@
|
||||
"""Tests for subagent announce text shaping on external channel surfaces."""
|
||||
|
||||
from nanobot.utils.subagent_channel_display import (
|
||||
scrub_subagent_announce_body,
|
||||
scrub_subagent_messages_for_channel,
|
||||
)
|
||||
from nanobot.utils.subagent_channel_display import scrub_subagent_announce_body
|
||||
|
||||
|
||||
def test_scrub_subagent_keeps_header_and_result_only() -> None:
|
||||
@ -22,24 +19,6 @@ Summarize this naturally for the user. Keep it brief."""
|
||||
assert "Summarize" not in out
|
||||
|
||||
|
||||
def test_scrub_subagent_messages_mutates_matching_rows() -> None:
|
||||
messages: list[dict] = [
|
||||
{"role": "assistant", "content": "hi"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": (
|
||||
"[Subagent 'x' completed successfully]\n\nTask: t\n\nResult:\nr\n\nSummarize this naturally"
|
||||
),
|
||||
"injected_event": "subagent_result",
|
||||
},
|
||||
]
|
||||
scrub_subagent_messages_for_channel(messages)
|
||||
assert messages[0]["content"] == "hi"
|
||||
assert "Task:" not in messages[1]["content"]
|
||||
assert "[Subagent 'x' completed successfully]" in messages[1]["content"]
|
||||
assert "r" in messages[1]["content"]
|
||||
|
||||
|
||||
def test_scrub_normalizes_crlf_before_result_marker() -> None:
|
||||
raw = "[Subagent 'z' failed]\r\n\r\nTask: x\r\n\r\nResult:\r\none line\r\n\r\nSummarize this naturally"
|
||||
out = scrub_subagent_announce_body(raw)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user