From a953f9c2c3d00ba6d64ce6e8491cc805fa9b17d0 Mon Sep 17 00:00:00 2001 From: ThorstenHellert Date: Wed, 12 Aug 2026 19:31:52 +0200 Subject: [PATCH 1/5] feat(web): record agent activity in a server-side history ring Activity frames were broadcast once over SSE and then lost. A client that reloaded or connected late had no way to see what the agent had done, and agent-driven panel changes produced no activity frame at all. Keep the last 50 accepted frames in an in-process ring and serve them newest-first from GET /api/agent-activity/recent, on the same loopback auth surface as the existing POST. Panel routes mirror agent-sourced focus, visibility and arrange broadcasts into the ring as synthetic tool frames, one frame per action. The kind vocabulary gains config and ui for the non-panel emitters. --- src/osprey/interfaces/web_terminal/app.py | 6 + .../web_terminal/routes/agent_activity.py | 83 ++- .../interfaces/web_terminal/routes/panels.py | 55 ++ .../web_terminal/test_agent_activity_ring.py | 490 ++++++++++++++++++ 4 files changed, 624 insertions(+), 10 deletions(-) create mode 100644 tests/interfaces/web_terminal/test_agent_activity_ring.py diff --git a/src/osprey/interfaces/web_terminal/app.py b/src/osprey/interfaces/web_terminal/app.py index 46c4a4958..9a3d8283f 100644 --- a/src/osprey/interfaces/web_terminal/app.py +++ b/src/osprey/interfaces/web_terminal/app.py @@ -8,6 +8,7 @@ import asyncio import os +from collections import deque from contextlib import asynccontextmanager, suppress from pathlib import Path from typing import TYPE_CHECKING, NamedTuple @@ -27,6 +28,7 @@ from osprey.interfaces.web_terminal.ownership import OwnershipStoreError from osprey.interfaces.web_terminal.pty_manager import PtyRegistry from osprey.interfaces.web_terminal.routes import router +from osprey.interfaces.web_terminal.routes.agent_activity import ACTIVITY_RING_MAX from osprey.interfaces.web_terminal.url_prefix import apply_url_prefix, compute_url_prefix from osprey.profiles.web_panels import BUILTIN_PANELS, UNIVERSAL_PANELS @@ -700,6 +702,10 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: ) app.state.broadcaster = FileEventBroadcaster() app.state.active_panel = None + # Bounded history of agent-activity events. The SSE stream only reaches + # browsers that are already connected, so the ring is what a browser + # opened (or reloaded) mid-session reads to catch up on recent actions. + app.state.agent_activity_ring = deque(maxlen=ACTIVITY_RING_MAX) # Optional human-readable deployment name shown in the header so # otherwise-identical web terminals are distinguishable. The # ``OSPREY_WEB_APP_NAME`` environment variable takes precedence over diff --git a/src/osprey/interfaces/web_terminal/routes/agent_activity.py b/src/osprey/interfaces/web_terminal/routes/agent_activity.py index 400854e09..3fe9e41a5 100644 --- a/src/osprey/interfaces/web_terminal/routes/agent_activity.py +++ b/src/osprey/interfaces/web_terminal/routes/agent_activity.py @@ -9,18 +9,33 @@ The payload shape is a fixed interface contract shared with the frontend:: - request: {"tool": str, "target": {"kind": "panel"|"channel"|"run"|"artifact", + request: {"tool": str, "target": {"kind": "panel"|"channel"|"run" + |"artifact"|"config"|"ui", "panel"?: str, "detail"?: str}} broadcast: {"type": "agent_activity", "tool": ..., "target": {...}, "ts": ...} The server adds ``type`` and ``ts``; optional target fields are omitted from the broadcast when absent. Like the panel routes, this endpoint relies on the loopback baseline for access control — no additional auth. + +Every accepted event is also appended to a bounded in-memory history ring on +``app.state.agent_activity_ring`` before it is broadcast, so a browser that +connects (or reconnects) after the fact can still see what the agent has been +doing. SSE clients see no difference — the ring is a pure side channel. + +``GET /api/agent-activity/recent?limit=N`` reads that ring back, newest first:: + + response: {"events": [{"type": "agent_activity", "tool": ..., + "target": {...}, "ts": ...}, ...]} + +The events are the broadcast frames verbatim, so a browser can feed them +through the same handler it uses for the SSE stream. """ from __future__ import annotations import time +from itertools import islice from typing import Literal from fastapi import APIRouter, Request @@ -34,11 +49,15 @@ _MAX_NAME_LEN = 256 _MAX_DETAIL_LEN = 1024 +#: Size of the ``app.state.agent_activity_ring`` history buffer. A browser +#: replays at most this many recent events on connect; the oldest fall off. +ACTIVITY_RING_MAX = 50 + class AgentActivityTarget(BaseModel): """The surface the agent is acting on.""" - kind: Literal["panel", "channel", "run", "artifact"] + kind: Literal["panel", "channel", "run", "artifact", "config", "ui"] panel: str | None = Field(default=None, max_length=_MAX_NAME_LEN) detail: str | None = Field(default=None, max_length=_MAX_DETAIL_LEN) @@ -50,19 +69,63 @@ class AgentActivityRequest(BaseModel): target: AgentActivityTarget +def record_activity(request: Request, tool: str, target: dict) -> dict: + """Stamp an agent-activity frame, append it to the history ring, return it. + + The single place the frame shape is written, so every producer — this + module's POST route and the panel routes, which mirror agent-origin panel + commands that never pass through it — puts the same thing in the ring that + ``GET /api/agent-activity/recent`` serves and the SSE stream carries. + + Appending is not broadcasting: callers that also broadcast pass the + returned frame on, and callers recording history only just drop it. + + Args: + request: Incoming FastAPI request carrying ``app.state``. + tool: Name of the tool (or synthetic verb) the frame reports. + target: Already-serialised target, optional keys omitted. + + Returns: + The frame, whether or not the app carries a ring. + """ + event = {"type": "agent_activity", "tool": tool, "target": target, "ts": time.time()} + # Apps that mount these routers standalone (tests, embedders) need not + # carry a ring; history is then simply unavailable, and the caller's own + # work (a broadcast, or nothing) still runs. + ring = getattr(request.app.state, "agent_activity_ring", None) + if ring is not None: + ring.append(event) + return event + + @router.post("/api/agent-activity") async def post_agent_activity(body: AgentActivityRequest, request: Request): """Broadcast an agent-activity event to all connected browsers via SSE. Malformed bodies and unknown target kinds are rejected with 422 by the Pydantic model before this handler runs — nothing is broadcast for them. + The accepted event is recorded in the history ring first, so an event is + never broadcast without also being in the history a late browser reads. """ - request.app.state.broadcaster.broadcast( - { - "type": "agent_activity", - "tool": body.tool, - "target": body.target.model_dump(exclude_none=True), - "ts": time.time(), - } - ) + event = record_activity(request, body.tool, body.target.model_dump(exclude_none=True)) + request.app.state.broadcaster.broadcast(event) return {"ok": True} + + +@router.get("/api/agent-activity/recent") +async def get_recent_agent_activity(request: Request, limit: int = ACTIVITY_RING_MAX): + """Return the most recent agent-activity events, newest first. + + A browser that opens mid-session, or reconnects after its SSE stream + dropped, reads this to rebuild the recent history it never received live. + Each event is the broadcast frame verbatim, including the server ``ts``. + + ``limit`` is clamped into ``0..ACTIVITY_RING_MAX`` rather than rejected, so + a caller asking for more than the ring can hold gets everything it has. + Apps that mount this router without a ring report an empty history. + """ + ring = getattr(request.app.state, "agent_activity_ring", None) + if ring is None: + return {"events": []} + limit = max(0, min(limit, ACTIVITY_RING_MAX)) + return {"events": list(islice(reversed(ring), limit))} diff --git a/src/osprey/interfaces/web_terminal/routes/panels.py b/src/osprey/interfaces/web_terminal/routes/panels.py index 0979a67c3..0df49f78f 100644 --- a/src/osprey/interfaces/web_terminal/routes/panels.py +++ b/src/osprey/interfaces/web_terminal/routes/panels.py @@ -17,6 +17,7 @@ from fastapi.responses import FileResponse, Response from pydantic import BaseModel +from osprey.interfaces.web_terminal.routes.agent_activity import record_activity from osprey.interfaces.web_terminal.url_prefix import apply_url_prefix, compute_url_prefix from osprey.profiles.web_panels import BUILTIN_PANEL_LABELS, BUILTIN_PANELS @@ -336,6 +337,33 @@ def _known_panel_ids(request: Request) -> set[str]: _TERMINAL_PANEL_ID = "terminal" +def _mirror_agent_panel_activity(request: Request, tool: str, panel: str) -> None: + """Record an agent-origin panel command in the agent-activity history ring. + + Panel commands reach the browser as their own SSE frames (``panel_focus``, + ``panel_visibility``, ...), never through ``POST /api/agent-activity``, so + without this they are invisible to a client that reads + ``GET /api/agent-activity/recent`` after connecting late. The row goes in + through that route's own ``record_activity``, so a consumer feeds it + through the handler it already uses for the SSE ``agent_activity`` stream. + + ``tool`` is synthetic: the panel routes carry no tool name of their own, so + the caller supplies the MCP verb the action corresponds to (``switch_panel``, + ``show_panel``, ``hide_panel``, ``arrange_workspace``, ``register_panel``) + and the frontend words the entry from it. + + Nothing is broadcast — this is history only. Callers must invoke it for + agent-origin requests exactly once per action, and never for human ones: a + human's own gestures are not the agent's activity. + + Args: + request: Incoming FastAPI request carrying ``app.state``. + tool: Synthetic tool name naming the action. + panel: The panel id the action targeted. + """ + record_activity(request, tool, {"kind": "panel", "panel": panel}) + + class PanelFocusRequest(BaseModel): panel: str url: str | None = None @@ -378,6 +406,11 @@ async def set_panel_focus(body: PanelFocusRequest, request: Request): A panel already in the rail — which is the only kind a human can click — changes nothing and emits no visibility frame. + An agent switch is also mirrored into the activity history ring as one + ``switch_panel`` row. When the switch additionally adds rail membership, + only the focus is mirrored: the pair of frames is one agent action, and + history counts actions, not frames. + Args: body: ``panel`` (panel id), optional ``url`` to load, and optional ``source`` attribution. @@ -420,6 +453,7 @@ async def set_panel_focus(body: PanelFocusRequest, request: Request): event: dict = {"type": "panel_focus", "panel": body.panel, "source": body.source} if body.url: event["url"] = _prefix_path(body.url) + _mirror_agent_panel_activity(request, "switch_panel", body.panel) request.app.state.broadcaster.broadcast(event) return {"status": "ok", "active_panel": body.panel} @@ -434,6 +468,10 @@ class PanelVisibilityRequest(BaseModel): async def set_panel_visibility(body: PanelVisibilityRequest, request: Request): """Show or hide a panel and broadcast the change via SSE. + An agent-origin change is also mirrored into the activity history ring, as + a ``show_panel`` or ``hide_panel`` row depending on the flag, so a client + reading the history can word it the way it words the live frame. + Args: body: ``panel`` (panel id) and ``visible`` (desired visibility). request: Incoming FastAPI request carrying ``app.state``. @@ -456,6 +494,10 @@ async def set_panel_visibility(body: PanelVisibilityRequest, request: Request): event: dict = {"type": "panel_visibility", "panel": body.panel, "visible": body.visible} if body.source: event["source"] = body.source + if body.source == "agent": + _mirror_agent_panel_activity( + request, "show_panel" if body.visible else "hide_panel", body.panel + ) request.app.state.broadcaster.broadcast(event) return {"status": "ok", "panel": body.panel, "visible": body.visible} @@ -584,6 +626,9 @@ async def arrange_panels(body: PanelArrangeRequest, request: Request): broadcast still carries ``focus`` only when one was requested, leaving the client's fallback rule in charge of what is actually focused on screen. + An agent arrangement is mirrored into the activity history ring as a single + ``arrange_workspace`` row targeting the recorded focus panel. + Args: body: ``tiles`` (explicit ids, left-to-right) **or** ``preset`` (a configured layout name), an optional ``focus`` target that must be @@ -642,6 +687,11 @@ async def arrange_panels(body: PanelArrangeRequest, request: Request): event["prune_rail"] = True if body.source: event["source"] = body.source + if body.source == "agent": + # One row for the whole arrangement, targeting the panel focus lands on + # — the same id recorded as ``active_panel`` above, so the history entry + # names the tile the operator's eye is sent to. + _mirror_agent_panel_activity(request, "arrange_workspace", body.focus or tiles[0]) request.app.state.broadcaster.broadcast(event) return { "status": "ok", @@ -894,6 +944,9 @@ async def register_panel(body: PanelRegisterRequest, request: Request): If a custom panel with the same ``id`` already exists it is replaced atomically (remove-then-append) so the proxy always returns the first match. + An agent-origin registration is mirrored into the activity history ring as + a ``register_panel`` row. + Args: body: Panel registration fields: ``id``, ``label``, ``url`` (raw), ``path`` (default ``"/"``), ``health_endpoint`` (optional). @@ -970,6 +1023,8 @@ async def register_panel(body: PanelRegisterRequest, request: Request): } if body.source: event["source"] = body.source + if body.source == "agent": + _mirror_agent_panel_activity(request, "register_panel", body.id) request.app.state.broadcaster.broadcast(event) return {"status": "ok", "id": body.id, "label": body.label, "url": browser_url} diff --git a/tests/interfaces/web_terminal/test_agent_activity_ring.py b/tests/interfaces/web_terminal/test_agent_activity_ring.py new file mode 100644 index 000000000..104f6ac08 --- /dev/null +++ b/tests/interfaces/web_terminal/test_agent_activity_ring.py @@ -0,0 +1,490 @@ +"""Tests for the agent-activity history ring and the activity-kind schema. + +The SSE stream (``GET /api/files/events``) only reaches browsers that are +already connected, so ``POST /api/agent-activity`` also records every accepted +event in a bounded deque on ``app.state.agent_activity_ring``. A browser that +opens or reloads mid-session reads that ring to catch up. + +Five concerns are covered, in the sections below: + +1. app startup creates the ring alongside its ``app.state`` peers; +2. the POST handler appends each accepted event — and only accepted events — + to the ring before broadcasting, bounded at ``ACTIVITY_RING_MAX``; +3. the target-kind schema accepts ``config`` and ``ui`` alongside the original + four kinds, and still rejects everything else; +4. ``GET /api/agent-activity/recent`` reads the ring back newest-first, with a + clamped ``limit``; +5. the panel routes mirror agent-origin commands into the same ring under + synthetic tool names, one row per action, and never mirror human gestures. +""" + +from __future__ import annotations + +from collections import deque +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from osprey.interfaces.web_terminal.routes.agent_activity import ACTIVITY_RING_MAX, router + + +def _make_client(*, with_ring: bool = True) -> TestClient: + """A minimal app exposing the router, a stub broadcaster and (optionally) a ring.""" + app = FastAPI() + app.include_router(router) + app.state.broadcaster = MagicMock() + if with_ring: + app.state.agent_activity_ring = deque(maxlen=ACTIVITY_RING_MAX) + return TestClient(app) + + +def _post(client: TestClient, tool: str = "switch_panel", **target) -> None: + """POST one activity event, asserting it was accepted.""" + body = {"tool": tool, "target": target or {"kind": "panel"}} + assert client.post("/api/agent-activity", json=body).status_code == 200 + + +# ---- App startup wires the ring onto app.state ---- + + +def test_app_lifespan_builds_a_bounded_ring(tmp_path): + """The ring is created with its ``app.state`` peers when the app starts up.""" + from osprey.interfaces.web_terminal.app import create_app + + with patch( + "osprey.interfaces.web_terminal.app._load_web_config", + return_value={"watch_dir": str(tmp_path)}, + ): + app = create_app(shell_command="echo") + with TestClient(app): + ring = app.state.agent_activity_ring + + assert isinstance(ring, deque) + assert ring.maxlen == ACTIVITY_RING_MAX + assert len(ring) == 0 + + +# ---- Ring append semantics ---- + + +def test_accepted_event_is_appended_to_the_ring(): + """The ring entry is the same frame the broadcaster receives.""" + client = _make_client() + _post(client, tool="read_channel", kind="channel", detail="SR01C:BPM1:X") + + ring = client.app.state.agent_activity_ring + assert len(ring) == 1 + frame = client.app.state.broadcaster.broadcast.call_args[0][0] + assert ring[0] == frame + assert ring[0] == { + "type": "agent_activity", + "tool": "read_channel", + "target": {"kind": "channel", "detail": "SR01C:BPM1:X"}, + "ts": frame["ts"], + } + + +def test_ring_keeps_events_in_arrival_order(): + """Oldest first — the ring reads as a timeline, not a stack.""" + client = _make_client() + for name in ("first", "second", "third"): + _post(client, tool=name) + + ring = client.app.state.agent_activity_ring + assert [event["tool"] for event in ring] == ["first", "second", "third"] + + +def test_ring_is_bounded_and_drops_the_oldest(): + """Past ``ACTIVITY_RING_MAX`` events the ring holds only the newest ones.""" + client = _make_client() + overflow = 5 + for index in range(ACTIVITY_RING_MAX + overflow): + _post(client, tool=f"tool-{index}") + + ring = client.app.state.agent_activity_ring + assert len(ring) == ACTIVITY_RING_MAX + assert ring[0]["tool"] == f"tool-{overflow}" + assert ring[-1]["tool"] == f"tool-{ACTIVITY_RING_MAX + overflow - 1}" + + +def test_rejected_event_is_not_appended(): + """A 422 leaves the ring untouched, just as it broadcasts nothing.""" + client = _make_client() + resp = client.post("/api/agent-activity", json={"tool": "x", "target": {"kind": "widget"}}) + + assert resp.status_code == 422 + assert len(client.app.state.agent_activity_ring) == 0 + client.app.state.broadcaster.broadcast.assert_not_called() + + +def test_append_happens_before_broadcast(): + """A broadcast never fires for an event the ring has not yet recorded. + + A late browser reads the ring; ordering the append first means there is no + window in which a connected client has seen an event the ring is missing. + """ + client = _make_client() + seen_at_broadcast: list[int] = [] + client.app.state.broadcaster.broadcast.side_effect = lambda _frame: seen_at_broadcast.append( + len(client.app.state.agent_activity_ring) + ) + + _post(client) + + assert seen_at_broadcast == [1] + + +def test_missing_ring_does_not_break_the_route(): + """Apps mounting the router standalone simply get no history.""" + client = _make_client(with_ring=False) + _post(client) + + client.app.state.broadcaster.broadcast.assert_called_once() + assert not hasattr(client.app.state, "agent_activity_ring") + + +# ---- Target-kind schema ---- + + +@pytest.mark.parametrize("kind", ["panel", "channel", "run", "artifact", "config", "ui"]) +def test_supported_kinds_are_accepted(kind): + """The original four kinds plus ``config`` and ``ui`` all round-trip.""" + client = _make_client() + _post(client, tool="emit", kind=kind) + + frame = client.app.state.broadcaster.broadcast.call_args[0][0] + assert frame["target"]["kind"] == kind + assert client.app.state.agent_activity_ring[0]["target"]["kind"] == kind + + +@pytest.mark.parametrize("kind", ["widget", "Config", "UI", "", "settings"]) +def test_unknown_kinds_still_rejected(kind): + """Widening the Literal must not turn it into a free-form string.""" + client = _make_client() + resp = client.post("/api/agent-activity", json={"tool": "emit", "target": {"kind": kind}}) + + assert resp.status_code == 422 + assert len(client.app.state.agent_activity_ring) == 0 + + +# ---- GET /api/agent-activity/recent reads the ring back ---- + + +def _get_recent(client: TestClient, **params) -> list[dict]: + """GET the recent-activity history, asserting a 200, and return its events.""" + resp = client.get("/api/agent-activity/recent", params=params) + assert resp.status_code == 200 + body = resp.json() + assert set(body) == {"events"} + return body["events"] + + +def test_recent_is_empty_before_any_activity(): + """A fresh ring reads back as an empty list, not a 404 or a null.""" + assert _get_recent(_make_client()) == [] + + +def test_recent_returns_newest_first(): + """The popover wants the latest action at the top, so the ring is reversed.""" + client = _make_client() + for name in ("first", "second", "third"): + _post(client, tool=name) + + assert [event["tool"] for event in _get_recent(client)] == ["third", "second", "first"] + + +def test_recent_returns_the_broadcast_frame_verbatim(): + """Consumers reuse their SSE handler, so the frame must survive the round trip.""" + client = _make_client() + _post(client, tool="read_channel", kind="channel", detail="SR01C:BPM1:X") + + frame = client.app.state.broadcaster.broadcast.call_args[0][0] + assert _get_recent(client) == [ + { + "type": "agent_activity", + "tool": "read_channel", + "target": {"kind": "channel", "detail": "SR01C:BPM1:X"}, + "ts": frame["ts"], + } + ] + + +def test_recent_limit_takes_the_newest_events(): + """``limit`` trims the tail of the history, never the head.""" + client = _make_client() + for index in range(5): + _post(client, tool=f"tool-{index}") + + assert [e["tool"] for e in _get_recent(client, limit=2)] == ["tool-4", "tool-3"] + + +def test_recent_defaults_to_the_whole_ring(): + """Omitting ``limit`` returns everything the ring holds.""" + client = _make_client() + for index in range(ACTIVITY_RING_MAX): + _post(client, tool=f"tool-{index}") + + assert len(_get_recent(client)) == ACTIVITY_RING_MAX + + +def test_recent_limit_above_the_ring_max_is_clamped_not_rejected(): + """An over-large ``limit`` yields everything available rather than a 422.""" + client = _make_client() + for index in range(3): + _post(client, tool=f"tool-{index}") + + assert len(_get_recent(client, limit=ACTIVITY_RING_MAX * 10)) == 3 + + +@pytest.mark.parametrize("limit", [0, -1, -100]) +def test_recent_non_positive_limit_returns_nothing(limit): + """Zero and negative limits clamp to zero — never to "the whole ring".""" + client = _make_client() + _post(client) + + assert _get_recent(client, limit=limit) == [] + + +def test_recent_non_integer_limit_is_rejected(): + """``limit`` stays typed: a junk value is a 422, not a silent default.""" + client = _make_client() + assert client.get("/api/agent-activity/recent", params={"limit": "lots"}).status_code == 422 + + +def test_recent_without_a_ring_reports_no_history(): + """Standalone mounts have no ring; the read degrades to an empty history.""" + client = _make_client(with_ring=False) + _post(client) + + assert _get_recent(client) == [] + + +def test_recent_route_registered_on_composite_router(): + """The composite web-terminal router exposes GET /api/agent-activity/recent. + + Starlette 1.x ``include_router`` does not flatten, so registration is + asserted through the OpenAPI schema, never through ``router.routes``. + """ + from osprey.interfaces.web_terminal.routes import router as composite_router + + app = FastAPI() + app.include_router(composite_router) + paths = app.openapi()["paths"] + assert "/api/agent-activity/recent" in paths + assert "get" in paths["/api/agent-activity/recent"] + + +# ---- Panel routes mirror agent-origin commands into the ring ---- +# +# Panel commands never pass through POST /api/agent-activity — they have their +# own SSE frames — so ``routes/panels.py`` appends an equivalent row directly. +# The synthetic tool name is what the frontend words the entry from, so each +# one is pinned here. + +# Resolve the register route's SSRF check to a routable LAN address without +# real DNS (the pattern test_panels_source_tag.py uses). +_LAN_ADDR = [(2, 1, 6, "", ("10.0.0.5", 0))] +_GETADDRINFO_TARGET = "osprey.interfaces.web_terminal.routes.panels.socket.getaddrinfo" + +#: Panel already in the launcher rail, so focusing it adds no membership. +_MEMBER_PANEL = "ariel" +#: Enabled panel deliberately left OUT of the rail, so focusing it takes the +#: membership-add path that broadcasts a visibility frame before the focus one. +_NON_MEMBER_PANEL = "artifacts" + + +def _make_panel_client(*, with_ring: bool = True) -> TestClient: + """An app exposing the panel routes *and* the activity routes over one ring. + + Both routers share ``app.state``, so a mirrored panel row can be read back + through ``GET /api/agent-activity/recent`` — the round trip a browser makes. + """ + from osprey.interfaces.web_terminal.routes.panels import router as panels_router + + app = FastAPI() + app.include_router(panels_router) + app.include_router(router) + app.state.broadcaster = MagicMock() + app.state.enabled_panels = {_MEMBER_PANEL, _NON_MEMBER_PANEL} + app.state.custom_panels = [] + app.state.visible_panels = [_MEMBER_PANEL] + app.state.allow_runtime_panels = True + if with_ring: + app.state.agent_activity_ring = deque(maxlen=ACTIVITY_RING_MAX) + return TestClient(app) + + +def _panel_post(client: TestClient, path: str, body: dict) -> None: + """POST a panel command, asserting it was accepted.""" + assert client.post(path, json=body).status_code == 200 + + +def _only_row(client: TestClient) -> dict: + """The ring's single row, asserting there is exactly one.""" + ring = client.app.state.agent_activity_ring + assert len(ring) == 1, [event["tool"] for event in ring] + return ring[0] + + +def test_agent_focus_mirrors_a_switch_panel_row(): + """A ``switch_panel`` row, shaped exactly like a broadcast activity frame.""" + client = _make_panel_client() + _panel_post(client, "/api/panel-focus", {"panel": _MEMBER_PANEL, "source": "agent"}) + + row = _only_row(client) + assert row == { + "type": "agent_activity", + "tool": "switch_panel", + "target": {"kind": "panel", "panel": _MEMBER_PANEL}, + "ts": row["ts"], + } + assert isinstance(row["ts"], float) + + +def test_membership_adding_switch_mirrors_only_the_focus(): + """Two frames, one action: the visibility frame that precedes the focus is + part of the same switch, so history records the switch once.""" + client = _make_panel_client() + _panel_post(client, "/api/panel-focus", {"panel": _NON_MEMBER_PANEL, "source": "agent"}) + + # Both frames really did go out — the mirroring is what is deduped, not the + # broadcast, so this test would pass vacuously if the path had changed. + kinds = [call[0][0]["type"] for call in client.app.state.broadcaster.broadcast.call_args_list] + assert kinds == ["panel_visibility", "panel_focus"] + + row = _only_row(client) + assert row["tool"] == "switch_panel" + assert row["target"] == {"kind": "panel", "panel": _NON_MEMBER_PANEL} + + +def test_agent_show_mirrors_a_show_panel_row(): + """Making a panel visible is a ``show_panel`` row.""" + client = _make_panel_client() + _panel_post( + client, + "/api/panel-visibility", + {"panel": _NON_MEMBER_PANEL, "visible": True, "source": "agent"}, + ) + + row = _only_row(client) + assert row["tool"] == "show_panel" + assert row["target"] == {"kind": "panel", "panel": _NON_MEMBER_PANEL} + + +def test_agent_hide_mirrors_a_hide_panel_row(): + """A hide is always mirrored — it broadcasts no focus frame to defer to.""" + client = _make_panel_client() + _panel_post( + client, + "/api/panel-visibility", + {"panel": _MEMBER_PANEL, "visible": False, "source": "agent"}, + ) + + kinds = [call[0][0]["type"] for call in client.app.state.broadcaster.broadcast.call_args_list] + assert kinds == ["panel_visibility"] + + row = _only_row(client) + assert row["tool"] == "hide_panel" + assert row["target"] == {"kind": "panel", "panel": _MEMBER_PANEL} + + +def test_agent_arrange_mirrors_one_row_targeting_the_focus(): + """A whole-workspace arrangement is one row, named by where focus lands.""" + client = _make_panel_client() + _panel_post( + client, + "/api/panel-arrange", + { + "tiles": [_MEMBER_PANEL, _NON_MEMBER_PANEL], + "focus": _NON_MEMBER_PANEL, + "source": "agent", + }, + ) + + row = _only_row(client) + assert row["tool"] == "arrange_workspace" + assert row["target"] == {"kind": "panel", "panel": _NON_MEMBER_PANEL} + + +def test_agent_arrange_without_focus_targets_the_first_tile(): + """No requested focus: the row names the tile the server records as active.""" + client = _make_panel_client() + _panel_post( + client, + "/api/panel-arrange", + {"tiles": [_NON_MEMBER_PANEL, _MEMBER_PANEL], "source": "agent"}, + ) + + assert _only_row(client)["target"] == {"kind": "panel", "panel": _NON_MEMBER_PANEL} + assert client.app.state.active_panel == _NON_MEMBER_PANEL + + +def test_agent_register_mirrors_a_register_panel_row(): + """A runtime registration is a ``register_panel`` row keyed by the new id.""" + client = _make_panel_client() + with patch(_GETADDRINFO_TARGET, return_value=_LAN_ADDR): + _panel_post( + client, + "/api/panels/register", + { + "id": "grafana", + "label": "GRAFANA", + "url": "http://grafana.lan:3000", + "source": "agent", + }, + ) + + row = _only_row(client) + assert row["tool"] == "register_panel" + assert row["target"] == {"kind": "panel", "panel": "grafana"} + + +@pytest.mark.parametrize( + ("path", "body"), + [ + ("/api/panel-focus", {"panel": _MEMBER_PANEL}), + ("/api/panel-focus", {"panel": _NON_MEMBER_PANEL}), # membership-add path + ("/api/panel-visibility", {"panel": _MEMBER_PANEL, "visible": False}), + ("/api/panel-visibility", {"panel": _NON_MEMBER_PANEL, "visible": True}), + ("/api/panel-arrange", {"tiles": [_MEMBER_PANEL]}), + ( + "/api/panels/register", + {"id": "grafana", "label": "GRAFANA", "url": "http://grafana.lan:3000"}, + ), + ], +) +def test_human_origin_commands_are_never_mirrored(path, body): + """The history is the *agent's* activity: an operator's own gestures — which + carry no ``source`` — leave it empty.""" + client = _make_panel_client() + with patch(_GETADDRINFO_TARGET, return_value=_LAN_ADDR): + _panel_post(client, path, body) + + assert list(client.app.state.agent_activity_ring) == [] + + +def test_panel_routes_mounted_without_a_ring_do_not_break(): + """Standalone mounts carry no ring; the command still applies and broadcasts.""" + client = _make_panel_client(with_ring=False) + _panel_post(client, "/api/panel-focus", {"panel": _MEMBER_PANEL, "source": "agent"}) + + client.app.state.broadcaster.broadcast.assert_called_once() + assert not hasattr(client.app.state, "agent_activity_ring") + + +def test_mirrored_rows_read_back_through_the_recent_endpoint(): + """The mirrored rows are consumable by the SSE handler, newest first.""" + client = _make_panel_client() + _panel_post(client, "/api/panel-focus", {"panel": _MEMBER_PANEL, "source": "agent"}) + _panel_post( + client, + "/api/panel-visibility", + {"panel": _MEMBER_PANEL, "visible": False, "source": "agent"}, + ) + + events = _get_recent(client) + assert [event["tool"] for event in events] == ["hide_panel", "switch_panel"] + assert all(set(event) == {"type", "tool", "target", "ts"} for event in events) + assert all(event["type"] == "agent_activity" for event in events) From 73e38ac4ae4b9315c06e3858260554433eb7a6f7 Mon Sep 17 00:00:00 2001 From: ThorstenHellert Date: Wed, 12 Aug 2026 19:33:38 +0200 Subject: [PATCH 2/5] feat(mcp): emit activity from every mutating agent tool An operator watching the terminal saw only a fraction of what the agent did. Nine tool families mutated state silently: logbook entries and publishes, plan writes and validations, queue stops, Phoebus drives, script runs with control-system writes, lattice edits, config patches, and window moves. Artifact saves were equally invisible. Emit one activity frame per successful mutation, placed after every refusal and failure return so a blocked action never reports itself. Withdrawn queue stops are distinguished from executed ones. Config frames carry key paths only, never values, since .mcp.json values can include credentials and the ring is persistent and GET-served. Artifact saves go through a store listener that hands frames to a worker thread, filtering notebook auto-saves so one execution cannot flood the ring. Unit runs used to leak real HTTP posts from these emit sites toward any live web terminal on the port. A directory-wide conftest guard now stubs both posters, with a marker escape hatch for the two suites that assert on the wire itself. --- pyproject.toml | 1 + src/osprey/mcp_server/ariel/tools/entry.py | 19 + src/osprey/mcp_server/ariel/tools/publish.py | 18 + src/osprey/mcp_server/artifact_activity.py | 168 +++ .../mcp_server/bluesky/tools/authoring.py | 36 + src/osprey/mcp_server/bluesky/tools/queue.py | 12 + .../mcp_server/phoebus/tools/bridge_tools.py | 28 +- .../python_executor/tools/python_execute.py | 31 + .../tools/python_execute_file.py | 22 + src/osprey/mcp_server/startup.py | 22 + .../workspace/tools/lattice_tools.py | 32 + .../workspace/tools/screen_capture.py | 14 + .../mcp_server/workspace/tools/setup.py | 51 + .../bluesky/test_draft_tools_emit.py | 263 ++++ tests/mcp_server/conftest.py | 51 + .../test_artifact_activity_listener.py | 297 +++++ tests/mcp_server/test_backend_emit_sites.py | 1129 ++++++++++++++++- tests/mcp_server/test_http.py | 6 + .../mcp_server/test_notify_agent_activity.py | 5 + 19 files changed, 2200 insertions(+), 5 deletions(-) create mode 100644 src/osprey/mcp_server/artifact_activity.py create mode 100644 tests/mcp_server/test_artifact_activity_listener.py diff --git a/pyproject.toml b/pyproject.toml index 653a662fd..4a5b813ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -497,6 +497,7 @@ markers = [ "e2e_services: E2E tests requiring local MCP service infrastructure (PostgreSQL, AccelPapers, etc.)", "dockerbuild: E2E tests that run a real docker build (skipped when docker is unavailable)", "real_workspace_watcher: Web-terminal app test that needs a live filesystem observer — opts out of the conftest stub that keeps broadcaster assertions deterministic", + "real_http_posters: MCP-server test of the HTTP posters themselves — opts out of the conftest stub that keeps notify_* POSTs from reaching a live web terminal", ] # Ruff configuration for modern Python linting diff --git a/src/osprey/mcp_server/ariel/tools/entry.py b/src/osprey/mcp_server/ariel/tools/entry.py index 8f580fd7b..cffa063c6 100644 --- a/src/osprey/mcp_server/ariel/tools/entry.py +++ b/src/osprey/mcp_server/ariel/tools/entry.py @@ -6,6 +6,7 @@ attachment limits, logbook name conventions """ +import functools import json import logging import os @@ -13,10 +14,12 @@ from datetime import UTC, datetime from pathlib import Path +import anyio from fastmcp.exceptions import ToolError from osprey.mcp_server.ariel.server import build_entry_url, make_error, mcp, serialize_entry from osprey.mcp_server.ariel.server_context import get_ariel_context +from osprey.mcp_server.http import notify_agent_activity logger = logging.getLogger("osprey.mcp_server.ariel.tools.entry") @@ -375,6 +378,22 @@ async def entry_create( await service.repository.upsert_entry(entry) + # Agent-activity highlight for the ARIEL panel, emitted the moment the + # entry is persisted and before attachments are processed — an + # attachment failure must not lose the signal for an entry that already + # exists. Passive on purpose: unlike the draft branch above, a direct + # write does not steal focus. notify_agent_activity never raises; the + # blocking call runs off the event loop. + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "entry_create", + "panel", + panel="ariel", + detail=entry_id, + ) + ) + # Process attachments if provided attachment_count = 0 if all_file_paths: diff --git a/src/osprey/mcp_server/ariel/tools/publish.py b/src/osprey/mcp_server/ariel/tools/publish.py index b68ffd107..852a36da2 100644 --- a/src/osprey/mcp_server/ariel/tools/publish.py +++ b/src/osprey/mcp_server/ariel/tools/publish.py @@ -1,12 +1,15 @@ """MCP tool: entry_publish — publish an existing ARIEL entry to the facility logbook.""" +import functools import json import logging +import anyio from fastmcp.exceptions import ToolError from osprey.mcp_server.ariel.server import build_entry_url, make_error, mcp from osprey.mcp_server.ariel.server_context import get_ariel_context +from osprey.mcp_server.http import notify_agent_activity from osprey.services.ariel_search.exceptions import AuthenticationRequiredError logger = logging.getLogger("osprey.mcp_server.ariel.tools.publish") @@ -43,6 +46,21 @@ async def entry_publish( result = await service.publish_entry(entry_id, logbook=logbook) + # Agent-activity highlight for the ARIEL panel. Only reached once the + # upstream write succeeded — every refusal (not_found, not_supported, + # auth_required, internal_error) raises out of publish_entry above and + # emits nothing. Passive: no focus steal. notify_agent_activity never + # raises; the blocking call runs off the event loop. + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "entry_publish", + "panel", + panel="ariel", + detail=result.entry_id, + ) + ) + # The just-published entry now carries a facility-assigned id, so the # canonical entry_url is correct at the write-then-link moment. published = { diff --git a/src/osprey/mcp_server/artifact_activity.py b/src/osprey/mcp_server/artifact_activity.py new file mode 100644 index 000000000..5d767d906 --- /dev/null +++ b/src/osprey/mcp_server/artifact_activity.py @@ -0,0 +1,168 @@ +"""Artifact-mutation activity emits for the Web Terminal. + +Every artifact save and delete passes through :class:`~osprey.stores.artifact_store.ArtifactStore`, +which fires the listener seam registered by +:func:`osprey.stores.artifact_store.register_artifact_listener` / +:func:`~osprey.stores.artifact_store.register_artifact_delete_listener`. This +module is the single subscriber that turns those store events into +``/api/agent-activity`` frames, so artifact visibility does not have to be +re-implemented in every tool that writes one. + +Two properties shape the implementation: + +**The caller is never blocked.** ``notify_agent_activity`` performs a blocking +HTTP POST, and ``ArtifactStore.delete_all`` fires the delete listener once per +removed entry — a "clear the gallery" call would otherwise stall for one POST +per artifact, on whatever thread (or event loop) invoked it. The listener +callbacks therefore only enqueue onto a :class:`queue.Queue`; a single daemon +worker thread drains the queue and does the POSTs. + +**Bookkeeping never emits.** An ``execute`` run auto-saves a notebook artifact +and a ``code_output`` record alongside whatever the code deliberately produced. +Emitting for those would turn one agent action into three strip entries, so +they are filtered out here; the run's own channel emit is that action's signal. +Deliberate saves — figures, ``save_artifact()``, the ``create_*`` tools, +``archiver_read`` / ``phoebus_snapshot`` persists — emit one frame per entry. +Accepted edge: a notebook the agent saves ON PURPOSE from inside an ``execute`` +run is indistinguishable from the auto-save (same type, same ``tool_source``) +and is silently dropped. A missed frame for a rare case beats three frames for +every run; the run's own emit still shows the activity. + +Headless dispatch agents register these listeners too. Their web terminal does +not exist, so each notify fails and is swallowed by ``notify_agent_activity`` +(bounded at roughly a second, and only ever on the worker thread). +""" + +from __future__ import annotations + +import logging +import queue +import threading +from typing import TYPE_CHECKING + +from osprey.mcp_server.http import notify_agent_activity + +if TYPE_CHECKING: + from osprey.stores.artifact_store import ArtifactEntry + +logger = logging.getLogger("osprey.mcp_server.artifact_activity") + +#: Activity kind for the ``/api/agent-activity`` route. Unknown kinds are +#: rejected server-side with a 422 that ``notify_agent_activity`` swallows, so +#: this string is asserted in the tests rather than trusted by eye. +_KIND = "artifact" + +#: Tool names reported for the two mutation types. +_SAVE_TOOL = "artifact_save" +_DELETE_TOOL = "artifact_delete" + +#: ``tool_source`` values whose notebook artifacts are auto-saved bookkeeping. +_NOTEBOOK_BOOKKEEPING_SOURCES = frozenset({"execute", "execute_file"}) + +#: Category of the per-execution result record the python executor writes. +_BOOKKEEPING_CATEGORY = "code_output" + +#: Backlog bound. When the web terminal is unreachable each notify costs up to +#: a second, so a bulk ``delete_all`` can outrun the worker; the frames are +#: ephemeral UI signals (the browser-side history ring holds 50) and dropping +#: the overflow is strictly better than growing a queue nobody will read. +_MAX_PENDING = 256 + +_pending: queue.Queue[tuple[str, ArtifactEntry]] = queue.Queue(maxsize=_MAX_PENDING) +_state_lock = threading.Lock() +_worker: threading.Thread | None = None +_registered = False + + +def _is_bookkeeping(entry: ArtifactEntry) -> bool: + """Return True when *entry* is a by-product of a run rather than an action.""" + if entry.category == _BOOKKEEPING_CATEGORY: + return True + return entry.artifact_type == "notebook" and entry.tool_source in _NOTEBOOK_BOOKKEEPING_SOURCES + + +def _detail_for(entry: ArtifactEntry) -> str: + """Human-readable subject for the activity frame.""" + title = entry.title or entry.id + return f"{title} ({entry.tool_source})" if entry.tool_source else title + + +def _drain_pending() -> None: + """Worker loop: POST one activity frame per queued store event.""" + while True: + tool, entry = _pending.get() + try: + notify_agent_activity(tool=tool, kind=_KIND, detail=_detail_for(entry)) + except Exception: + # notify_agent_activity swallows its own failures; this guard only + # keeps an unforeseen error from killing the worker for the process. + logger.debug("artifact activity notify failed", exc_info=True) + finally: + _pending.task_done() + + +def _enqueue(tool: str, entry: ArtifactEntry) -> None: + if _is_bookkeeping(entry): + return + try: + _pending.put_nowait((tool, entry)) + except queue.Full: + # Never wait for room: the caller is a store mutation, not a reporter. + logger.debug("artifact activity backlog full — dropping %s frame", tool) + + +def _on_artifact_saved(entry: ArtifactEntry) -> None: + """Store save listener — enqueue only, never POST on the caller's thread.""" + _enqueue(_SAVE_TOOL, entry) + + +def _on_artifact_deleted(entry: ArtifactEntry) -> None: + """Store delete listener — fires once per entry, including from ``delete_all``.""" + _enqueue(_DELETE_TOOL, entry) + + +def register_artifact_activity_listeners() -> None: + """Subscribe to artifact saves and deletes, at most once per process. + + ``ArtifactStore.register_listener`` appends unconditionally and the store + listener lists live on the class, so a second call would double every + frame. MCP server startup runs repeatedly in-process under test, hence the + flag guard rather than a bare register. + """ + global _worker, _registered + + from osprey.stores.artifact_store import ( + register_artifact_delete_listener, + register_artifact_listener, + ) + + with _state_lock: + if _registered: + return + register_artifact_listener(_on_artifact_saved) + register_artifact_delete_listener(_on_artifact_deleted) + _registered = True + if _worker is None: + _worker = threading.Thread(target=_drain_pending, name="artifact-activity", daemon=True) + _worker.start() + + +def unregister_artifact_activity_listeners() -> None: + """Unsubscribe from the store. No-op when not registered. + + The worker thread is left running: it is a daemon parked on an empty queue, + and keeping it lets a later re-registration reuse it. + """ + global _registered + + from osprey.stores.artifact_store import ( + unregister_artifact_delete_listener, + unregister_artifact_listener, + ) + + with _state_lock: + if not _registered: + return + unregister_artifact_listener(_on_artifact_saved) + unregister_artifact_delete_listener(_on_artifact_deleted) + _registered = False diff --git a/src/osprey/mcp_server/bluesky/tools/authoring.py b/src/osprey/mcp_server/bluesky/tools/authoring.py index 53295f254..33e99e8b4 100644 --- a/src/osprey/mcp_server/bluesky/tools/authoring.py +++ b/src/osprey/mcp_server/bluesky/tools/authoring.py @@ -26,12 +26,30 @@ from __future__ import annotations import json +import logging import anyio from osprey.mcp_server.bluesky.server import mcp from osprey.mcp_server.bluesky.server_context import _http_post_json, bridge_error_message +from osprey.mcp_server.bluesky.tools.draft import _plans_panel_id from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity + +logger = logging.getLogger("osprey.mcp_server.bluesky.tools.authoring") + + +def _notify_authoring_activity(tool: str, detail: str | None) -> None: + """Sync body of the fire-and-forget activity emit (worker thread only). + + Authoring a plan changes what the human's BLUESKY panel lists, so the + highlight targets that panel — the same id draft edits resolve, hence the + shared :func:`~osprey.mcp_server.bluesky.tools.draft._plans_panel_id`. + ``notify_agent_activity`` is blocking and the panel-id lookup reads + config.yml, so both stay off the event loop behind + ``anyio.to_thread.run_sync``. + """ + notify_agent_activity(tool=tool, kind="panel", panel=_plans_panel_id(), detail=detail) # --------------------------------------------------------------------------- @@ -100,6 +118,13 @@ async def write_plan( "Check category/required_devices/writes are present and well-typed.", ], ) + + # The file exists only past the rejection returns above: best-effort + # highlight of the panel that now lists it; must never alter the result. + try: + await anyio.to_thread.run_sync(_notify_authoring_activity, "write_plan", name) + except Exception as exc: + logger.debug("agent-activity emit failed (non-fatal): %s", exc) return json.dumps(resp_body) @@ -154,4 +179,15 @@ async def validate_plan( ) if status != 200: return make_error("bluesky_bridge_error", bridge_error_message(resp_body, status)) + + # A pass is what changes the plan's standing (loadable, enqueueable) and + # what the panel re-renders; a failed validation leaves everything as it + # was, so only a pass is reported. + if resp_body.get("passed"): + try: + await anyio.to_thread.run_sync( + _notify_authoring_activity, "validate_plan", f"validated {name}" + ) + except Exception as exc: + logger.debug("agent-activity emit failed (non-fatal): %s", exc) return json.dumps(resp_body) diff --git a/src/osprey/mcp_server/bluesky/tools/queue.py b/src/osprey/mcp_server/bluesky/tools/queue.py index 10e0903de..b7637d4fc 100644 --- a/src/osprey/mcp_server/bluesky/tools/queue.py +++ b/src/osprey/mcp_server/bluesky/tools/queue.py @@ -723,4 +723,16 @@ async def queue_stop(cancel: bool = False) -> str: status, fallback_hints=["Check queue_list for whether a stop is already pending."], ) + + # The two directions are opposite operations, so they must not share a + # label: rendering a withdrawal as "stop" would tell the operator the queue + # is halting when it has just been released to keep draining. + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "queue_stop", + "run", + detail="stop-withdrawn" if cancel else "stop", + ) + ) return json.dumps(body) diff --git a/src/osprey/mcp_server/phoebus/tools/bridge_tools.py b/src/osprey/mcp_server/phoebus/tools/bridge_tools.py index 14455ee78..60ec801c6 100644 --- a/src/osprey/mcp_server/phoebus/tools/bridge_tools.py +++ b/src/osprey/mcp_server/phoebus/tools/bridge_tools.py @@ -49,6 +49,7 @@ """ import asyncio +import functools import json import logging import os @@ -64,7 +65,12 @@ from fastmcp.exceptions import ToolError from osprey.mcp_server.errors import make_error -from osprey.mcp_server.http import _post_json_with_response, notify_panel_focus, phoebus_bridge_url +from osprey.mcp_server.http import ( + _post_json_with_response, + notify_agent_activity, + notify_panel_focus, + phoebus_bridge_url, +) from osprey.mcp_server.phoebus.server import mcp from osprey.utils.workspace import load_osprey_config, resolve_config_path @@ -647,6 +653,26 @@ async def phoebus_drive( _bridge_error_message(body, status), ["Check the widget reference and that verb/mode are valid."], ) + + # Agent-activity highlight, emitted only when the drive actually reached a + # control. A synthetic 200 with fired=false means the bridge resolved no + # interactive control, so nothing was written — that stays silent. Semantic + # ``type`` is the exception: it writes the widget's PV through the runtime + # and so reports fired=false even though the value landed. Every refusal + # (validation, handle enforcement, unreachable bridge, non-200) returns + # above, so those emit nothing. notify_agent_activity never raises; the + # blocking call runs off the event loop. + fired = bool(body.get("fired")) + if fired or (verb_l == "type" and mode_l == "semantic"): + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "phoebus_drive", + "channel", + detail=f"{verb_l} {widget} on {display}", + ) + ) + return json.dumps( { "status": "success", diff --git a/src/osprey/mcp_server/python_executor/tools/python_execute.py b/src/osprey/mcp_server/python_executor/tools/python_execute.py index a6256948a..df455b258 100644 --- a/src/osprey/mcp_server/python_executor/tools/python_execute.py +++ b/src/osprey/mcp_server/python_executor/tools/python_execute.py @@ -1,9 +1,13 @@ """MCP tool: execute — run user-provided Python code with safety checks.""" +import functools import json import logging +import anyio + from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity from osprey.mcp_server.python_executor.server import mcp from osprey.mcp_server.python_executor.tools._package_inventory import with_live_packages @@ -125,6 +129,33 @@ async def execute( description=description, ) + # Report the write to the Web Terminal once the script has actually been + # handed to the subprocess: writes it performed are already on the machine + # and a mid-run error does not undo them, so a failed run still reports. + # + # `execution_time_seconds` is the launch discriminator — only the subprocess + # path sets it, on both its completed and its timed-out return, while every + # "never launched" outcome comes back from execute_code's setup handler with + # it still None. Every pre-execution gate above returns before this point. + # + # The mode test is the complement of the readonly gate, not equality with + # "readwrite": any mode string that got past that gate let the writes run, + # so it has to be visible. Emitting before build_execution_response keeps + # the report independent of artifact/notebook persistence failures. + if ( + patterns.get("has_writes") + and execution_mode != "readonly" + and exec_result.execution_time_seconds is not None + ): + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "execute", + "channel", + detail="ran a script with control-system writes", + ) + ) + from osprey.mcp_server.python_executor.tools._response_builder import build_execution_response return await build_execution_response( diff --git a/src/osprey/mcp_server/python_executor/tools/python_execute_file.py b/src/osprey/mcp_server/python_executor/tools/python_execute_file.py index a2ca9375f..d8feeba79 100644 --- a/src/osprey/mcp_server/python_executor/tools/python_execute_file.py +++ b/src/osprey/mcp_server/python_executor/tools/python_execute_file.py @@ -1,10 +1,14 @@ """MCP tool: execute_file — run an existing Python file with safety checks.""" +import functools import json import logging from pathlib import Path +import anyio + from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity from osprey.mcp_server.python_executor.server import mcp logger = logging.getLogger("osprey.mcp_server.tools.execute_file") @@ -152,6 +156,24 @@ async def execute_file( description=description, ) + # Same emit contract as the ``execute`` tool — see the comment there for why + # `execution_time_seconds` is the launch discriminator and why the mode test + # is the complement of the readonly gate rather than equality with + # "readwrite". + if ( + patterns.get("has_writes") + and execution_mode != "readonly" + and exec_result.execution_time_seconds is not None + ): + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "execute_file", + "channel", + detail="ran a script with control-system writes", + ) + ) + # Build response using original code (not augmented) for metadata/notebook from osprey.mcp_server.python_executor.tools._response_builder import build_execution_response diff --git a/src/osprey/mcp_server/startup.py b/src/osprey/mcp_server/startup.py index 94a0fcc21..620004a08 100644 --- a/src/osprey/mcp_server/startup.py +++ b/src/osprey/mcp_server/startup.py @@ -74,12 +74,34 @@ def initialize_workspace_singletons() -> None: (``resolve_agent_data_root`` appends ``sessions//`` when ``OSPREY_SESSION_ID`` is set) would make a session's artifacts invisible to the gallery. + + Also subscribes the artifact-activity listeners, so every save and delete + an MCP server performs shows up in the Web Terminal. Registration is + idempotent — this function runs more than once in a process under test. + + The listeners are armed per PROCESS, not per store instance: they hang off + the ArtifactStore class, so every store built in a process that called this + emits. Code paths that run in their own process (dispatch ingest, retention + sweeps, a separately launched gallery) never call this and stay silent. + + .. warning:: + That process boundary is not guaranteed. ``ServerLauncher`` can start + the artifact gallery IN-THREAD inside this very process — the store + auto-launches it on first save when no other process owns the port + (``artifact_store.py`` save paths → ``ensure_artifact_server``). In + that topology a HUMAN deleting an artifact in the gallery UI fires the + same delete listener, and the activity frame is attributed to the + agent. Telling the two apart needs origin plumbing through the store or + the gallery route; until then this is a known limitation, recorded here + rather than papered over with a thread-name guess. """ + from osprey.mcp_server.artifact_activity import register_artifact_activity_listeners from osprey.stores.artifact_store import initialize_artifact_store from osprey.utils.workspace import resolve_shared_data_root with startup_timer("workspace_singletons"): initialize_artifact_store(workspace_root=resolve_shared_data_root()) + register_artifact_activity_listeners() def run_mcp_server(server_module: str) -> None: diff --git a/src/osprey/mcp_server/workspace/tools/lattice_tools.py b/src/osprey/mcp_server/workspace/tools/lattice_tools.py index 1fd6b4e0c..5675c5d7b 100644 --- a/src/osprey/mcp_server/workspace/tools/lattice_tools.py +++ b/src/osprey/mcp_server/workspace/tools/lattice_tools.py @@ -13,14 +13,17 @@ - ``lattice_update_settings``: Deep-merge new settings (partial update). """ +import functools import json import logging import os +import anyio import httpx from fastmcp.exceptions import ToolError from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity from osprey.mcp_server.workspace.server import mcp from osprey.utils.workspace import load_osprey_config @@ -59,6 +62,28 @@ async def _dashboard_request( return resp.json() +async def _notify_lattice(tool: str, detail: str) -> None: + """Report a lattice-dashboard mutation to the Web Terminal activity feed. + + Call only after the dashboard has acknowledged the change — every refusal + path in this module raises out of ``make_error`` before reaching a call + site, so nothing is reported for a mutation that did not happen. + + Args: + tool: Name of the lattice tool that mutated the dashboard. + detail: Short human-readable description of what changed. + """ + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + tool, + "panel", + panel="lattice", + detail=detail, + ) + ) + + @mcp.tool() async def lattice_init(lattice_path: str) -> str: """Load a lattice file into the dashboard. @@ -82,6 +107,7 @@ async def lattice_init(lattice_path: str) -> str: json_body={"lattice_path": lattice_path}, timeout=60.0, ) + await _notify_lattice("lattice_init", lattice_path) return json.dumps( { "status": "ok", @@ -155,6 +181,7 @@ async def lattice_set_param(family: str, value: float) -> str: "/api/state/param", json_body={"family": family, "value": value}, ) + await _notify_lattice("lattice_set_param", f"{family} = {value}") return json.dumps( { "status": "ok", @@ -204,6 +231,7 @@ async def lattice_refresh(figure: str | None = None) -> str: result = await _dashboard_request("POST", "/api/verify") else: result = await _dashboard_request("POST", f"/api/refresh/{figure}") + await _notify_lattice("lattice_refresh", f"recomputing {figure or 'fast figures'}") return json.dumps(result, default=str) except httpx.ConnectError: return make_error( @@ -230,6 +258,7 @@ async def lattice_set_baseline() -> str: """ try: result = await _dashboard_request("POST", "/api/baseline") + await _notify_lattice("lattice_set_baseline", "baseline set") return json.dumps( { "status": "ok", @@ -375,6 +404,8 @@ async def lattice_update_settings(settings: dict) -> str: """ try: result = await _dashboard_request("PUT", "/api/settings", json_body={"settings": settings}) + groups = ", ".join(sorted(str(key) for key in settings)) or "no groups" + await _notify_lattice("lattice_update_settings", f"settings: {groups}") return json.dumps(result, default=str) except httpx.HTTPStatusError as exc: return make_error( @@ -406,6 +437,7 @@ async def lattice_clear_baseline() -> str: """ try: result = await _dashboard_request("DELETE", "/api/baseline") + await _notify_lattice("lattice_clear_baseline", "baseline cleared") return json.dumps(result, default=str) except httpx.ConnectError: return make_error( diff --git a/src/osprey/mcp_server/workspace/tools/screen_capture.py b/src/osprey/mcp_server/workspace/tools/screen_capture.py index 28fdf2201..c705dabb9 100644 --- a/src/osprey/mcp_server/workspace/tools/screen_capture.py +++ b/src/osprey/mcp_server/workspace/tools/screen_capture.py @@ -5,14 +5,17 @@ The tool functions below are thin validation + delegation layers. """ +import functools import json import logging from datetime import UTC, datetime from pathlib import Path +import anyio from fastmcp.exceptions import ToolError from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity from osprey.mcp_server.workspace.server import mcp from osprey.mcp_server.workspace.tools.screen_capture_backends import ( BackendUnavailableError, @@ -262,6 +265,17 @@ async def manage_window( elif action == "resize": await backend.resize_window(app, width, height) + # Only once the backend moved the window: a refused or failed action + # raises out of make_error above and reports nothing. + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "manage_window", + "ui", + detail=f"{action} {app}", + ) + ) + return json.dumps( { "status": "success", diff --git a/src/osprey/mcp_server/workspace/tools/setup.py b/src/osprey/mcp_server/workspace/tools/setup.py index fce0fa107..fc8f45940 100644 --- a/src/osprey/mcp_server/workspace/tools/setup.py +++ b/src/osprey/mcp_server/workspace/tools/setup.py @@ -4,15 +4,18 @@ Used by the /setup-mode skill to help operators troubleshoot setup issues. """ +import functools import json import logging import os import re from pathlib import Path +import anyio from fastmcp.exceptions import ToolError from osprey.mcp_server.errors import make_error +from osprey.mcp_server.http import notify_agent_activity from osprey.mcp_server.workspace.server import mcp from osprey.utils.workspace import load_osprey_config, resolve_config_path @@ -32,6 +35,9 @@ }, } +# Key paths reported to the activity feed with a safety marker +_SAFETY_KEY_PREFIX = "control_system." + # Cold keys whose generic "restart the MCP server" note would understate what is # actually required. `writes_enabled` is enforced at three layers and only the # hook layer re-reads config per call, so a patch alone leaves writes denied. @@ -201,6 +207,49 @@ def _classify_change(file: str, key_path: str) -> str: return "cold — requires MCP server restart (`osprey claude restart` or new session)" +def _activity_detail(file: str, key_path: str) -> str: + """Describe a patch for the activity feed — file and key only, never values. + + Config values are secrets: ``.mcp.json`` carries API keys and tokens, and + the activity ring is persistent and served over HTTP, so neither the old + nor the new value may appear here. ``control_system.*`` paths get a marker + so a safety-relevant change is distinguishable at a glance; the prefix + match is exact-case, like the hot/cold lookups in :func:`_classify_change`. + + Args: + file: Target file name, already validated against ``_PATCHABLE_FILES``. + key_path: Dot-notation path that was patched. + + Returns: + Feed detail string naming the file and key path. + """ + label = f"{file}: {key_path}" + if key_path.startswith(_SAFETY_KEY_PREFIX): + return f"safety config — {label}" + return label + + +async def _notify_patch(file: str, key_path: str) -> None: + """Report an applied patch to the Web Terminal activity feed. + + Call only once the file has been rewritten — every refusal in + :func:`setup_patch` raises out of ``make_error`` before reaching the call + site, so nothing is reported for a patch that did not land. + + Args: + file: Target file name. + key_path: Dot-notation path that was patched. + """ + await anyio.to_thread.run_sync( + functools.partial( + notify_agent_activity, + "setup_patch", + "config", + detail=_activity_detail(file, key_path), + ) + ) + + def _set_nested(data: dict, keys: list[str], value) -> None: """Set a value in a nested dict using a list of keys.""" for key in keys[:-1]: @@ -298,6 +347,8 @@ async def setup_patch(file: str, key_path: str, value: str) -> str: with open(file_path, "w", encoding="utf-8") as f: ryaml.dump(data, f) + await _notify_patch(file, key_path) + note = _classify_change(file, key_path) return json.dumps( diff --git a/tests/mcp_server/bluesky/test_draft_tools_emit.py b/tests/mcp_server/bluesky/test_draft_tools_emit.py index 87b21dd1f..1a551934b 100644 --- a/tests/mcp_server/bluesky/test_draft_tools_emit.py +++ b/tests/mcp_server/bluesky/test_draft_tools_emit.py @@ -12,6 +12,11 @@ into its own namespace (``from osprey.mcp_server.http import ...``), so the patch must target ``osprey.mcp_server.bluesky.tools.draft``, not ``osprey.mcp_server.http``. + +Later sections cover the emits on the other bluesky tool modules (``queue_stop`` +and the authoring tools). Each is delimited by a ``#####`` banner, keeps its own +helpers and its own module-path constant, and names every test after the tool it +covers so a ``-k`` run selects the whole section. """ from __future__ import annotations @@ -20,6 +25,7 @@ from unittest.mock import patch import pytest +import yaml from osprey.mcp_server.bluesky.server_context import ( initialize_server_context, @@ -206,3 +212,260 @@ async def test_clear_draft_result_identical_when_web_terminal_down(monkeypatch): with patch(f"{_MOD}._http_delete_json", return_value=(200, _CLEAR_RESP)): result = await _clear_fn()() assert extract_response_dict(result) == _CLEAR_RESP + + +# ######################################################################### +# queue_stop — the emit on the halting surface +# +# A stop and a withdrawal are opposite operations that share one tool and one +# success return, so the emit's ``detail`` is the only thing distinguishing +# them in the web terminal. These tests pin both strings exactly, and pin that +# the three refusals ahead of the mutation stay silent. +# ######################################################################### + +_QUEUE_MOD = "osprey.mcp_server.bluesky.tools.queue" + +_QUEUE_STOP_TOKEN = "queue-stop-launch-token" +_STOP_RESP = {"stop_pending": True, "msg": "queue will stop after the running item"} +_WITHDRAWN_RESP = {"stop_pending": False, "msg": "pending stop withdrawn"} + + +def _queue_stop_fn(): + from osprey.mcp_server.bluesky.tools import queue + + return get_tool_fn(queue.queue_stop) + + +def _queue_stop_posture(tmp_path, monkeypatch, *, writes: bool, token: str | None) -> None: + """Re-arm the bridge context for a queue_stop test: writes on/off, token set/unset.""" + (tmp_path / "config.yml").write_text(yaml.dump({"control_system": {"writes_enabled": writes}})) + if token is None: + monkeypatch.delenv("BLUESKY_LAUNCH_TOKEN", raising=False) + else: + monkeypatch.setenv("BLUESKY_LAUNCH_TOKEN", token) + initialize_server_context() + + +async def test_queue_stop_plain_stop_emits_detail_stop(tmp_path, monkeypatch): + """The ungated halt still reports itself: one run-kind emit, detail 'stop'.""" + _queue_stop_posture(tmp_path, monkeypatch, writes=False, token=None) + with ( + patch(f"{_QUEUE_MOD}._http_post_json", return_value=(200, _STOP_RESP)), + patch(f"{_QUEUE_MOD}.notify_agent_activity") as notify, + ): + result = await _queue_stop_fn()() + + notify.assert_called_once_with("queue_stop", "run", detail="stop") + assert extract_response_dict(result) == _STOP_RESP + + +async def test_queue_stop_withdrawal_emits_detail_stop_withdrawn(tmp_path, monkeypatch): + """A withdrawal must never render as a stop — the operator reads this string + to decide whether the queue is halting or draining toward hardware.""" + _queue_stop_posture(tmp_path, monkeypatch, writes=True, token=_QUEUE_STOP_TOKEN) + with ( + patch(f"{_QUEUE_MOD}._http_post_json", return_value=(200, _WITHDRAWN_RESP)), + patch(f"{_QUEUE_MOD}.notify_agent_activity") as notify, + ): + result = await _queue_stop_fn()(cancel=True) + + notify.assert_called_once_with("queue_stop", "run", detail="stop-withdrawn") + assert extract_response_dict(result) == _WITHDRAWN_RESP + + +async def test_queue_stop_withdrawal_refused_for_writes_disabled_does_not_emit( + tmp_path, monkeypatch +): + """Refused before the mutation: nothing was withdrawn, so nothing is reported.""" + _queue_stop_posture(tmp_path, monkeypatch, writes=False, token=_QUEUE_STOP_TOKEN) + with ( + patch(f"{_QUEUE_MOD}._http_post_json") as post, + patch(f"{_QUEUE_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="writes_disabled"): + await _queue_stop_fn()(cancel=True) + + post.assert_not_called() + notify.assert_not_called() + + +async def test_queue_stop_withdrawal_refused_without_a_token_does_not_emit(tmp_path, monkeypatch): + _queue_stop_posture(tmp_path, monkeypatch, writes=True, token=None) + with ( + patch(f"{_QUEUE_MOD}._http_post_json") as post, + patch(f"{_QUEUE_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="launch_token_required"): + await _queue_stop_fn()(cancel=True) + + post.assert_not_called() + notify.assert_not_called() + + +async def test_queue_stop_bridge_refusal_does_not_emit(tmp_path, monkeypatch): + """The manager refused, so the queue's state is unchanged — no activity.""" + _queue_stop_posture(tmp_path, monkeypatch, writes=False, token=None) + body = {"detail": {"code": "queue_request_rejected", "detail": "no stop is pending"}} + with ( + patch(f"{_QUEUE_MOD}._http_post_json", return_value=(409, body)), + patch(f"{_QUEUE_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="queue_request_rejected"): + await _queue_stop_fn()() + + notify.assert_not_called() + + +async def test_queue_stop_result_identical_when_web_terminal_down(tmp_path, monkeypatch): + """The REAL notify helper against a dead port leaves the halt's result intact.""" + _queue_stop_posture(tmp_path, monkeypatch, writes=False, token=None) + monkeypatch.setenv("OSPREY_WEB_PORT", str(_dead_port())) + with patch(f"{_QUEUE_MOD}._http_post_json", return_value=(200, _STOP_RESP)): + result = await _queue_stop_fn()() + assert extract_response_dict(result) == _STOP_RESP + + +# ######################################################################### +# write_plan / validate_plan — the emits on the authoring surface +# +# Both tools reach no hardware and are never kill-switch-gated, so the only +# thing separating an emit from silence is whether the plan file actually +# changed standing: write_plan mutates on any 200, validate_plan only on a +# PASS. A failed validation writes nothing the human's panel would re-render, +# so it must stay silent even though its HTTP call succeeded. +# ######################################################################### + +_AUTHORING_MOD = "osprey.mcp_server.bluesky.tools.authoring" + +_WRITE_RESP = {"name": "tiny", "content_hash": "deadbeef"} +_PASS_RESP = {"passed": True, "reasons": [], "content_hash": "deadbeef", "upload": {}} +_FAIL_RESP = {"passed": False, "reasons": ["import of 'os' is not allowed"], "upload": {}} + +_WRITE_ARGS = { + "name": "tiny", + "category": "accelerator", + "required_devices": ["correctors"], + "writes": False, + "body": "def build_plan(devices, params):\n yield\n", +} + + +def _write_plan_fn(): + from osprey.mcp_server.bluesky.tools import authoring + + return get_tool_fn(authoring.write_plan) + + +def _validate_plan_fn(): + from osprey.mcp_server.bluesky.tools import authoring + + return get_tool_fn(authoring.validate_plan) + + +async def test_write_plan_success_emits_one_plans_panel_activity(): + """Authoring changes what the BLUESKY panel lists, so the highlight lands + on that panel — same id the draft emits resolve, not a second spelling.""" + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _WRITE_RESP)), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + result = await _write_plan_fn()(**_WRITE_ARGS) + + notify.assert_called_once_with(tool="write_plan", kind="panel", panel="bluesky", detail="tiny") + assert extract_response_dict(result) == _WRITE_RESP + + +async def test_write_plan_panel_id_resolved_from_web_panels_config(): + """The panel id is config-resolved for authoring too — a facility that + registered the panel under its own key gets that key in the emit.""" + config = {"web": {"panels": {"operator-plan": {"path": "/bluesky/"}}}} + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _WRITE_RESP)), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + patch("osprey.utils.workspace.load_osprey_config", return_value=config), + ): + await _write_plan_fn()(**_WRITE_ARGS) + + assert notify.call_args.kwargs["panel"] == "operator-plan" + + +async def test_write_plan_rejected_does_not_emit(): + """Refused before the file was written — no plan exists to light up.""" + body = {"detail": "invalid plan name '1bad': must be a valid Python identifier"} + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(400, body)), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="plan_write_rejected"): + await _write_plan_fn()(**{**_WRITE_ARGS, "name": "1bad"}) + + notify.assert_not_called() + + +async def test_validate_plan_pass_emits_the_validated_detail(): + """A pass is what makes the plan loadable and enqueueable; the detail names + the plan so the operator can tell which one just cleared.""" + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _PASS_RESP)), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + result = await _validate_plan_fn()(name="tiny") + + notify.assert_called_once_with( + tool="validate_plan", kind="panel", panel="bluesky", detail="validated tiny" + ) + assert extract_response_dict(result) == _PASS_RESP + + +async def test_validate_plan_failure_does_not_emit(): + """A 200 carrying passed=false is still a non-event: the plan's standing is + unchanged, so reporting it would announce a validation that did not happen.""" + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _FAIL_RESP)), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + result = await _validate_plan_fn()(name="tiny") + + notify.assert_not_called() + assert extract_response_dict(result) == _FAIL_RESP + + +async def test_validate_plan_unknown_name_does_not_emit(): + with ( + patch( + f"{_AUTHORING_MOD}._http_post_json", + return_value=(404, {"detail": "unknown session plan 'nope'"}), + ), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="unknown_session_plan"): + await _validate_plan_fn()(name="nope") + + notify.assert_not_called() + + +async def test_validate_plan_bridge_error_does_not_emit(): + with ( + patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(503, {"detail": "bridge down"})), + patch(f"{_AUTHORING_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="bluesky_bridge_error"): + await _validate_plan_fn()(name="tiny") + + notify.assert_not_called() + + +async def test_write_plan_result_identical_when_web_terminal_down(monkeypatch): + """The REAL notify helper against a dead port leaves the authored plan's + result intact — the emit is best-effort, never load-bearing.""" + monkeypatch.setenv("OSPREY_WEB_PORT", str(_dead_port())) + with patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _WRITE_RESP)): + result = await _write_plan_fn()(**_WRITE_ARGS) + assert extract_response_dict(result) == _WRITE_RESP + + +async def test_validate_plan_result_identical_when_web_terminal_down(monkeypatch): + monkeypatch.setenv("OSPREY_WEB_PORT", str(_dead_port())) + with patch(f"{_AUTHORING_MOD}._http_post_json", return_value=(200, _PASS_RESP)): + result = await _validate_plan_fn()(name="tiny") + assert extract_response_dict(result) == _PASS_RESP diff --git a/tests/mcp_server/conftest.py b/tests/mcp_server/conftest.py index ae9534046..9865b61e8 100644 --- a/tests/mcp_server/conftest.py +++ b/tests/mcp_server/conftest.py @@ -141,6 +141,57 @@ def _reset_all(): _cfg._config_cache.update(saved_cache) +@pytest.fixture(autouse=True) +def _unregister_artifact_activity(): + """Disarm the artifact-activity listener around every test in this directory. + + ``initialize_workspace_singletons()`` subscribes the listener to the + ArtifactStore *class*, so any test that calls it leaves every later test in + the same worker emitting real ``/api/agent-activity`` POSTs at whatever is + listening on the web-terminal port. Unregistering on both sides keeps that + process-global arming inside the test that asked for it. + """ + from osprey.mcp_server.artifact_activity import unregister_artifact_activity_listeners + + unregister_artifact_activity_listeners() + yield + unregister_artifact_activity_listeners() + + +@pytest.fixture(autouse=True) +def _block_web_terminal_posts(request, monkeypatch): + """Keep the notify_* helpers' HTTP POSTs inside the test process. + + Every ``notify_*`` helper in :mod:`osprey.mcp_server.http` opens a real + socket to the resolved web-terminal port. On CI nothing is listening and the + connection is merely refused, but on a developer box a live web terminal is + — and then the unit suite drives the operator's actual UI, glowing tiles and + filling the activity strip. Both posters are stubbed here to the outcome + they already produce when the terminal is down: ``post_json`` swallows and + returns ``None``; ``_post_json_with_response`` raises ``URLError`` so + ``notify_panel_register`` / ``notify_panel_arrange`` take their existing + unreachable branch. Patching only ``post_json`` would miss those two. + + Patched in the ``http`` module's own namespace, which is where the helpers + resolve them from, so it holds however a tool module imported the helper. + Tests that assert on emits patch ``notify_agent_activity`` at their own call + site — above this seam — and are unaffected. ``test_http.py`` exercises the + posters themselves and opts out with the ``real_http_posters`` marker. + """ + if request.node.get_closest_marker("real_http_posters"): + return + + import urllib.error + + from osprey.mcp_server import http as _http + + def _unreachable(url, payload, *, timeout=3): + raise urllib.error.URLError("web terminal POSTs are blocked in unit tests") + + monkeypatch.setattr(_http, "post_json", lambda *args, **kwargs: None) + monkeypatch.setattr(_http, "_post_json_with_response", _unreachable) + + @pytest.fixture def init_registry(tmp_path, monkeypatch): """Initialize the MCP registry after chdir and config setup. diff --git a/tests/mcp_server/test_artifact_activity_listener.py b/tests/mcp_server/test_artifact_activity_listener.py new file mode 100644 index 000000000..6343dce59 --- /dev/null +++ b/tests/mcp_server/test_artifact_activity_listener.py @@ -0,0 +1,297 @@ +"""Tests for the artifact-mutation activity listener. + +The listener is the single emit site standing in for every artifact-writing +tool, so the properties worth pinning are the ones a per-tool emit would not +have given us for free: + +* it registers exactly once even though MCP startup runs repeatedly in-process; +* one ``execute`` run that produces a figure emits ONE frame, not three (the + auto-saved notebook and the ``code_output`` record are bookkeeping); +* the store callback itself performs no HTTP — the blocking POST happens on the + worker thread, so ``delete_all`` over a full gallery does not stall the + caller; +* a process that never registers (gallery, retention sweep) emits nothing. + +``notify_agent_activity`` is patched at this module's import site because +``osprey.mcp_server.http`` has two posters and the listener names the notify +helper directly. +""" + +from __future__ import annotations + +import queue +import threading +import time +from unittest.mock import patch + +import pytest + +from osprey.mcp_server import artifact_activity +from osprey.stores.artifact_store import ArtifactStore + +DRAIN_TIMEOUT = 5.0 + + +@pytest.fixture +def project(tmp_path, monkeypatch): + """Minimal deployed project: config.yml with artifact auto-launch disabled.""" + (tmp_path / "config.yml").write_text( + "agent_data:\n base_dir: ./_agent_data\nartifact_server:\n auto_launch: false\n" + ) + monkeypatch.setenv("OSPREY_CONFIG", str(tmp_path / "config.yml")) + monkeypatch.chdir(tmp_path) + return tmp_path + + +@pytest.fixture(autouse=True) +def clean_registration(): + """Listener registration is process-global — never let it leak between tests.""" + artifact_activity.unregister_artifact_activity_listeners() + yield + artifact_activity.unregister_artifact_activity_listeners() + + +@pytest.fixture +def notified(): + """Patched notify recording ``(kwargs, thread_ident)`` per emitted frame.""" + calls: list[tuple[dict, int]] = [] + + def record(**kwargs): + calls.append((kwargs, threading.get_ident())) + + with patch.object(artifact_activity, "notify_agent_activity", side_effect=record): + yield calls + + +def wait_drained(timeout: float = DRAIN_TIMEOUT) -> None: + """Block until the worker has processed every queued event. + + ``Queue.join()`` has no timeout, so it runs on a throwaway thread and the + test fails loudly instead of hanging the suite if the worker ever wedges. + """ + done = threading.Event() + + def join_then_signal(): + artifact_activity._pending.join() + done.set() + + threading.Thread(target=join_then_signal, daemon=True).start() + assert done.wait(timeout), "artifact activity queue never drained" + + +def save_figure(store: ArtifactStore, title: str = "Beam profile") -> None: + """A deliberate save: the figure the executed code chose to keep.""" + store.save_file( + file_content=b"\x89PNG fake", + filename="beam.png", + artifact_type="image", + title=title, + mime_type="image/png", + tool_source="execute", + ) + + +def save_run_bookkeeping(store: ArtifactStore) -> None: + """The two records every ``execute`` run writes on its own.""" + store.save_file( + file_content=b"{}", + filename="run.ipynb", + artifact_type="notebook", + title="Notebook: plot the orbit", + mime_type="application/x-ipynb+json", + tool_source="execute", + ) + store.save_data( + tool="execute", + data={"stdout": "done"}, + title="plot the orbit", + category="code_output", + ) + + +@pytest.mark.unit +def test_repeated_startup_registers_exactly_once(project): + """Startup runs 3x in-process under test; the store appends unconditionally.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + for _ in range(3): + initialize_workspace_singletons() + + assert ArtifactStore._listeners.count(artifact_activity._on_artifact_saved) == 1 + assert ArtifactStore._delete_listeners.count(artifact_activity._on_artifact_deleted) == 1 + + +@pytest.mark.unit +def test_execute_run_with_figure_emits_one_frame(project, notified): + """The flood case: figure + auto-notebook + code_output record → ONE frame.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + save_figure(store) + save_run_bookkeeping(store) + wait_drained() + + assert len(notified) == 1 + kwargs, _ = notified[0] + assert kwargs["kind"] == "artifact" + assert kwargs["tool"] == "artifact_save" + assert "Beam profile" in kwargs["detail"] + assert "execute" in kwargs["detail"] + + +@pytest.mark.unit +def test_deliberately_saved_notebook_still_emits(project, notified): + """The filter keys on the auto-save sources, not on the notebook type alone.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + store.save_file( + file_content=b"{}", + filename="analysis.ipynb", + artifact_type="notebook", + title="Orbit analysis", + mime_type="application/x-ipynb+json", + tool_source="artifact_save", + ) + wait_drained() + + assert len(notified) == 1 + assert "Orbit analysis" in notified[0][0]["detail"] + + +@pytest.mark.unit +def test_code_output_record_never_emits(project, notified): + """A ``code_output`` record is bookkeeping whatever wrote it.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + store.save_data( + tool="execute_file", + data={"stdout": ""}, + title="script run", + category="code_output", + ) + wait_drained() + + assert notified == [] + + +@pytest.mark.unit +def test_delete_all_emits_per_surviving_entry(project, notified): + """Deletes emit one frame per entry, under the same bookkeeping filter.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + save_figure(store, title="Orbit X") + save_figure(store, title="Orbit Y") + save_run_bookkeeping(store) + wait_drained() + notified.clear() + + store.delete_all() + wait_drained() + + assert [kwargs["tool"] for kwargs, _ in notified] == ["artifact_delete"] * 2 + details = " ".join(kwargs["detail"] for kwargs, _ in notified) + assert "Orbit X" in details and "Orbit Y" in details + + +@pytest.mark.unit +def test_notify_runs_on_the_worker_not_the_caller(project, notified): + """The store callback must not do HTTP on the thread that saved.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + save_figure(store) + wait_drained() + + assert len(notified) == 1 + _, ident = notified[0] + assert ident != threading.get_ident() + + +@pytest.mark.unit +def test_delete_all_returns_while_notifies_are_still_blocked(project): + """A slow web terminal must not stall a gallery-wide delete.""" + from osprey.mcp_server.startup import initialize_workspace_singletons + + initialize_workspace_singletons() + store = ArtifactStore(workspace_root=project / "_agent_data") + + release = threading.Event() + entered = threading.Event() + + def block(**_kwargs): + entered.set() + release.wait(DRAIN_TIMEOUT) + + with patch.object(artifact_activity, "notify_agent_activity", side_effect=block): + for i in range(5): + save_figure(store, title=f"Orbit {i}") + assert entered.wait(DRAIN_TIMEOUT), "worker never picked up the first event" + + t0 = time.perf_counter() + store.delete_all() + elapsed = time.perf_counter() - t0 + + release.set() + # Drain before the patch lifts, so no leftover event reaches the real + # (network-touching) notify. + wait_drained() + + # The worker is parked inside a notify for the whole call; delete_all fires + # the listener five more times and must still return promptly. + assert elapsed < 1.0, f"delete_all blocked on the notify path ({elapsed:.2f}s)" + + +@pytest.mark.unit +@pytest.mark.timeout(10) +def test_full_backlog_drops_instead_of_waiting(monkeypatch): + """A stalled worker must never turn the enqueue into a blocking put. + + The timeout is the real assertion for a ``put_nowait`` → ``put`` regression: + a blocking put on a full queue with no worker draining it would hang here + forever, and the guard turns that into a failure. ``pytest-timeout`` is a + declared dev dependency (pyproject.toml), so the marker is always live. + """ + from osprey.stores.artifact_store import ArtifactEntry + + monkeypatch.setattr(artifact_activity, "_pending", queue.Queue(maxsize=1)) + entry = ArtifactEntry( + id="abc", + artifact_type="image", + title="Orbit", + description="", + filename="abc_orbit.png", + mime_type="image/png", + size_bytes=1, + timestamp="2026-01-01T00:00:00Z", + tool_source="execute", + ) + + for _ in range(3): + artifact_activity._on_artifact_saved(entry) + + assert artifact_activity._pending.qsize() == 1 + + +@pytest.mark.unit +def test_unregistered_process_emits_nothing(project, notified): + """Gallery / retention processes build their own store and never register.""" + store = ArtifactStore(workspace_root=project / "_agent_data") + + save_figure(store) + store.delete_all() + wait_drained() + + assert notified == [] diff --git a/tests/mcp_server/test_backend_emit_sites.py b/tests/mcp_server/test_backend_emit_sites.py index 555c3af8b..017a8c0c1 100644 --- a/tests/mcp_server/test_backend_emit_sites.py +++ b/tests/mcp_server/test_backend_emit_sites.py @@ -1,9 +1,13 @@ """Agent-activity emit sites for backend-direct tools. -Verifies that channel_write, the bluesky queue tools, and the artifact focus tools report -agent activity via ``notify_agent_activity`` — and, just as important, that -refusal paths emit NOTHING and that tool results are unchanged when the web -terminal is down. +Verifies that the mutating backend tools report agent activity via +``notify_agent_activity`` — and, just as important, that refusal paths emit +NOTHING and that tool results are unchanged when the web terminal is down. + +Layout: one ``# ── ──`` section per tool family, each owning its +own helpers and fixtures, followed by a trailing cross-cutting "terminal down" +section. New emit sites append a new section immediately BEFORE that trailing +section; shared helpers stay generic and live at module top. Patch seam: each tool module imports ``notify_agent_activity`` directly, so the mock must target the caller's namespace (e.g. @@ -11,6 +15,7 @@ not ``osprey.mcp_server.http``. """ +import contextlib import socket from unittest.mock import AsyncMock, MagicMock, patch @@ -28,6 +33,14 @@ _CW_MOD = "osprey.mcp_server.control_system.tools.channel_write" _QUEUE_MOD = "osprey.mcp_server.bluesky.tools.queue" _FOCUS_MOD = "osprey.mcp_server.workspace.tools.focus_tools" +_ARIEL_ENTRY_MOD = "osprey.mcp_server.ariel.tools.entry" +_ARIEL_PUBLISH_MOD = "osprey.mcp_server.ariel.tools.publish" +_PHOEBUS_MOD = "osprey.mcp_server.phoebus.tools.bridge_tools" +_EXECUTE_MOD = "osprey.mcp_server.python_executor.tools.python_execute" +_EXECUTE_FILE_MOD = "osprey.mcp_server.python_executor.tools.python_execute_file" +_LATTICE_MOD = "osprey.mcp_server.workspace.tools.lattice_tools" +_SETUP_MOD = "osprey.mcp_server.workspace.tools.setup" +_SCREEN_MOD = "osprey.mcp_server.workspace.tools.screen_capture" def _free_port() -> int: @@ -375,6 +388,1114 @@ async def test_artifact_focus_not_found_no_emit(tmp_path, monkeypatch): notify.assert_not_called() +# ── ariel entry_create / entry_publish ────────────────────────────────────── + + +@pytest.fixture +def _ariel_context(tmp_path, monkeypatch): + """Initialize the ARIEL MCP singleton against a throwaway config.yml. + + The singleton is reset afterwards so ARIEL state never leaks into the other + sections of this module. + """ + from osprey.mcp_server.ariel.server_context import ( + initialize_ariel_context, + reset_ariel_context, + ) + + monkeypatch.chdir(tmp_path) + (tmp_path / "config.yml").write_text( + yaml.dump({"ariel": {"database": {"uri": "postgresql://localhost/test"}}}) + ) + initialize_ariel_context() + yield + reset_ariel_context() + + +def _patch_ariel_service(mock_service): + """Patch ARIELContext.service to hand every tool the same mock service.""" + return patch( + "osprey.mcp_server.ariel.server_context.ARIELContext.service", + new=AsyncMock(return_value=mock_service), + ) + + +def _get_ariel_tool(module_name: str, tool_name: str): + import importlib + + module = importlib.import_module(f"osprey.mcp_server.ariel.tools.{module_name}") + return get_tool_fn(getattr(module, tool_name)) + + +async def test_ariel_entry_create_direct_emits_panel(_ariel_context): + """A direct (non-draft) write emits one passive panel activity for ARIEL.""" + mock_service = AsyncMock() + mock_service.repository.upsert_entry.return_value = None + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_ENTRY_MOD}.notify_agent_activity") as notify, + ): + result = await _get_ariel_tool("entry", "entry_create")( + subject="Beam lost", details="Injector trip at 03:12", draft=False + ) + + data = extract_response_dict(result) + entry_id = data["entry_id"] + assert notify.call_count == 1 + assert notify.call_args.args[:2] == ("entry_create", "panel") + assert notify.call_args.kwargs["panel"] == "ariel" + assert notify.call_args.kwargs["detail"] == entry_id + + +async def test_ariel_entry_create_direct_does_not_steal_focus(_ariel_context): + """The direct-write branch reports activity passively — no panel focus. + + The draft branch deliberately focuses the ARIEL panel so a human can finish + the entry; a direct write has nothing for the human to do, so stealing the + active panel would be a regression. + """ + mock_service = AsyncMock() + mock_service.repository.upsert_entry.return_value = None + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_ENTRY_MOD}.notify_agent_activity"), + patch("osprey.mcp_server.http.notify_panel_focus") as focus, + ): + await _get_ariel_tool("entry", "entry_create")( + subject="Beam lost", details="Injector trip at 03:12", draft=False + ) + + focus.assert_not_called() + + +async def test_ariel_entry_create_emits_before_attachment_failure(_ariel_context, tmp_path): + """The entry is persisted, so a later attachment failure must not lose the emit. + + This is why the emit sits between the upsert and attachment processing: the + tool call ends in an error envelope here, but the entry really does exist. + """ + attachment = tmp_path / "trip.log" + attachment.write_text("trip") + + mock_service = AsyncMock() + mock_service.repository.upsert_entry.return_value = None + + with ( + _patch_ariel_service(mock_service), + patch( + "osprey.services.ariel_search.attachments.process_attachments_for_entry", + new=AsyncMock(side_effect=RuntimeError("attachment store offline")), + ), + patch(f"{_ARIEL_ENTRY_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="internal_error"): + await _get_ariel_tool("entry", "entry_create")( + subject="Beam lost", + details="Injector trip at 03:12", + file_paths=[str(attachment)], + draft=False, + ) + + assert notify.call_count == 1 + assert notify.call_args.kwargs["panel"] == "ariel" + + +async def test_ariel_entry_create_validation_refusal_no_emit(_ariel_context): + """Argument validation refuses before any write, so nothing is emitted.""" + with patch(f"{_ARIEL_ENTRY_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="validation_error"): + await _get_ariel_tool("entry", "entry_create")( + subject=" ", details="Injector trip at 03:12", draft=False + ) + + notify.assert_not_called() + + +async def test_ariel_entry_create_upsert_failure_no_emit(_ariel_context): + """A failed upsert means no entry exists — emit nothing.""" + mock_service = AsyncMock() + mock_service.repository.upsert_entry.side_effect = RuntimeError("database unreachable") + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_ENTRY_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="internal_error"): + await _get_ariel_tool("entry", "entry_create")( + subject="Beam lost", details="Injector trip at 03:12", draft=False + ) + + notify.assert_not_called() + + +async def test_ariel_entry_publish_success_emits_facility_id(_ariel_context): + """A successful upstream publish emits the facility-assigned entry id.""" + from osprey.services.ariel_search.models import FacilityEntryCreateResult, SyncStatus + + mock_service = AsyncMock() + mock_service.publish_entry.return_value = FacilityEntryCreateResult( + entry_id="published-001", + source_system="ALS eLog", + sync_status=SyncStatus.SYNCED, + message="Published successfully", + ) + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_PUBLISH_MOD}.notify_agent_activity") as notify, + ): + result = await _get_ariel_tool("publish", "entry_publish")( + entry_id="e1", logbook="Operations" + ) + + data = extract_response_dict(result) + assert data["entry_id"] == "published-001" + assert notify.call_count == 1 + assert notify.call_args.args[:2] == ("entry_publish", "panel") + assert notify.call_args.kwargs["panel"] == "ariel" + assert notify.call_args.kwargs["detail"] == "published-001" + + +async def test_ariel_entry_publish_validation_refusal_no_emit(_ariel_context): + """An empty entry_id is refused before the service is touched.""" + with patch(f"{_ARIEL_PUBLISH_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="validation_error"): + await _get_ariel_tool("publish", "entry_publish")(entry_id="") + + notify.assert_not_called() + + +async def test_ariel_entry_publish_not_found_no_emit(_ariel_context): + """Nothing was published, so nothing is emitted.""" + mock_service = AsyncMock() + mock_service.publish_entry.side_effect = KeyError("Entry e99 not found") + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_PUBLISH_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="not_found"): + await _get_ariel_tool("publish", "entry_publish")(entry_id="e99") + + notify.assert_not_called() + + +async def test_ariel_entry_publish_auth_required_no_emit(_ariel_context): + """A credential refusal blocks the upstream write — emit nothing.""" + from osprey.services.ariel_search.exceptions import AuthenticationRequiredError + + mock_service = AsyncMock() + mock_service.publish_entry.side_effect = AuthenticationRequiredError( + "OLOG publishing requires credentials.", source_system="ALS eLog" + ) + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_PUBLISH_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="auth_required"): + await _get_ariel_tool("publish", "entry_publish")(entry_id="e1") + + notify.assert_not_called() + + +async def test_ariel_entry_publish_not_supported_no_emit(_ariel_context): + """An adapter without write support never publishes — emit nothing.""" + mock_service = AsyncMock() + mock_service.publish_entry.side_effect = NotImplementedError( + "Adapter does not support writing entries" + ) + + with ( + _patch_ariel_service(mock_service), + patch(f"{_ARIEL_PUBLISH_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="not_supported"): + await _get_ariel_tool("publish", "entry_publish")(entry_id="e1") + + notify.assert_not_called() + + +# ── phoebus_drive ─────────────────────────────────────────────────────────── +# +# Bridge contract this section pins: on HTTP 200 the bridge reports ``fired``, +# which is true only when a real interactive control was driven. A synthetic +# drive that resolves no control still answers 200 with ``fired: false`` and +# nothing was written — that must stay silent. A semantic ``type`` writes the +# widget's PV through the runtime without firing a GUI control, so it reports +# ``fired: false`` even though the value landed — that must emit. + + +@pytest.fixture +def _phoebus_active_display_allowed(monkeypatch): + """Pin require-handle mode OFF so ``display="active"`` reaches the bridge. + + Without this the setting falls through to whatever config.yml the test + process resolves, which would make the emit paths depend on ambient state. + """ + monkeypatch.setenv("PHOEBUS_REQUIRE_HANDLE", "0") + + +def _get_phoebus_drive(): + from osprey.mcp_server.phoebus.tools.bridge_tools import phoebus_drive + + return get_tool_fn(phoebus_drive) + + +def _phoebus_bridge(status: int, body: dict): + """Patch the drive HTTP boundary with one canned bridge response.""" + return patch(f"{_PHOEBUS_MOD}._http_post_drive", return_value=(status, body)) + + +async def test_phoebus_drive_click_fired_emits(_phoebus_active_display_allowed): + """A click that fired a control wrote to the machine — emit once, kind channel.""" + with ( + _phoebus_bridge(200, {"fired": True, "detail": "fired via ButtonBase.fire()"}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + result = await _get_phoebus_drive()(widget="SetButton", verb="click") + + assert extract_response_dict(result)["fired"] is True + notify.assert_called_once_with("phoebus_drive", "channel", detail="click SetButton on active") + + +async def test_phoebus_drive_synthetic_type_fired_emits(): + """A synthetic type that fired echoes the normalized verb and the display ref.""" + with ( + _phoebus_bridge(200, {"fired": True, "detail": "committed 42"}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + await _get_phoebus_drive()(widget="Setpoint", verb="TYPE", value="42", display="handle:d-3") + + notify.assert_called_once_with("phoebus_drive", "channel", detail="type Setpoint on handle:d-3") + + +async def test_phoebus_drive_synthetic_200_not_fired_no_emit(_phoebus_active_display_allowed): + """Bridge contract: a synthetic 200 with fired=false resolved no control — no write.""" + with ( + _phoebus_bridge(200, {"fired": False, "detail": "no interactive control resolved"}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + result = await _get_phoebus_drive()(widget="Readback", verb="type", value="42") + + assert extract_response_dict(result)["fired"] is False + notify.assert_not_called() + + +@pytest.mark.parametrize("verb,mode", [("type", "semantic"), ("TYPE", "SEMANTIC")]) +async def test_phoebus_drive_semantic_type_not_fired_emits( + verb, mode, _phoebus_active_display_allowed +): + """Bridge contract: semantic type writes the PV directly and reports fired=false — emit. + + Parametrized over verb/mode case because the emit condition compares the + NORMALIZED ``verb_l``/``mode_l``: comparing the raw arguments would silence + this write for the equally supported upper-case spelling. + """ + with ( + _phoebus_bridge(200, {"fired": False, "detail": "wrote PV SR:CORR:SP"}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + await _get_phoebus_drive()(widget="Setpoint", verb=verb, value="1.5", mode=mode) + + notify.assert_called_once_with("phoebus_drive", "channel", detail="type Setpoint on active") + + +async def test_phoebus_drive_semantic_click_bypass_no_emit(_phoebus_active_display_allowed): + """Semantic mode bypasses click entirely — nothing was driven, so emit nothing.""" + with ( + _phoebus_bridge(200, {"fired": False, "detail": "semantic mode bypasses click"}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + await _get_phoebus_drive()(widget="SetButton", verb="click", mode="semantic") + + notify.assert_not_called() + + +@pytest.mark.parametrize( + "kwargs", + [ + {"widget": "0", "verb": "frobnicate"}, + {"widget": "0", "verb": "click", "mode": "warp"}, + {"widget": "0", "verb": "type"}, + ], +) +async def test_phoebus_drive_validation_refusal_no_emit(kwargs, _phoebus_active_display_allowed): + """Argument validation refuses before the bridge is contacted — emit nothing.""" + with ( + patch(f"{_PHOEBUS_MOD}._http_post_drive") as post, + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="validation_error"): + await _get_phoebus_drive()(**kwargs) + + post.assert_not_called() + notify.assert_not_called() + + +async def test_phoebus_drive_handle_required_refusal_no_emit(monkeypatch): + """Require-handle mode refuses implicit 'active' before any drive — emit nothing.""" + monkeypatch.setenv("PHOEBUS_REQUIRE_HANDLE", "1") + + with ( + patch(f"{_PHOEBUS_MOD}._http_post_drive") as post, + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="phoebus_handle_required"): + await _get_phoebus_drive()(widget="SetButton", verb="click") + + post.assert_not_called() + notify.assert_not_called() + + +async def test_phoebus_drive_bridge_rejected_no_emit(_phoebus_active_display_allowed): + """A non-200 from the bridge means no widget was driven — emit nothing.""" + with ( + _phoebus_bridge(400, {"error": "Unknown widget '0'", "status": 400}), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="phoebus_rejected"): + await _get_phoebus_drive()(widget="0", verb="click") + + notify.assert_not_called() + + +async def test_phoebus_drive_unreachable_no_emit(_phoebus_active_display_allowed): + """An unreachable bridge never drove anything — emit nothing.""" + import urllib.error + + with ( + patch(f"{_PHOEBUS_MOD}._http_post_drive", side_effect=urllib.error.URLError("refused")), + patch(f"{_PHOEBUS_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="phoebus_unreachable"): + await _get_phoebus_drive()(widget="SetButton", verb="click") + + notify.assert_not_called() + + +async def test_phoebus_drive_emit_runs_off_the_event_loop(_phoebus_active_display_allowed): + """The blocking notify must be thread-wrapped, never awaited inline.""" + import threading + + seen: list[int] = [] + + with ( + _phoebus_bridge(200, {"fired": True, "detail": "ok"}), + patch( + f"{_PHOEBUS_MOD}.notify_agent_activity", + side_effect=lambda *a, **k: seen.append(threading.get_ident()), + ), + ): + await _get_phoebus_drive()(widget="SetButton", verb="click") + + assert seen and seen[0] != threading.get_ident() + + +# ── python executor: execute / execute_file ───────────────────────────────── +# +# Launch discriminator this section pins: ``ExecutionResult.execution_time_seconds`` +# is None only for a result built by ``execute_code``'s setup handler, i.e. the +# script was never handed to a subprocess. Every result that came back from the +# subprocess path carries an elapsed time — including the timed-out and the +# script-error returns — and those scripts may already have written to the +# machine, so a failed run still has to report. +# +# Mode: the emit condition is the complement of the readonly gate +# (``execution_mode != "readonly"``), not equality with "readwrite". The mode +# argument is an unvalidated free string, so any other spelling reaches the +# subprocess with its writes intact and must stay visible. + +_EXECUTE_WRITE_CODE = "epics.caput('SR:CORR:SP', 1.0)\n" +_EXECUTE_DETAIL = "ran a script with control-system writes" +_EXECUTE_TOOLS = ["execute", "execute_file"] + + +def _execute_patterns(has_writes: bool) -> dict: + """Canned ``detect_control_system_operations()`` return value.""" + return { + "has_writes": has_writes, + "has_reads": False, + "detected_patterns": {"caput": ["caput('SR:CORR:SP', 1.0)"]} if has_writes else {}, + } + + +def _execute_result(*, success=True, launched=True, stderr="", error_message=None): + """Build one adapter outcome. + + ``launched=False`` reproduces the shape ``execute_code`` returns from its + setup handler (no execution folder, no subprocess, no elapsed time). + """ + from osprey.mcp_server.python_executor.executor import ExecutionResult + + return ExecutionResult( + success=success, + stdout="", + stderr=stderr, + execution_time_seconds=0.25 if launched else None, + error_message=error_message, + ) + + +def _execute_tool_call(tool_name: str, tmp_path): + """Bind one executor tool to a uniform ``(patch_module, call)`` pair. + + Both tools run the same write-bearing source; ``execute_file`` reads it from + a script inside the (patched) project root so the containment check passes. + """ + if tool_name == "execute": + from osprey.mcp_server.python_executor.tools.python_execute import execute + + fn = get_tool_fn(execute) + + async def call(**kwargs): + return await fn( + code=_EXECUTE_WRITE_CODE, + description="write script", + save_output=False, + **kwargs, + ) + + return _EXECUTE_MOD, call + + from osprey.mcp_server.python_executor.tools.python_execute_file import execute_file + + fn = get_tool_fn(execute_file) + script = tmp_path / "write_script.py" + script.write_text(_EXECUTE_WRITE_CODE, encoding="utf-8") + + async def call(**kwargs): + return await fn( + file_path=str(script), + description="write script", + save_output=False, + **kwargs, + ) + + return _EXECUTE_FILE_MOD, call + + +@contextlib.contextmanager +def _execute_env(mod, tmp_path, *, exec_result=None, has_writes=True, writes_enabled=True): + """Patch every boundary the executor tools cross; yield ``(notify, execute_code)``.""" + from osprey.services.python_executor.execution.control import ExecutionControlConfig + + exec_code = AsyncMock(return_value=exec_result if exec_result else _execute_result()) + with ( + patch( + "osprey.mcp_server.python_executor.executor._resolve_project_root", + return_value=tmp_path, + ), + patch( + "osprey.services.python_executor.analysis.pattern_detection" + ".detect_control_system_operations", + return_value=_execute_patterns(has_writes), + ), + patch( + "osprey.services.python_executor.execution.control.get_execution_control_config", + return_value=ExecutionControlConfig(control_system_writes_enabled=writes_enabled), + ), + patch("osprey.mcp_server.python_executor.executor.execute_code", exec_code), + patch(f"{mod}.notify_agent_activity") as notify, + ): + yield notify, exec_code + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_write_run_emits(tool_name, tmp_path, monkeypatch): + """A readwrite run of a write-bearing script reports once, kind channel.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + + with _execute_env(mod, tmp_path) as (notify, _): + result = await call(execution_mode="readwrite") + + assert extract_response_dict(result)["has_errors"] is False + notify.assert_called_once_with(tool_name, "channel", detail=_EXECUTE_DETAIL) + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_script_error_after_launch_still_emits(tool_name, tmp_path, monkeypatch): + """The script ran and then raised — writes before the error already landed.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + failed = _execute_result( + success=False, + launched=True, + stderr="Traceback...\nNameError: name 'undefined' is not defined", + error_message="NameError: name 'undefined' is not defined", + ) + + with _execute_env(mod, tmp_path, exec_result=failed) as (notify, _): + with assert_raises_error(error_type="execution_error"): + await call(execution_mode="readwrite") + + notify.assert_called_once_with(tool_name, "channel", detail=_EXECUTE_DETAIL) + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_launch_failure_no_emit(tool_name, tmp_path, monkeypatch): + """Sandbox setup failed before any subprocess started — nothing ran, nothing wrote.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + never_launched = _execute_result( + success=False, + launched=False, + stderr="Traceback...\nOSError: no execution folder", + error_message="Execution setup failed: no execution folder", + ) + + with _execute_env(mod, tmp_path, exec_result=never_launched) as (notify, _): + with assert_raises_error(error_type="execution_error"): + await call(execution_mode="readwrite") + + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_readonly_gate_refusal_no_emit(tool_name, tmp_path, monkeypatch): + """Write patterns in readonly mode are refused before launch — emit nothing.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + + with _execute_env(mod, tmp_path) as (notify, exec_code): + with assert_raises_error(error_type="safety_error"): + await call(execution_mode="readonly") + + exec_code.assert_not_called() + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_safety_check_refusal_no_emit(tool_name, tmp_path, monkeypatch): + """The pre-execution safety check refuses before launch — emit nothing.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + + with _execute_env(mod, tmp_path) as (notify, exec_code): + with patch( + "osprey.services.python_executor.analysis.safety_checks.quick_safety_check", + return_value=(False, ["subprocess use is blocked"]), + ): + with assert_raises_error(error_type="safety_error"): + await call(execution_mode="readwrite") + + exec_code.assert_not_called() + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_readwrite_without_write_patterns_no_emit(tool_name, tmp_path, monkeypatch): + """readwrite alone is not a write: with no detected write patterns, stay silent.""" + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + + with _execute_env(mod, tmp_path, has_writes=False) as (notify, exec_code): + await call(execution_mode="readwrite") + + exec_code.assert_called_once() + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_uncanonical_write_mode_still_emits(tool_name, tmp_path, monkeypatch): + """A non-canonical mode spelling clears the readonly gate, so its writes must report. + + ``execution_mode`` is an unvalidated free string and is never normalized, so + "READWRITE" runs the script exactly as "readwrite" does. Testing equality + with "readwrite" instead of the gate's complement would silence this run. + """ + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + + with _execute_env(mod, tmp_path) as (notify, exec_code): + await call(execution_mode="READWRITE") + + exec_code.assert_called_once() + notify.assert_called_once_with(tool_name, "channel", detail=_EXECUTE_DETAIL) + + +async def test_execute_deployment_writes_disabled_no_emit(tmp_path, monkeypatch): + """The deployment kill switch refuses before launch — emit nothing. + + Only the ``execute`` tool carries this gate; ``execute_file`` has no + equivalent, so this case is not parametrized. + """ + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call("execute", tmp_path) + + with _execute_env(mod, tmp_path, writes_enabled=False) as (notify, exec_code): + with assert_raises_error(error_type="safety_error"): + await call(execution_mode="readwrite") + + exec_code.assert_not_called() + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name", _EXECUTE_TOOLS) +async def test_execute_emit_runs_off_the_event_loop(tool_name, tmp_path, monkeypatch): + """The blocking notify must be thread-wrapped, never awaited inline.""" + import threading + + monkeypatch.chdir(tmp_path) + mod, call = _execute_tool_call(tool_name, tmp_path) + seen: list[int] = [] + + with _execute_env(mod, tmp_path) as (notify, _): + notify.side_effect = lambda *a, **k: seen.append(threading.get_ident()) + await call(execution_mode="readwrite") + + assert seen and seen[0] != threading.get_ident() + + +# ── lattice dashboard mutators ────────────────────────────────────────────── +# +# The six mutators share one refusal shape: every failure arrives as an +# exception out of ``_dashboard_request`` and every handler funnels into +# ``make_error``, which raises before the emit can run. Both failure classes +# the module handles by name — an unreachable dashboard and a non-2xx response +# — are therefore exercised against all six rather than per-mutator variants. +# The read-only tools share the same body minus the emit, so they are pinned +# as a group too. + +_LATTICE_MUTATORS = [ + ("lattice_init", {"lattice_path": "machine_data/als.m"}), + ("lattice_set_param", {"family": "QF", "value": 1.25}), + ("lattice_refresh", {}), + ("lattice_set_baseline", {}), + ("lattice_update_settings", {"settings": {"da": {"n_angles": 25}}}), + ("lattice_clear_baseline", {}), +] + +_LATTICE_READERS = [ + ("lattice_state", {}), + ("lattice_get_figure", {"name": "optics"}), + ("lattice_get_data", {"name": "optics"}), + ("lattice_get_settings", {}), +] + + +def _get_lattice_tool(name: str): + from osprey.mcp_server.workspace.tools import lattice_tools + + return get_tool_fn(getattr(lattice_tools, name)) + + +def _lattice_request(*, return_value=None, side_effect=None): + """Patch the dashboard HTTP boundary with a canned answer or failure.""" + body = {"summary": {"energy": 2.0}, "families": {"QF": {}}} + return patch( + f"{_LATTICE_MOD}._dashboard_request", + new=AsyncMock( + return_value=body if return_value is None else return_value, side_effect=side_effect + ), + ) + + +def _lattice_unreachable(): + import httpx + + return httpx.ConnectError("connection refused") + + +def _lattice_http_error(status: int = 400, text: str = "unknown family"): + import httpx + + request = httpx.Request("POST", "http://dash/api") + return httpx.HTTPStatusError( + "bad status", request=request, response=httpx.Response(status, text=text, request=request) + ) + + +@pytest.mark.parametrize( + "tool_name,kwargs,detail", + [ + ("lattice_init", {"lattice_path": "machine_data/als.m"}, "machine_data/als.m"), + ("lattice_set_param", {"family": "QF", "value": 1.25}, "QF = 1.25"), + ("lattice_refresh", {}, "recomputing fast figures"), + ("lattice_refresh", {"figure": "da"}, "recomputing da"), + ("lattice_set_baseline", {}, "baseline set"), + ( + "lattice_update_settings", + {"settings": {"lma": {"n_steps": 8}, "da": {"n_angles": 25}}}, + "settings: da, lma", + ), + ("lattice_clear_baseline", {}, "baseline cleared"), + ], +) +async def test_lattice_mutator_emits(tool_name, kwargs, detail): + """Each acknowledged mutation reports once against the 'lattice' panel.""" + with ( + _lattice_request(), + patch(f"{_LATTICE_MOD}.notify_agent_activity") as notify, + ): + result = await _get_lattice_tool(tool_name)(**kwargs) + + assert extract_response_dict(result) + notify.assert_called_once_with(tool_name, "panel", panel="lattice", detail=detail) + + +@pytest.mark.parametrize("tool_name,kwargs", _LATTICE_MUTATORS) +async def test_lattice_mutator_unreachable_dashboard_no_emit(tool_name, kwargs): + """A dashboard that never answered changed nothing — emit nothing.""" + with ( + _lattice_request(side_effect=_lattice_unreachable()), + patch(f"{_LATTICE_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="service_unavailable"): + await _get_lattice_tool(tool_name)(**kwargs) + + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name,kwargs", _LATTICE_MUTATORS) +async def test_lattice_mutator_rejected_request_no_emit(tool_name, kwargs): + """A non-2xx answer means the dashboard refused the change — emit nothing.""" + with ( + _lattice_request(side_effect=_lattice_http_error()), + patch(f"{_LATTICE_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="lattice_error"): + await _get_lattice_tool(tool_name)(**kwargs) + + notify.assert_not_called() + + +@pytest.mark.parametrize("tool_name,kwargs", _LATTICE_READERS) +async def test_lattice_read_only_tool_never_emits(tool_name, kwargs): + """Reading state or a figure mutates nothing, so it stays out of the feed.""" + with ( + _lattice_request(return_value={"status": "ok"}), + patch(f"{_LATTICE_MOD}.notify_agent_activity") as notify, + ): + await _get_lattice_tool(tool_name)(**kwargs) + + notify.assert_not_called() + + +async def test_lattice_emit_runs_off_the_event_loop(): + """The blocking notify must be thread-wrapped, never awaited inline.""" + import threading + + seen: list[int] = [] + + with ( + _lattice_request(), + patch( + f"{_LATTICE_MOD}.notify_agent_activity", + side_effect=lambda *a, **k: seen.append(threading.get_ident()), + ), + ): + await _get_lattice_tool("lattice_set_param")(family="QF", value=1.25) + + assert seen and seen[0] != threading.get_ident() + + +# ── setup_patch / manage_window ───────────────────────────────────────────── +# +# setup_patch edits the two files that hold the agent's credentials, so its +# detail names the file and the key path and NOTHING else: the ring is +# persistent and served back over HTTP, and a leaked `.mcp.json` value is an +# API key. The leak tests use sentinel values distinctive enough that no +# coincidental substring can let a leak through, and check every notify +# argument rather than just the detail. +# +# Both tools refuse before touching anything, so each refusal shape is pinned +# as a no-emit: an out-of-whitelist file, a malformed key path, a missing +# file and an unparseable file for setup_patch; an unknown action, both +# incomplete-parameter shapes and a failing backend for manage_window. + +_OLD_SENTINEL = "zzqqOLDVALUEsentinel42" +_NEW_SENTINEL = "wwvvNEWVALUEsentinel99" + +_MANAGE_WINDOW_ACTIONS = [ + ("bring_to_front", {}), + ("move", {"x": 10, "y": 20}), + ("resize", {"width": 800, "height": 600}), +] + + +@pytest.fixture +def setup_project(tmp_path): + """Project root holding both patchable files, with config resolution pinned.""" + (tmp_path / "config.yml").write_text( + yaml.dump({"control_system": {"type": "mock", "writes_enabled": False}}) + ) + (tmp_path / ".mcp.json").write_text( + '{\n "mcpServers": {\n "demo": {\n "env": {\n' + f' "API_KEY": "{_OLD_SENTINEL}"\n' + " }\n }\n }\n}\n" + ) + with patch(f"{_SETUP_MOD}.resolve_config_path", return_value=tmp_path / "config.yml"): + yield tmp_path + + +def _get_setup_patch(): + from osprey.mcp_server.workspace.tools.setup import setup_patch + + return get_tool_fn(setup_patch) + + +def _get_manage_window(): + from osprey.mcp_server.workspace.tools.screen_capture import manage_window + + return get_tool_fn(manage_window) + + +def _screen_backend(*, side_effect=None): + """Patch the window backend with an async stub, optionally a failing one.""" + backend = AsyncMock() + if side_effect is not None: + backend.bring_to_front.side_effect = side_effect + backend.move_window.side_effect = side_effect + backend.resize_window.side_effect = side_effect + return patch(f"{_SCREEN_MOD}.get_backend", return_value=backend) + + +def _backend_unavailable(): + from osprey.mcp_server.workspace.tools.screen_capture_backends import ( + BackendUnavailableError, + ) + + return BackendUnavailableError("no window manager", ["Install wmctrl."]) + + +async def test_setup_patch_emits_config_activity(setup_project): + """A landed patch reports the file and key path under the 'config' kind.""" + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + result = await _get_setup_patch()( + file="config.yml", key_path="agent_data.base_dir", value="./_agent_data" + ) + + assert extract_response_dict(result)["key_path"] == "agent_data.base_dir" + notify.assert_called_once_with( + "setup_patch", "config", detail="config.yml: agent_data.base_dir" + ) + + +async def test_setup_patch_emits_for_json_target(setup_project): + """The `.mcp.json` branch reports too — it is the same mutation.""" + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + await _get_setup_patch()( + file=".mcp.json", key_path="mcpServers.demo.env.API_KEY", value=_NEW_SENTINEL + ) + + notify.assert_called_once_with( + "setup_patch", "config", detail=".mcp.json: mcpServers.demo.env.API_KEY" + ) + + +async def test_setup_patch_detail_never_carries_the_patched_values(setup_project): + """Hard security bound: neither the old nor the new value may reach the feed.""" + import json + + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + await _get_setup_patch()( + file=".mcp.json", key_path="mcpServers.demo.env.API_KEY", value=_NEW_SENTINEL + ) + + # The patch really did swap the sentinels, so their absence below is a + # property of the emit and not of an inert test. + on_disk = json.loads((setup_project / ".mcp.json").read_text()) + assert on_disk["mcpServers"]["demo"]["env"]["API_KEY"] == _NEW_SENTINEL + + call = notify.call_args + reported = [str(arg) for arg in call.args] + [str(v) for v in call.kwargs.values()] + for sentinel in (_OLD_SENTINEL, _NEW_SENTINEL): + for piece in reported: + assert sentinel not in piece, f"value leaked into the activity feed: {piece!r}" + + +async def test_setup_patch_marks_control_system_keys_as_safety_config(setup_project): + """A safety-relevant key path is distinguishable at a glance in the feed.""" + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + await _get_setup_patch()( + file="config.yml", key_path="control_system.writes_enabled", value="true" + ) + + notify.assert_called_once_with( + "setup_patch", + "config", + detail="safety config — config.yml: control_system.writes_enabled", + ) + + +async def test_setup_patch_safety_prefix_is_exact_case(setup_project): + """The prefix match is case-sensitive, like the hot/cold key lookups. + + Nothing normalises `key_path`, so both spellings are pinned: the canonical + lowercase one carries the marker and a differently-cased path — which names + a different key, and gets no hot/cold note either — does not. + """ + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + await _get_setup_patch()( + file="config.yml", key_path="Control_System.writes_enabled", value="true" + ) + + notify.assert_called_once_with( + "setup_patch", "config", detail="config.yml: Control_System.writes_enabled" + ) + + +async def test_setup_patch_unpatchable_file_no_emit(setup_project): + """A file outside the whitelist was never opened — emit nothing.""" + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="validation_error"): + await _get_setup_patch()(file="settings.json", key_path="permissions.deny", value="[]") + + notify.assert_not_called() + + +@pytest.mark.parametrize("key_path", ["", "../../etc/passwd", "/abs/path", "has space"]) +async def test_setup_patch_invalid_key_path_no_emit(setup_project, key_path): + """A rejected key path changed nothing — emit nothing.""" + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="validation_error"): + await _get_setup_patch()(file="config.yml", key_path=key_path, value="1") + + notify.assert_not_called() + + +async def test_setup_patch_missing_file_no_emit(setup_project): + """Nothing to patch means nothing to report.""" + (setup_project / ".mcp.json").unlink() + + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="not_found"): + await _get_setup_patch()(file=".mcp.json", key_path="mcpServers.x", value="1") + + notify.assert_not_called() + + +async def test_setup_patch_unparseable_file_no_emit(setup_project): + """A file that could not be read back was never rewritten — emit nothing.""" + (setup_project / ".mcp.json").write_text("{ this is not json") + + with patch(f"{_SETUP_MOD}.notify_agent_activity") as notify: + with assert_raises_error(error_type="internal_error"): + await _get_setup_patch()(file=".mcp.json", key_path="mcpServers.x", value="1") + + notify.assert_not_called() + + +async def test_setup_patch_emit_runs_off_the_event_loop(setup_project): + """The blocking notify must be thread-wrapped, never awaited inline.""" + import threading + + seen: list[int] = [] + + with patch( + f"{_SETUP_MOD}.notify_agent_activity", + side_effect=lambda *a, **k: seen.append(threading.get_ident()), + ): + await _get_setup_patch()(file="config.yml", key_path="control_system.type", value="mock") + + assert seen and seen[0] != threading.get_ident() + + +@pytest.mark.parametrize("action,kwargs", _MANAGE_WINDOW_ACTIONS) +async def test_manage_window_emits_ui_activity(action, kwargs): + """Each completed window action reports once under the 'ui' kind.""" + with ( + _screen_backend(), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + result = await _get_manage_window()(app="Phoebus", action=action, **kwargs) + + assert extract_response_dict(result)["status"] == "success" + notify.assert_called_once_with("manage_window", "ui", detail=f"{action} Phoebus") + + +async def test_manage_window_detail_preserves_app_name_case(): + """Nothing normalises `app`, so the feed shows the name the operator sees.""" + with ( + _screen_backend(), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + await _get_manage_window()(app="google Chrome", action="bring_to_front") + + notify.assert_called_once_with("manage_window", "ui", detail="bring_to_front google Chrome") + + +@pytest.mark.parametrize("action", ["fullscreen", "Bring_To_Front", "MOVE"]) +async def test_manage_window_unknown_action_no_emit(action): + """Action matching is exact-case; every unmatched spelling is refused, not reported.""" + with ( + _screen_backend(), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="validation_error"): + await _get_manage_window()(app="Phoebus", action=action, x=1, y=2) + + notify.assert_not_called() + + +@pytest.mark.parametrize( + "action,kwargs", + [ + ("move", {}), + ("move", {"x": 10}), + ("resize", {}), + ("resize", {"width": 800}), + ], +) +async def test_manage_window_incomplete_parameters_no_emit(action, kwargs): + """A refusal before the backend call moved no window — emit nothing.""" + with ( + _screen_backend(), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="validation_error"): + await _get_manage_window()(app="Phoebus", action=action, **kwargs) + + notify.assert_not_called() + + +@pytest.mark.parametrize("action,kwargs", _MANAGE_WINDOW_ACTIONS) +async def test_manage_window_backend_failure_no_emit(action, kwargs): + """A backend that could not act leaves the window alone — emit nothing.""" + with ( + _screen_backend(side_effect=_backend_unavailable()), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="platform_error"): + await _get_manage_window()(app="Phoebus", action=action, **kwargs) + + notify.assert_not_called() + + +async def test_manage_window_unknown_app_no_emit(): + """A window the backend cannot find was never touched — emit nothing.""" + with ( + _screen_backend(side_effect=ValueError("no window for app 'Nope'")), + patch(f"{_SCREEN_MOD}.notify_agent_activity") as notify, + ): + with assert_raises_error(error_type="validation_error"): + await _get_manage_window()(app="Nope", action="bring_to_front") + + notify.assert_not_called() + + +async def test_manage_window_emit_runs_off_the_event_loop(): + """The blocking notify must be thread-wrapped, never awaited inline.""" + import threading + + seen: list[int] = [] + + with ( + _screen_backend(), + patch( + f"{_SCREEN_MOD}.notify_agent_activity", + side_effect=lambda *a, **k: seen.append(threading.get_ident()), + ), + ): + await _get_manage_window()(app="Phoebus", action="bring_to_front") + + assert seen and seen[0] != threading.get_ident() + + # ── terminal down: real helper against a dead port ────────────────────────── diff --git a/tests/mcp_server/test_http.py b/tests/mcp_server/test_http.py index f671867c1..6a5edf5f0 100644 --- a/tests/mcp_server/test_http.py +++ b/tests/mcp_server/test_http.py @@ -17,6 +17,12 @@ from osprey.mcp_server import http +# These tests drive the posters themselves, so they opt out of the conftest +# stub that blocks notify_* POSTs from leaving the process. Nothing here can +# reach a live web terminal: every socket-level call targets a dead port or a +# patched ``urlopen``. +pytestmark = pytest.mark.real_http_posters + @pytest.fixture def patch_config(): diff --git a/tests/mcp_server/test_notify_agent_activity.py b/tests/mcp_server/test_notify_agent_activity.py index a27b24daf..29c0a561d 100644 --- a/tests/mcp_server/test_notify_agent_activity.py +++ b/tests/mcp_server/test_notify_agent_activity.py @@ -19,6 +19,11 @@ _MODULE = "osprey.mcp_server.http" +# The helper's own contract test: it must really POST, so it opts out of the +# conftest stub that blocks notify_* POSTs. Every case here redirects +# ``web_terminal_url`` at a capture server or a dead port of its own. +pytestmark = pytest.mark.real_http_posters + def _free_port() -> int: """Reserve a localhost port and release it (nothing will be listening).""" From 21b49fd37eaf51371ee13de1022b25a8eb0b49df Mon Sep 17 00:00:00 2001 From: ThorstenHellert Date: Wed, 12 Aug 2026 19:36:26 +0200 Subject: [PATCH 3/5] feat(web): surface agent actions across the terminal UI A user could not tell what the agent was doing to their workspace. Agent-driven panel switches looked identical to their own clicks, hides and shows left no trace, and the activity strip spoke raw tool names for a handful of tools and stayed silent for the rest. Agent focus and arrange now glow the affected tile body; users with reduced motion get a held static ring instead of the pulse. The strip words each action with a verb and the panel's display label, hides and shows included, and a click opens a history popover fed from the server ring so the record survives reload. Per-panel badges are acknowledged by server timestamp in localStorage, so seen activity stays cleared across reloads while unseen activity comes back. The rail scrolls an off-screen badge into view and tooltips the touch time. The session page connects its strip to the event stream it was booted for, and chat activity lines phrase tool names in plain language. Formatting and the history popover move to their own modules (activity-format.js, activity-history.js); the strip keeps the live line and delegates. --- CHANGELOG.md | 9 + eslint.config.js | 13 + .../design_system/static/css/highlight.css | 20 +- .../static/css/activity-strip.css | 78 ++- .../web_terminal/static/css/terminal.css | 20 + .../web_terminal/static/js/activity-format.js | 104 ++++ .../static/js/activity-history.js | 275 ++++++++++ .../web_terminal/static/js/activity-strip.js | 112 ++-- .../web_terminal/static/js/chat-render.js | 194 ++++++- .../web_terminal/static/js/dock-iframe.js | 84 ++- .../web_terminal/static/js/panel-manager.js | 135 ++++- .../web_terminal/static/js/panel-placement.js | 2 +- .../web_terminal/static/js/panel-rail.js | 68 ++- .../web_terminal/static/js/session.js | 43 +- .../design_system/highlight.test.mjs | 71 +++ .../web_terminal/activity-strip.test.mjs | 501 +++++++++++++++++- .../web_terminal/chat-render.test.mjs | 108 +++- .../web_terminal/dock-glow.test.mjs | 355 +++++++++++++ .../web_terminal/panel-manager.test.mjs | 310 +++++++++++ .../web_terminal/panel-rail.test.mjs | 151 +++++- .../web_terminal/session-strip.test.mjs | 201 +++++++ 21 files changed, 2782 insertions(+), 72 deletions(-) create mode 100644 src/osprey/interfaces/web_terminal/static/js/activity-format.js create mode 100644 src/osprey/interfaces/web_terminal/static/js/activity-history.js create mode 100644 tests/interfaces/web_terminal/dock-glow.test.mjs create mode 100644 tests/interfaces/web_terminal/session-strip.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a73279e0..ae27f5858 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,6 +172,15 @@ Compatibility is documented in release notes, not encoded in the version string. `config: {control_system.type: ...}`, so a connector can be chosen from the command line with `--set connector=epics`. Giving both spellings on one command line is an error rather than a silent last-one-wins. +- You can see what the agent did to your workspace. A tile the agent focuses + or rearranges glows briefly, and its rail tab flashes with it, so a layout + that changes under you is never unattributed — your own clicks stay quiet. + An activity strip names each action in plain words ("agent opened + WORKSPACE"), and its history popover holds the recent ones for when you + looked away. Panels that changed while you were elsewhere keep a badge + across a reload until you visit them. Every agent tool that changes + something — queue and plan authoring, logbook entries, Phoebus drives, + python execution, lattice and window management — reports itself there. ### Changed diff --git a/eslint.config.js b/eslint.config.js index c28b77550..bc7f82d17 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -54,6 +54,19 @@ export default [ }, }, + // (4c) max-lines ratchet on panel-manager.js alone. It sits right at the 450 + // cap above while three in-flight tasks (wire-glow-call-sites, + // strip-verbs-and-labels, badge-ack-restore) all land agent-visibility code + // in it. Splitting the module mid-flight would be more destabilizing than the + // raised ceiling; the split is earmarked for the polish pass, and this block + // goes away with it. Scoped to the one file so nothing else drifts upward. + { + files: ['src/osprey/interfaces/web_terminal/static/js/panel-manager.js'], + rules: { + 'max-lines': ['error', { max: 550, skipComments: true, skipBlankLines: true }], + }, + }, + // (5) Root config files: node globals. { files: ['vitest.config.js', 'eslint.config.js'], diff --git a/src/osprey/interfaces/design_system/static/css/highlight.css b/src/osprey/interfaces/design_system/static/css/highlight.css index fc877b92d..4c44b8ec9 100644 --- a/src/osprey/interfaces/design_system/static/css/highlight.css +++ b/src/osprey/interfaces/design_system/static/css/highlight.css @@ -51,8 +51,26 @@ pointer-events: none; } +/* Reduced motion: still a flash, just a still one. + + `animation: none` cannot go here — it never fires `animationend`, so + flashElement's cleanup listener never runs and `.agent-flash` stays on the + element forever. Instead the same keyframes name is re-declared as a + constant ring and stepped through: nothing moves, the element holds a + static attribution ring, and the animation still ends and self-cleans. + The name must stay `agent-flash-glow` — flashElement ignores animationend + for any other name, which would strand the class just as `none` did. */ @media (prefers-reduced-motion: reduce) { + @keyframes agent-flash-glow { + 0%, + 100% { + box-shadow: 0 0 0 3px var(--accent-tint-25); + } + } + .agent-flash { - animation: none; + /* Held longer than the 900ms decay: a static ring has no motion to catch + the eye, so it needs the extra dwell to register. */ + animation: agent-flash-glow 1200ms steps(1, end); /* hygiene-allow-scale: reduced-motion hold */ } } diff --git a/src/osprey/interfaces/web_terminal/static/css/activity-strip.css b/src/osprey/interfaces/web_terminal/static/css/activity-strip.css index 0a757c2bb..878173541 100644 --- a/src/osprey/interfaces/web_terminal/static/css/activity-strip.css +++ b/src/osprey/interfaces/web_terminal/static/css/activity-strip.css @@ -41,7 +41,7 @@ } .activity-strip-subject { - color: var(--accent); + color: var(--color-accent); min-width: 0; overflow: hidden; text-overflow: ellipsis; @@ -84,3 +84,79 @@ opacity: 0; pointer-events: none; } + +/* ---- History popover ---- + * + * The strip is its own trigger: clicking it expands the recent-activity list. + * Cursor and focus ring are the only affordances — no added box, so the live + * line's layout is untouched. */ +.activity-strip-trigger { + cursor: pointer; +} + +.activity-strip-trigger:focus-visible { + outline: 1px solid var(--color-accent); + outline-offset: 2px; + border-radius: var(--radius-sm); +} + +/* Body-level and fixed — the hub's strip lives in the footer status bar, + * which is one row tall and overflow:hidden, so an in-place popover would be + * clipped away. Coordinates come from JS (placeHistory()); everything here is + * appearance only. Not scoped under the status bar: it is a child of . */ +.activity-history-popover { + position: fixed; + z-index: var(--z-dropdown); + min-width: 240px; + max-width: min(90vw, 420px); + max-height: min(50vh, 320px); + overflow-y: auto; + padding: var(--space-1); + background: var(--bg-elevated); + border: 1px solid var(--border-default); + border-radius: var(--radius-md); + box-shadow: var(--shadow-dropdown); + font-family: var(--font-mono); + font-size: var(--text-sm); +} + +.activity-history-popover:focus { + outline: none; +} + +.activity-history-row { + display: flex; + align-items: baseline; + gap: var(--space-1); + padding: 3px var(--space-2); + white-space: nowrap; +} + +.activity-history-verb { + color: var(--text-secondary); + flex: 0 0 auto; +} + +/* The agent-supplied half: it gets the remaining width and truncates, so a + long channel list can never widen the popover past its max. */ +.activity-history-subject { + color: var(--color-accent); + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; +} + +.activity-history-time { + color: var(--text-muted); + font-size: var(--text-xs); + flex: 0 0 auto; + margin-left: auto; + padding-left: var(--space-2); +} + +.activity-history-message { + padding: var(--space-2); + color: var(--text-muted); + text-align: center; +} diff --git a/src/osprey/interfaces/web_terminal/static/css/terminal.css b/src/osprey/interfaces/web_terminal/static/css/terminal.css index 7cb063ca3..0b002dbdf 100644 --- a/src/osprey/interfaces/web_terminal/static/css/terminal.css +++ b/src/osprey/interfaces/web_terminal/static/css/terminal.css @@ -1170,6 +1170,26 @@ html[data-rail-position="top"] .rail-hint { pointer-events: none; } +/* Agent attribution glow for a whole tile body. dock-iframe.js sizes one of + these to the affected panel's group rectangle and fires the design system's + `.agent-flash` on it (highlight.css supplies every colour, accent tokens + only, so it reads in all four themes). It is a separate overlay element + rather than the panel iframe because the iframe is another document the + flash cannot reach into, and flashing the iframe element itself would clip + the embedded app to the flash's border-radius. + + At rest the element is fully transparent — the flash animates background and + box-shadow back to transparent and removes its own class — so a spent glow + costs nothing parked in the overlay. `z-index` lifts it above the overlay's + iframes, which are positioned but unlayered and would otherwise paint over + any glow created before them; `pointer-events` re-states the overlay's own + inertness so the layer stays click-through even if it is ever re-parented. */ +.dock-iframe-overlay .tile-glow { + position: absolute; + z-index: 1; + pointer-events: none; +} + /* ---- Terminal panel ---- */ .terminal-panel { diff --git a/src/osprey/interfaces/web_terminal/static/js/activity-format.js b/src/osprey/interfaces/web_terminal/static/js/activity-format.js new file mode 100644 index 000000000..3c65ca709 --- /dev/null +++ b/src/osprey/interfaces/web_terminal/static/js/activity-format.js @@ -0,0 +1,104 @@ +// @ts-check +/* OSPREY Web Terminal — Agent Activity Vocabulary + * + * How an agent_activity frame is WORDED. Two surfaces render one — the live + * line in activity-strip.js and the history rows in activity-history.js — and + * both word it through here, so a hide reads the same wherever it appears. + * + * Pure by construction: no DOM, no fetch, no module state. Callers own the + * rendering, and every agent-supplied string they get back goes in as a text + * node (createElement + textContent, never innerHTML). + */ + +/** @typedef {import('./panel-manager.js').AgentActivityEvent} AgentActivityFrame */ + +/** + * The tool name the panel routes mirror an agent arrange under. Its subject is + * the workspace itself rather than a single panel, so it is worded apart from + * the PANEL_VERBS table — and the strip keys its burst coalescing on it. + */ +export const ARRANGE_TOOL = 'arrange_workspace'; + +/** + * Verb per panel action, keyed on the synthetic tool names the panel routes + * record for agent-origin gestures (see routes/panels.py). + * @type {Record} + */ +const PANEL_VERBS = { + hide_panel: 'agent closed', + show_panel: 'agent opened', + switch_panel: 'agent focused', + register_panel: 'agent added', +}; + +/** + * Human-readable two-part label for a frame. The subject carries the + * agent-supplied string and is rendered as a text node by the caller. + * @param {AgentActivityFrame} frame + * @param {{ labelOf?: (id: string) => string, count?: number }} [opts] + * `labelOf` resolves a panel id to its catalog label; without it (or for an + * id the catalog does not know) the raw id is shown. `count` is how many + * coalesced arrange frames this one line stands for. + * @returns {{ verb: string, subject: string }} + */ +export function formatActivity(frame, opts = {}) { + const t = frame.target; + /** Catalog label for a panel id, falling back to the id, then the tool. + * @param {string | undefined} id @returns {string} */ + const label = (id) => (id ? opts.labelOf?.(id) || id : frame.tool); + switch (t.kind) { + case 'channel': + // Neutral on purpose: a channel detail is the display/channel the tool + // was ASKED to drive, never a read-back of what the machine confirmed. + return { verb: 'agent wrote', subject: t.detail || frame.tool }; + case 'run': + return t.detail + ? { verb: 'agent launched run', subject: t.detail } + : { verb: 'agent launched a run', subject: '' }; + case 'artifact': + return { verb: 'agent focused', subject: t.detail || 'an artifact' }; + case 'config': + return { verb: 'agent changed config', subject: t.detail || frame.tool }; + case 'ui': + return { verb: 'agent moved window', subject: t.detail || frame.tool }; + case 'panel': { + if (frame.tool === ARRANGE_TOOL) { + // A lone arrange row (in history, or the first frame of a burst) has + // no count to report; only a coalesced run names how many tiles moved. + const n = opts.count ?? 1; + return { verb: 'agent arranged', subject: n > 1 ? `workspace (${n} tiles)` : 'workspace' }; + } + const verb = PANEL_VERBS[frame.tool]; + if (verb) return { verb, subject: label(t.panel) }; + // Generic fallback: a panel-kind frame from some other tool. + return { verb: 'agent touched', subject: t.panel || frame.tool }; + } + default: + // Unknown future kind from a newer server — still show something. + return { verb: 'agent activity', subject: frame.tool }; + } +} + +/** + * Coarse "how long ago" label for a frame's server timestamp. + * + * Deliberately coarse: history rows answer "what has the agent been doing", + * not "at exactly which instant". A missing ts (older server, or a frame + * synthesised client-side) yields an empty string, which the caller skips. + * Browser and server clocks can disagree by a little, so any non-positive + * age reads as "just now" rather than a negative number. + * @param {number | undefined} ts epoch seconds, as the server stamps them + * @param {number} nowMs current wall clock in ms (Date.now()) + * @returns {string} + */ +export function formatRelativeTime(ts, nowMs) { + if (typeof ts !== 'number' || !Number.isFinite(ts)) return ''; + const secs = Math.round(nowMs / 1000 - ts); + if (secs < 1) return 'just now'; + if (secs < 60) return `${secs}s ago`; + const mins = Math.floor(secs / 60); + if (mins < 60) return `${mins}m ago`; + const hours = Math.floor(mins / 60); + if (hours < 24) return `${hours}h ago`; + return `${Math.floor(hours / 24)}d ago`; +} diff --git a/src/osprey/interfaces/web_terminal/static/js/activity-history.js b/src/osprey/interfaces/web_terminal/static/js/activity-history.js new file mode 100644 index 000000000..12103793c --- /dev/null +++ b/src/osprey/interfaces/web_terminal/static/js/activity-history.js @@ -0,0 +1,275 @@ +// @ts-check +/* OSPREY Web Terminal — Agent Activity History Popover + * + * The activity strip's live line shows only the latest action, so clicking the + * strip opens this popover listing the recent ones. The history itself lives on + * the server (a bounded ring, GET /api/agent-activity/recent) — this module + * keeps no client-side ring, it just fetches on open and renders the rows. + * Frames arriving while it is open are prepended live; because the server + * appends to its ring BEFORE broadcasting, a refetch always re-includes them, + * so a live row is never lost when a slower fetch resolves over it. + * + * The popover is a child of , position:fixed, because the hub's strip + * sits inside the footer status bar and that bar is overflow:hidden and one + * row tall — an in-place popover would be clipped away (same reason, same + * recipe as the tile contribution menu in tile-header-items.js). + * + * The strip mount is its own trigger. It is an aria-live region provided by + * the template, so this module cannot re-role it into a button or nest a + * persistent button inside it (the live slot is wiped on every frame). + * Instead the mount takes aria-expanded plus keyboard activation. + * + * All agent-supplied strings are rendered as text nodes only — createElement + + * textContent, never innerHTML. + */ + +import { withPrefix } from './api.js'; +import { formatActivity, formatRelativeTime } from './activity-format.js'; + +/** @typedef {import('./panel-manager.js').AgentActivityEvent} AgentActivityFrame */ + +/** + * Rows requested from the server ring, and the cap on rows kept in the open + * popover's DOM. Matches ACTIVITY_RING_MAX server-side: asking for more just + * gets clamped, and a popover left open through a long run must not grow + * without bound. + */ +export const HISTORY_LIMIT = 50; + +/** + * Default history reader: the server's ring, newest first. Throws on a + * transport or HTTP failure so the popover can show its error state rather + * than an empty list that would read as "the agent did nothing". + * @param {number} limit + * @returns {Promise} + */ +async function fetchRecentActivity(limit) { + const resp = await fetch(withPrefix(`/api/agent-activity/recent?limit=${limit}`), { + cache: 'no-store', + }); + if (!resp.ok) throw new Error(`recent agent activity: HTTP ${resp.status}`); + const body = await resp.json(); + // Contract: {"events": [...]} — an object, never a bare array. + return Array.isArray(body?.events) ? body.events : []; +} + +/** + * Build the history popover for a strip, anchored to and triggered by `mount`. + * Wiring the trigger is part of construction: the mount is the affordance, so + * a history that existed without it could never be opened by an operator. + * + * Dependencies are injected so tests drive it without a network. + * @param {{ + * mount: HTMLElement, + * fetchRecent?: (limit: number) => Promise, + * labelOf?: (id: string) => string, + * }} deps + * @returns {{ + * open: () => Promise, + * close: () => void, + * isOpen: () => boolean, + * prepend: (frame: AgentActivityFrame) => void, + * }} + */ +export function createActivityHistory({ + mount, + fetchRecent = fetchRecentActivity, + labelOf: panelLabel, +}) { + /** @type {HTMLElement | null} */ + let popoverEl = null; + let historyOpen = false; + /** Bumped on every open and close, so a fetch from a stale open is dropped. */ + let historyGeneration = 0; + + function ensurePopover() { + if (popoverEl) return popoverEl; + const el = document.createElement('div'); + el.className = 'activity-history-popover'; + el.setAttribute('role', 'region'); + el.setAttribute('aria-label', 'Recent agent activity'); + el.tabIndex = -1; + popoverEl = el; + return el; + } + + /** + * Replace the popover body with a single status line (loading, empty, error). + * @param {string} text + */ + function showHistoryMessage(text) { + const el = ensurePopover(); + const msg = document.createElement('div'); + msg.className = 'activity-history-message'; + msg.textContent = text; + el.replaceChildren(msg); + } + + /** + * One history row: verb, subject, and a coarse age. Every agent-supplied + * string goes in through textContent, exactly as the live entry does. + * @param {AgentActivityFrame} frame + * @returns {HTMLElement} + */ + function buildHistoryRow(frame) { + const row = document.createElement('div'); + row.className = 'activity-history-row'; + + const { verb, subject } = formatActivity(frame, { labelOf: panelLabel }); + const verbEl = document.createElement('span'); + verbEl.className = 'activity-history-verb'; + verbEl.textContent = verb; + row.appendChild(verbEl); + + if (subject) { + const subjectEl = document.createElement('span'); + subjectEl.className = 'activity-history-subject'; + subjectEl.textContent = subject; + row.appendChild(subjectEl); + } + + const age = formatRelativeTime(frame.ts, Date.now()); + if (age) { + const timeEl = document.createElement('span'); + timeEl.className = 'activity-history-time'; + timeEl.textContent = age; + row.appendChild(timeEl); + } + return row; + } + + /** + * Render the server's history, newest first — the endpoint already orders it + * that way, so rows go in array order, top to bottom. + * @param {AgentActivityFrame[]} events + */ + function renderHistory(events) { + const el = ensurePopover(); + const usable = events.filter((e) => e && e.target); + if (usable.length === 0) { + showHistoryMessage('No recent agent activity'); + return; + } + el.replaceChildren(...usable.slice(0, HISTORY_LIMIT).map(buildHistoryRow)); + } + + /** + * Add a frame that arrived while the popover is open at the top of the list. + * @param {AgentActivityFrame} frame + */ + function prependHistoryRow(frame) { + const el = ensurePopover(); + // A message line ("No recent agent activity", an error) is not a row — + // the first real event replaces it. + const msg = el.querySelector('.activity-history-message'); + if (msg) el.replaceChildren(); + el.prepend(buildHistoryRow(frame)); + while (el.children.length > HISTORY_LIMIT) el.lastElementChild?.remove(); + } + + /** + * Anchor the fixed popover to the strip. It opens upward by default: in the + * hub the strip is the footer status bar, so above is where the room is. + */ + function placeHistory() { + if (!popoverEl) return; + const anchor = mount.getBoundingClientRect(); + const w = popoverEl.offsetWidth; + const h = popoverEl.offsetHeight; + const margin = 4; + let left = anchor.left + anchor.width / 2 - w / 2; + left = Math.max(margin, Math.min(left, window.innerWidth - w - margin)); + let top = anchor.top - h - margin; + if (top < margin) top = Math.min(anchor.bottom + margin, window.innerHeight - h - margin); + popoverEl.style.left = `${Math.round(left)}px`; + popoverEl.style.top = `${Math.round(Math.max(margin, top))}px`; + } + + /** @param {MouseEvent} e */ + function onDocumentClick(e) { + if (!(e.target instanceof Node)) return; + if (mount.contains(e.target)) return; + if (popoverEl && popoverEl.contains(e.target)) return; + closeHistory(); + } + + /** @param {KeyboardEvent} e */ + function onDocumentKeydown(e) { + if (e.key === 'Escape') { + closeHistory(); + mount.focus(); + } + } + + function isHistoryOpen() { + return historyOpen; + } + + async function openHistory() { + if (historyOpen) return; + historyOpen = true; + const generation = ++historyGeneration; + + const el = ensurePopover(); + showHistoryMessage('Loading…'); + document.body.appendChild(el); + mount.setAttribute('aria-expanded', 'true'); + placeHistory(); + // Capture phase, so an outside click closes before it does anything else. + document.addEventListener('click', onDocumentClick, true); + document.addEventListener('keydown', onDocumentKeydown, true); + window.addEventListener('resize', placeHistory); + el.focus(); + + /** @type {AgentActivityFrame[] | null} */ + let events = null; + try { + events = await fetchRecent(HISTORY_LIMIT); + } catch { + events = null; + } + // Closed (or closed and reopened) while the request was in flight. + if (generation !== historyGeneration) return; + if (events == null) showHistoryMessage('Could not load recent activity'); + else renderHistory(events); + placeHistory(); + } + + function closeHistory() { + if (!historyOpen) return; + historyOpen = false; + historyGeneration++; + popoverEl?.remove(); + mount.setAttribute('aria-expanded', 'false'); + document.removeEventListener('click', onDocumentClick, true); + document.removeEventListener('keydown', onDocumentKeydown, true); + window.removeEventListener('resize', placeHistory); + } + + function toggleHistory() { + if (historyOpen) closeHistory(); + else void openHistory(); + } + + mount.classList.add('activity-strip-trigger'); + mount.setAttribute('aria-expanded', 'false'); + mount.title = 'Recent agent activity'; + if (!mount.hasAttribute('tabindex')) mount.tabIndex = 0; + mount.addEventListener('click', (e) => { + e.stopPropagation(); + toggleHistory(); + }); + mount.addEventListener('keydown', (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleHistory(); + } + }); + + return { + open: openHistory, + close: closeHistory, + isOpen: isHistoryOpen, + prepend: prependHistoryRow, + }; +} diff --git a/src/osprey/interfaces/web_terminal/static/js/activity-strip.js b/src/osprey/interfaces/web_terminal/static/js/activity-strip.js index 906b88f39..f80740490 100644 --- a/src/osprey/interfaces/web_terminal/static/js/activity-strip.js +++ b/src/osprey/interfaces/web_terminal/static/js/activity-strip.js @@ -18,11 +18,18 @@ * fired in panel-manager). The kind→panel mapping is the SUPPRESSION table * below; suppression itself is a pure function (exported for tests). * + * Three modules, one feature: this one owns the live line, activity-format.js + * words every frame for both surfaces, and activity-history.js is the + * click-to-expand popover over the server's ring (the strip mount is its + * trigger, so the strip builds it and hands out its open/close). + * * All agent-supplied strings (tool, detail, panel) are rendered as text nodes * only — createElement + textContent, never innerHTML. */ -import { setActivityStripHandler, getActivePanel } from './panel-manager.js'; +import { setActivityStripHandler, getActivePanel, labelOf } from './panel-manager.js'; +import { ARRANGE_TOOL, formatActivity } from './activity-format.js'; +import { createActivityHistory } from './activity-history.js'; /** @typedef {import('./panel-manager.js').AgentActivityEvent} AgentActivityFrame */ /** @typedef {AgentActivityFrame['target']} ActivityTarget */ @@ -68,50 +75,44 @@ export function isSuppressed(target, activePanel) { return mapped != null && mapped === activePanel; } -/** - * Human-readable two-part label for a frame. The subject carries the - * agent-supplied string and is rendered as a text node by the caller. - * @param {AgentActivityFrame} frame - * @returns {{ verb: string, subject: string }} - */ -export function formatActivity(frame) { - const t = frame.target; - switch (t.kind) { - case 'channel': - return { verb: 'agent wrote', subject: t.detail || frame.tool }; - case 'run': - return t.detail - ? { verb: 'agent launched run', subject: t.detail } - : { verb: 'agent launched a run', subject: '' }; - case 'artifact': - return { verb: 'agent focused', subject: t.detail || 'an artifact' }; - case 'panel': - // Generic fallback: a panel-kind frame whose id had no rail entry. - return { verb: 'agent touched', subject: t.panel || frame.tool }; - default: - // Unknown future kind from a newer server — still show something. - return { verb: 'agent activity', subject: frame.tool }; - } -} - // ---- Strip factory ---- /** * Build a strip bound to a mount element. Dependencies are injected so tests - * drive it directly (frames via handleActivity, active panel via a stub). + * drive it directly (frames via handleActivity, active panel via a stub, + * history via a stub reader). * @param {{ * mount: HTMLElement, * getActivePanel: () => string | null, * clearMs?: number, + * fetchRecent?: (limit: number) => Promise, + * labelOf?: (id: string) => string, * }} deps - * @returns {{ handleActivity: (frame: AgentActivityFrame) => void, clear: () => void }} + * @returns {{ + * handleActivity: (frame: AgentActivityFrame) => void, + * clear: () => void, + * openHistory: () => Promise, + * closeHistory: () => void, + * isHistoryOpen: () => boolean, + * }} */ -export function createActivityStrip({ mount, getActivePanel, clearMs = ACTIVITY_CLEAR_MS }) { +export function createActivityStrip({ + mount, + getActivePanel, + clearMs = ACTIVITY_CLEAR_MS, + fetchRecent, + labelOf: panelLabel, +}) { /** @type {ReturnType | null} */ let timer = null; + /** Arrange frames shown back-to-back in the current live window (see below). */ + let arrangeRun = 0; + + const history = createActivityHistory({ mount, fetchRecent, labelOf: panelLabel }); function clear() { if (timer != null) { clearTimeout(timer); timer = null; } + arrangeRun = 0; mount.textContent = ''; } @@ -119,9 +120,21 @@ export function createActivityStrip({ mount, getActivePanel, clearMs = ACTIVITY_ function handleActivity(frame) { const target = frame?.target; if (!target) return; // malformed frame — ignore + + // History records what the agent did, not what the strip chose to show, + // so the live prepend happens BEFORE the suppression check — the server + // ring keeps suppressed frames too, and the two must not disagree. + if (history.isOpen()) history.prepend(frame); + if (isSuppressed(target, getActivePanel())) return; - const { verb, subject } = formatActivity(frame); + // Arranging a workspace lands one frame per tile, and the single slot would + // otherwise flicker through them. Same idiom as the latest-wins replacement + // below — the run collapses into one line — except that consecutive arrange + // frames count up instead of overwriting. Anything else ends the run. + arrangeRun = frame.tool === ARRANGE_TOOL ? arrangeRun + 1 : 0; + + const { verb, subject } = formatActivity(frame, { labelOf: panelLabel, count: arrangeRun }); // Text nodes only — agent-supplied strings must never reach innerHTML. const entry = document.createElement('span'); @@ -144,7 +157,13 @@ export function createActivityStrip({ mount, getActivePanel, clearMs = ACTIVITY_ timer = setTimeout(clear, clearMs); } - return { handleActivity, clear }; + return { + handleActivity, + clear, + openHistory: history.open, + closeHistory: history.close, + isHistoryOpen: history.isOpen, + }; } // ---- Self-boot ---- @@ -154,15 +173,32 @@ export function createActivityStrip({ mount, getActivePanel, clearMs = ACTIVITY_ // mount and this module registers itself on panel-manager's seam. Pages // without the mount (or without a running panel-manager) no-op harmlessly. -function boot() { +/** @type {ReturnType | null} */ +let bootedStrip = null; + +/** + * Boot the page's one strip on the template's #activity-strip mount and + * register it on panel-manager's seam. + * + * Idempotent, and that is load-bearing: the session page (session.js) drives + * the strip from its own SSE subscription because no panel-manager runs + * there, so it calls this to reach the same instance the module's own boot + * creates. A second strip on the shared mount would bind a second set of + * click handlers and open a second history popover. + * + * @returns {ReturnType | null} null on a page with no mount + */ +export function bootActivityStrip() { + if (bootedStrip) return bootedStrip; const mount = document.getElementById('activity-strip'); - if (!mount) return; - const strip = createActivityStrip({ mount, getActivePanel }); - setActivityStripHandler(strip.handleActivity); + if (!mount) return null; + bootedStrip = createActivityStrip({ mount, getActivePanel, labelOf }); + setActivityStripHandler(bootedStrip.handleActivity); + return bootedStrip; } if (document.readyState === 'loading') { - document.addEventListener('DOMContentLoaded', boot, { once: true }); + document.addEventListener('DOMContentLoaded', bootActivityStrip, { once: true }); } else { - boot(); + bootActivityStrip(); } diff --git a/src/osprey/interfaces/web_terminal/static/js/chat-render.js b/src/osprey/interfaces/web_terminal/static/js/chat-render.js index 784d871e3..f60557f49 100644 --- a/src/osprey/interfaces/web_terminal/static/js/chat-render.js +++ b/src/osprey/interfaces/web_terminal/static/js/chat-render.js @@ -34,6 +34,8 @@ * | `tool_result` | `result` | `session_reset` | `error` | `system` | … * @property {string} [content] - incremental text (`text` events) * @property {string} [tool_name] - display name, prefix-stripped (`tool_use`) + * @property {string} [tool_name_raw] - the SDK's own tool name, prefix intact + * (`tool_use`); the preferred {@link TOOL_PHRASES} key * @property {string} [message] - human-readable error text (`error` events) * @property {boolean} [is_error] - turn/tool errored (`result`, `tool_result`) */ @@ -148,6 +150,196 @@ export function renderMarkdownInto(el, text) { } } +// ---- Tool vocabulary ---- // + +/** + * Operator phrases for the tools a turn is likely to use, keyed by normalised + * tool name (see {@link normaliseToolName}). + * + * In Simple mode the chat is the whole interface, so this line is the only + * account an operator gets of what the agent is doing. Phrases say it in + * control-room terms and share the activity strip's vocabulary (open/close/ + * focus/arrange for panels, "wrote" for channels). + * + * They are lower-case gerunds — "writing control channels" — so a caller can + * drop one mid-sentence; {@link activityLabel} capitalises for the activity + * line. Coverage is deliberately partial: a tool whose formatted name already + * reads as plain English ("List Panels", "Session Summary") falls through to + * the raw-name fallback instead of earning a row. Adding one is a single line. + * + * Phrases cannot name the specific panel, channel, or file involved: the chat + * route (`_strip_for_chat`) drops a `tool_use` event's `input` before it leaves + * the server, so no arguments reach this module. + * + * @type {Readonly>} + */ +export const TOOL_PHRASES = Object.freeze({ + // Control system — the writes an operator most needs to see coming. + channel_write: 'writing control channels', + channel_read: 'reading control channels', + channel_limits: 'checking channel limits', + archiver_read: 'reading archived data', + archiver_downsample: 'thinning archived data', + + // Python executor. + execute: 'running Python', + execute_file: 'running Python', + + // Workspace panels. The synthetic panel activity uses these same names. + show_panel: 'opening a panel', + hide_panel: 'closing a panel', + switch_panel: 'switching panels', + arrange_workspace: 'arranging the workspace', + register_panel: 'adding a panel', + manage_window: 'arranging a window', + screenshot_capture: 'taking a screenshot', + + // Workspace artifacts and saved data. + artifact_save: 'saving an artifact', + artifact_get: 'opening an artifact', + artifact_focus: 'showing an artifact', + artifact_pin: 'pinning an artifact', + artifact_export: 'exporting an artifact', + artifact_delete: 'deleting an artifact', + artifact_delete_all: 'deleting every artifact', + create_static_plot: 'drawing a plot', + create_interactive_plot: 'drawing an interactive plot', + create_dashboard: 'building a dashboard', + create_document: 'writing a document', + data_list: 'listing saved data', + data_read: 'reading saved data', + data_delete: 'deleting saved data', + + // Workspace project setup and session record. + setup_inspect: 'inspecting the project setup', + setup_patch: 'changing the project setup', + session_log: 'writing to the session log', + + // Scan queue (bluesky). + queue_status: 'checking the scan queue', + queue_list: 'listing the scan queue', + queue_add: 'queueing a scan', + queue_start: 'starting the scan queue', + queue_stop: 'stopping the scan queue', + stop_run: 'stopping the running scan', + write_plan: 'drafting a scan plan', + validate_plan: 'checking the scan plan', + get_draft: 'reading the scan draft', + set_draft: 'editing the scan draft', + clear_draft: 'clearing the scan draft', + get_run: 'reading a scan run', + get_run_data: 'reading scan data', + + // Phoebus displays. + phoebus_open_panel: 'opening a Phoebus display', + phoebus_open_databrowser: 'opening the data browser', + phoebus_list_displays: 'listing Phoebus displays', + phoebus_perceive: 'reading a Phoebus display', + phoebus_perceive_region: 'reading part of a Phoebus display', + phoebus_snapshot: 'capturing a Phoebus display', + phoebus_drive: 'operating a Phoebus display', + + // Logbook (ARIEL). + browse: 'browsing the logbook', + keyword_search: 'searching the logbook', + semantic_search: 'searching the logbook', + sql_query: 'querying the logbook', + filter_options: 'listing logbook filters', + entry_get: 'reading a logbook entry', + entries_by_ids: 'reading logbook entries', + entry_create: 'drafting a logbook entry', + entry_publish: 'publishing a logbook entry', + + // Channel finder. + list_channels: 'looking up channels', + query_channels: 'looking up channels', + build_channels: 'building a channel list', + list_families: 'looking up channel families', + list_systems: 'looking up systems', + get_common_names: 'looking up channel names', + inspect_fields: 'inspecting channel fields', + + // Facility knowledge. + list_concepts: 'browsing facility knowledge', + read_concept: 'reading facility knowledge', + draft_concept: 'drafting a facility note', + + // Lattice model. The mutators are phrased; the getters fall back. + lattice_init: 'loading the lattice', + lattice_state: 'reading the lattice', + lattice_set_param: 'changing a lattice parameter', + lattice_set_baseline: 'setting the lattice baseline', + lattice_clear_baseline: 'clearing the lattice baseline', + lattice_update_settings: 'changing lattice settings', + lattice_refresh: 'refreshing the lattice view', + + // Health. + health_check: 'checking system health', + health_check_full: 'checking system health', + + // Built-in agent tools. + read: 'reading a file', + write: 'writing a file', + edit: 'editing a file', + bash: 'running a shell command', + glob: 'looking for files', + grep: 'searching files', + task: 'delegating to a helper agent', + todowrite: 'updating its plan', + webfetch: 'fetching a web page', + websearch: 'searching the web', +}); + +/** + * Fold a tool name to a {@link TOOL_PHRASES} key: drop the `mcp____` + * prefix, collapse whitespace and hyphens to underscores, lower-case. + * + * A `tool_use` event carries the name twice — `tool_name_raw` + * (`mcp__osprey__channel_write`) and the server-formatted `tool_name` + * (`Channel Write`, from operator_session `_format_tool_name`) — and both fold + * to the same key, so the table works whichever spelling an event carries. + * + * @param {string} name + * @returns {string} + */ +export function normaliseToolName(name) { + return name + .replace(/^mcp__[^_]+__/, '') + .trim() + .replace(/[\s-]+/g, '_') + .toLowerCase(); +} + +/** + * The operator phrase for a `tool_use` event, or null when the tool has no + * table entry. Lower-case and sentence-fragment shaped; capitalise at the + * point of display. + * @param {ChatEvent} event + * @returns {string | null} + */ +export function toolPhrase(event) { + for (const name of [event.tool_name_raw, event.tool_name]) { + if (!name) continue; + const phrase = TOOL_PHRASES[normaliseToolName(name)]; + if (phrase) return phrase; + } + return null; +} + +/** + * The activity-line label for a `tool_use` event. A mapped tool reads as a + * sentence ("Writing control channels…"); anything unmapped keeps its raw name + * ("Using Queue Reorder…"), so a tool added elsewhere in the codebase is never + * invisible here — only terse until it earns a phrase. + * @param {ChatEvent} event + * @returns {string} + */ +export function activityLabel(event) { + const phrase = toolPhrase(event); + if (phrase !== null) return `${phrase.charAt(0).toUpperCase()}${phrase.slice(1)}…`; + return `Using ${event.tool_name ?? event.tool_name_raw ?? 'tool'}…`; +} + // ---- Pure DOM builders ---- // /** @@ -348,7 +540,7 @@ export function createChatRenderer(container) { setActivity('Thinking…'); break; case 'tool_use': - setActivity(`Using ${event.tool_name ?? 'tool'}…`); + setActivity(activityLabel(event)); break; case 'tool_result': // Stripped of its body; nothing to render. The activity line stays as diff --git a/src/osprey/interfaces/web_terminal/static/js/dock-iframe.js b/src/osprey/interfaces/web_terminal/static/js/dock-iframe.js index d0b52cd01..89dc732e5 100644 --- a/src/osprey/interfaces/web_terminal/static/js/dock-iframe.js +++ b/src/osprey/interfaces/web_terminal/static/js/dock-iframe.js @@ -60,12 +60,14 @@ import { setServiceRedock, } from './dock-workspace.js'; import { PLACEHOLDER_PREFIX } from './dock-reconcile.js'; +import { flashElement } from '/design-system/js/highlight.js'; /** * One tracked service panel: its cached iframe (created/owned by panel-manager), * the id of the empty dockview placeholder it follows, the tab title, whether it - * is currently meant to be on screen (false once closed/hidden), and the last - * synced size (to throttle resize re-dispatch to real size changes). + * is currently meant to be on screen (false once closed/hidden), the last synced + * size (to throttle resize re-dispatch to real size changes), and its lazily + * created agent-glow overlay (see glowPanel). * @typedef {object} ManagedPanel * @property {HTMLIFrameElement} iframe * @property {string} placeholderId @@ -73,6 +75,7 @@ import { PLACEHOLDER_PREFIX } from './dock-reconcile.js'; * @property {boolean} visible * @property {number} [lastW] * @property {number} [lastH] + * @property {HTMLElement} [glowEl] */ const OVERLAY_CLASS = 'dock-iframe-overlay'; @@ -654,3 +657,80 @@ export function concealPanel(panelId) { entry.visible = false; entry.iframe.style.display = 'none'; } + +// ---- Agent attribution glow ------------------------------------------------ + +/** Class of the per-panel glow element; styled in css/terminal.css. */ +const GLOW_CLASS = 'tile-glow'; + +/** + * The glow element for a managed panel, created on first use and reused after. + * It is a sibling of the overlay iframes rather than anything inside them: the + * iframes are separate documents this stylesheet cannot reach, and an + * `.agent-flash` on the iframe element itself would clip the embedded app to + * the flash's border-radius. The element is transparent and pointer-inert at + * rest, so a spent glow can simply stay parked in the overlay. + * @param {ManagedPanel} entry + * @returns {HTMLElement} + */ +function ensureGlowEl(entry) { + if (entry.glowEl?.isConnected) return entry.glowEl; + const el = document.createElement('div'); + el.className = GLOW_CLASS; + /** @type {HTMLElement} */ (overlayEl).appendChild(el); + entry.glowEl = el; + return el; +} + +/** + * Flash the agent-activity glow over a panel's TILE BODY — the visual companion + * to the rail entry's flash, so an agent action reads on the panel it actually + * touched and not only on a ~20px rail tab. + * + * No-ops unless the panel is genuinely on screen: it must be managed, visible, + * hold a live placeholder, and be the ACTIVE tab in that placeholder's group. + * Anything else has no rectangle to glow, and glowing the tile a hidden panel + * sits behind would attribute the action to the wrong panel. + * + * The rectangle is read inside a requestAnimationFrame: a glow commonly follows + * the activation that created the tile, and dockview's geometry only lands once + * the layout settles — reading synchronously would measure a zero-sized (or + * stale) group. + * + * FALLBACK (no dockview): there are no tiles, so the single mounted host + * (#panel-content) is the panel body and takes the flash directly. + * @param {string} panelId + */ +export function glowPanel(panelId) { + const api = ensureDock(); + if (!api || !overlayEl) { + if (fallbackHostEl) flashElement(fallbackHostEl); + return; + } + if (!managed.has(panelId)) return; + requestAnimationFrame(() => flashTileGlow(panelId)); +} + +/** + * Deferred half of glowPanel: re-resolve the panel (the tile may have moved, + * closed, or lost focus during the frame), copy its group content rectangle + * onto the glow element with applyGeometry's math, and fire the flash. + * @param {string} panelId + */ +function flashTileGlow(panelId) { + const dockApi = getDockApi(); + const entry = managed.get(panelId); + if (!dockApi || !overlayEl || !entry?.visible) return; + const panel = dockApi.getPanel(entry.placeholderId); + const group = panel?.group; + const content = group?.element?.querySelector('.dv-content-container'); + if (!panel || !content || group.activePanel !== panel) return; + const base = overlayEl.getBoundingClientRect(); + const r = content.getBoundingClientRect(); + const el = ensureGlowEl(entry); + el.style.left = Math.round(r.left - base.left) + 'px'; + el.style.top = Math.round(r.top - base.top) + 'px'; + el.style.width = Math.round(r.width) + 'px'; + el.style.height = Math.round(r.height) + 'px'; + flashElement(el); +} diff --git a/src/osprey/interfaces/web_terminal/static/js/panel-manager.js b/src/osprey/interfaces/web_terminal/static/js/panel-manager.js index 6ec308304..fa7a83d8c 100644 --- a/src/osprey/interfaces/web_terminal/static/js/panel-manager.js +++ b/src/osprey/interfaces/web_terminal/static/js/panel-manager.js @@ -26,7 +26,7 @@ import { applyPreset, wirePanelHeaderControls } from './panel-presets.js'; import { setPanelVisibility, setPanelFocus, registerUrlPanel } from './panel-commands.js'; import { initDockIframeAdapter, focusPanel, hidePanel, concealPanel, - setKnownServicePanels, setServerVisiblePanels, + setKnownServicePanels, setServerVisiblePanels, glowPanel, } from './dock-iframe.js'; import { initPanelPlacement, openPanelBeside, dropPanelAt, applyAgentSwitch, applyArrange, @@ -96,7 +96,8 @@ import { * @typedef {object} AgentActivityEvent * @property {'agent_activity'} type * @property {string} tool - * @property {{ kind: 'panel' | 'channel' | 'run' | 'artifact', panel?: string, detail?: string }} target + * @property {{ kind: 'panel' | 'channel' | 'run' | 'artifact' | 'config' | 'ui', + * panel?: string, detail?: string }} target * @property {number} [ts] * * @typedef {PanelFocusEvent | PanelVisibilityEvent | PanelRegisterEvent | PanelArrangeEvent @@ -208,7 +209,11 @@ export async function initPanelManager(panelId) { getActive: () => activeTabId, clearActive: clearActivePanel, renderEmpty: renderEmptyState, - glow: flashAgentGlow, + // An arranged tile is attributed on both surfaces: its rail entry flashes, + // and the tile body itself glows. Only the arrange path passes through + // here, which is the one placement verb an agent drives — the rail ⊞ and + // drag-and-drop are human gestures and never glow. + glow: (id) => { flashAgentGlow(id); glowPanel(id); }, openTerminal: openTerminalPanel, }); @@ -407,7 +412,9 @@ export async function initPanelManager(panelId) { // without a resync a client that missed one frame never converges again. // The hook fires on every open, including the first; the extra boot-time // fetch is a no-op delta. - onOpen: () => { void resyncPanelState(); }, + // Badges are restored from the history ring after the membership delta, so + // an entry the resync just re-added can carry one (restoreAgentBadges). + onOpen: () => { void resyncPanelState().then(restoreAgentBadges); }, onMessage: (raw) => { try { const data = /** @type {PanelSSEEvent} */ (raw); @@ -425,10 +432,12 @@ export async function initPanelManager(panelId) { // the gesturing client applies it locally), so an unattributed frame // can only come from an out-of-contract caller; it keeps the plain // activation. The glow runs after the switch so a just-added entry - // can flash. + // can flash, and the tile glow after the placement so it measures the + // tile the switch actually surfaced. if (data.source === 'agent') { applyAgentSwitch(data.panel); flashAgentGlow(data.panel); + glowPanel(data.panel); } else { activateTab(data.panel); } @@ -459,7 +468,13 @@ export async function initPanelManager(panelId) { // setEntryAttention; everything the rail cannot anchor falls through // to the activity-strip seam (no-op until a handler registers). const t = data.target; - if (t.kind !== 'panel' || !t.panel || !setEntryAttention(railEl, t.panel, true)) onAgentActivity(data); + if (t.kind === 'panel' && t.panel && setEntryAttention(railEl, t.panel, true, data.ts)) { + // Remember the badge's server ts so clearing it can acknowledge + // exactly this event and the reload restore can skip it. + noteBadgeTs(t.panel, data.ts); + } else { + onAgentActivity(data); + } } } catch (err) { @@ -478,7 +493,8 @@ export async function initPanelManager(panelId) { * ends up exactly where a connected one would be. * * Order matters and is pinned by the test suite: membership + rail entry - * first (the agent glow runs after the add so a just-added entry can flash); + * first (the agent glow runs after the add so a just-added entry can flash, + * and an agent-origin change reports itself on the activity strip); * then the simple-UX chat-only reveal (showing a panel while the workspace is * suppressed brings the workspace up ON that panel — {auto: true} keeps the * health guard); then, on a hide, the dock tile drop (one panel per tile — a @@ -497,7 +513,14 @@ function applyPanelVisibility(panel, visible, source) { visiblePanels.delete(panel); removeEntry(railEl, panel); } - if (source === 'agent') flashAgentGlow(panel); + // A show can glow its just-added rail entry; a hide has no entry left to + // glow, so the strip is the only surface that can report it. Both synthesize + // an activity frame — deliberately straight to the seam, past the + // agent_activity branch's rail-anchor routing, so the two halves of the + // agent's visibility vocabulary read the same way on the strip. + if (visible && source === 'agent') flashAgentGlow(panel); + if (source === 'agent') onAgentActivity({ type: 'agent_activity', ts: Date.now(), + tool: visible ? 'show_panel' : 'hide_panel', target: { kind: 'panel', panel } }); if (visible && workspaceSuppressed) { workspaceSuppressed = false; @@ -554,6 +577,85 @@ async function resyncPanelState() { */ function flashAgentGlow(panelId) { const entry = getEntry(railEl, panelId); if (entry) flashElement(entry); } +// ---- Agent-attention badges across reloads ---- +// +// A badge must outlive the page: the server's history ring is re-read on every +// SSE open (restoreAgentBadges) and any panel activity the operator has not +// seen re-badges its entry. "Seen" is an ACKNOWLEDGMENT — the server ts of the +// newest badge the operator cleared by surfacing that panel, kept per panel in +// localStorage under `agent-ack:`. +// +// Only SERVER timestamps are ever stored or compared here. The ring's `ts` is +// the web-terminal process's clock; a browser clock skewed ahead of it would +// permanently suppress real badges, and one skewed behind would resurrect +// cleared ones on every reconnect. So a badge whose frame carried no ts leaves +// the stored ack untouched rather than substituting Date.now(). + +/** Newest badge-causing server ts seen this page lifetime, per panel. + * @type {Map} */ +const badgeTs = new Map(); + +const ACK_KEY_PREFIX = 'agent-ack:'; + +/** + * Record a badge's server ts, keeping the newest. The history ring replays a + * panel's older events alongside its newest, so this must not walk backwards. + * @param {string} panelId + * @param {number} [ts] + */ +function noteBadgeTs(panelId, ts) { + if (typeof ts !== 'number') return; + const prev = badgeTs.get(panelId); + if (prev === undefined || ts > prev) badgeTs.set(panelId, ts); +} + +/** + * The acknowledged server ts for a panel. A panel that was never acknowledged + * (and a storage read that is denied or corrupt) has seen nothing, so it reads + * as -Infinity: an unknown ack RESTORES a badge, it never suppresses one. + * @param {string} panelId + * @returns {number} + */ +function ackedTs(panelId) { + let raw = null; + try { raw = localStorage.getItem(ACK_KEY_PREFIX + panelId); } catch { return -Infinity; } + const ts = raw === null ? NaN : Number(raw); + return Number.isFinite(ts) ? ts : -Infinity; +} + +/** + * Clear a panel's badge and acknowledge it up to that badge's own server ts, + * so a reload does not bring it back. With no ts on record the badge is still + * cleared but the stored ack is left exactly as it was (see the section note). + * @param {string} panelId + */ +function clearBadge(panelId) { + setEntryAttention(railEl, panelId, false); + const ts = badgeTs.get(panelId); + if (ts === undefined) return; + try { localStorage.setItem(ACK_KEY_PREFIX + panelId, String(ts)); } catch { /* storage denied */ } +} + +/** + * Re-badge rail entries for panel activity this operator has not acknowledged. + * Runs after the membership resync on every SSE open — including the first, so + * a reload restores badges — which is also why it runs after it: an entry that + * the resync delta just added can then carry a badge. + * + * Only 'panel'-kind rows with a ts can badge anything; the strip owns the rest + * and history there is its own concern (the seam is not replayed). + */ +async function restoreAgentBadges() { + let body = null; + try { body = await fetchJSON('/api/agent-activity/recent'); } catch { return; } + for (const ev of (body?.events || [])) { + const target = ev?.target; + if (target?.kind !== 'panel' || !target.panel || typeof ev.ts !== 'number') continue; + if (ev.ts <= ackedTs(target.panel)) continue; + if (setEntryAttention(railEl, target.panel, true, ev.ts)) noteBadgeTs(target.panel, ev.ts); + } +} + /** * Interaction closures handed to every rail render/append call. Routing * activation and close through here keeps a human click/"×" and an agent MCP @@ -610,8 +712,9 @@ function railOptions() { } /** The catalog label for a panel id (falls back to the id itself). + * Exported for the activity strip, which words panel actions with labels. * @param {string} id @returns {string} */ -function labelOf(id) { +export function labelOf(id) { return PANELS.find((p) => p.id === id)?.label ?? id; } @@ -911,10 +1014,11 @@ export function activateTab(panelId, { userInitiated = false, auto = false } = { if (auto && !visiblePanels.has(panelId)) return; // Past the guards the panel actually surfaces (rail click, agent focus, - // palette, dock — any source): its agent-attention badge is served, clear it. + // palette, dock — any source): its agent-attention badge is served, so clear + // it AND acknowledge it, or the next reload would restore it from history. // The guarded returns above deliberately keep the badge on panels that // refused to surface. - setEntryAttention(railEl, panelId, false); + clearBadge(panelId); // Any surfaced panel means the workspace is open — the simple-UX chat-only // suppression (if still armed) is over for this page lifetime. @@ -1015,6 +1119,15 @@ function navigatePanel(panelId, url) { state.pendingUrl = null; } +/** Human-local twin of the panel_focus SSE path: navigate a panel to `url` + * and plain-activate it. No agent glow, no server broadcast. + * @param {string} panelId + * @param {string} url */ +export function navigateAndActivatePanel(panelId, url) { + if (url) navigatePanel(panelId, url); + activateTab(panelId); +} + // ---- Iframe Management ---- /** Build + adopt the panel's iframe via panel-iframe-factory.js (which owns diff --git a/src/osprey/interfaces/web_terminal/static/js/panel-placement.js b/src/osprey/interfaces/web_terminal/static/js/panel-placement.js index a9a68a01f..f021057b3 100644 --- a/src/osprey/interfaces/web_terminal/static/js/panel-placement.js +++ b/src/osprey/interfaces/web_terminal/static/js/panel-placement.js @@ -57,7 +57,7 @@ import { TERMINAL_RAIL_ID } from './panel-catalog.js'; * @property {() => string | null} getActive - the locally surfaced panel id * @property {() => void} clearActive - drop the local active accent/stamp * @property {(message: string) => void} renderEmpty - paint the strand-proof empty pane - * @property {(id: string) => void} glow - transient agent glow on a rail entry + * @property {(id: string) => void} glow - transient agent glow on a panel's rail entry and its tile * @property {() => void} openTerminal - reopen the native terminal tile */ diff --git a/src/osprey/interfaces/web_terminal/static/js/panel-rail.js b/src/osprey/interfaces/web_terminal/static/js/panel-rail.js index 6dc5f174d..f11d69c0f 100644 --- a/src/osprey/interfaces/web_terminal/static/js/panel-rail.js +++ b/src/osprey/interfaces/web_terminal/static/js/panel-rail.js @@ -44,7 +44,9 @@ * * * State classes on an entry: `.active` (surfaced panel), `.disabled` (backend - * not healthy yet), `.agent-attention` (badge). + * not healthy yet), `.agent-attention` (badge). A badged entry also carries a + * transient `data-title-base` holding the tooltip text the badge borrowed; + * clearing the badge restores it and removes the attribute. */ import { flashElement } from '/design-system/js/highlight.js'; @@ -80,6 +82,15 @@ import { flashElement } from '/design-system/js/highlight.js'; const BUTTON_SELECTOR = '.panel-rail-button'; +/** + * Where an entry's pre-suffix tooltip is parked while the agent-attention + * badge owns the `title`. Present only for the badge's lifetime — see + * {@link applyTouchedTooltip}. + */ +const TITLE_BASE_ATTR = 'data-title-base'; + +const TOUCHED_SEPARATOR = ' · agent touched '; + // ---- Rendering ---- /** @@ -302,23 +313,68 @@ export function setEntryEnabled(railEl, panelId, enabled) { getEntry(railEl, panelId)?.classList.toggle('disabled', !enabled); } +/** + * Point an entry's tooltip at the moment the agent touched its panel, or put + * the tooltip back the way it was. + * + * The pre-suffix text is stashed on the entry for the badge's lifetime rather + * than recomputed, so the restore is exact even if the caller retitled the + * entry, and so a second event REPLACES the time instead of appending a second + * suffix. Restoring is keyed on the stash, which makes a clear on an unbadged + * entry a true no-op. + * @param {HTMLElement} entry + * @param {number | null} ts - server epoch seconds, or null to restore the base + */ +function applyTouchedTooltip(entry, ts) { + const base = entry.getAttribute(TITLE_BASE_ATTR) ?? entry.title; + if (ts === null) { + if (entry.hasAttribute(TITLE_BASE_ATTR)) { + entry.title = base; + entry.removeAttribute(TITLE_BASE_ATTR); + } + return; + } + const touchedAt = new Date(ts * 1000).toLocaleTimeString([], { + hour: 'numeric', + minute: '2-digit', + }); + entry.setAttribute(TITLE_BASE_ATTR, base); + entry.title = `${base}${TOUCHED_SEPARATOR}${touchedAt}`; +} + /** * Set or clear the agent-attention affordance on an entry. Turning it on * toggles the persistent `agent-attention` badge class (the design system's * highlight.css draws an absolutely-positioned `::after` accent dot — class - * only, no child nodes, no layout shift) and fires the one-shot `agent-flash` - * glow via {@link flashElement}. Turning it off removes only the badge class; - * an in-flight flash is left to finish on its own `animationend`. + * only, no child nodes, no layout shift), fires the one-shot `agent-flash` + * glow via {@link flashElement}, and scrolls the entry into view: a rail + * taller than its viewport can otherwise take a badge entirely off-screen, + * which is the one case where the affordance reports nothing to the operator. + * `block: 'nearest'` leaves an already-visible entry exactly where it is. + * + * Turning it off removes the badge class and restores the tooltip; an + * in-flight flash is left to finish on its own `animationend`. * @param {HTMLElement} railEl * @param {string} panelId * @param {boolean} on + * @param {number} [ts] - the originating event's SERVER timestamp (epoch + * seconds, as the `agent_activity` SSE frames carry it), appended to the + * entry's tooltip as "· agent touched