From 8c59cf8d4f3dd26379c250ef382447bed67ef12c Mon Sep 17 00:00:00 2001 From: praxagent Date: Sun, 26 Jul 2026 23:12:54 +0000 Subject: [PATCH] fix(mcp): show an agent's writes without reloading the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A card filed over MCP did not appear until the browser was refreshed. The Kanban and note queries have no polling and no subscription, so they only refreshed when the UI itself made the change — which means a write from anywhere else was invisible by construction. That is worst for exactly the case this feature exists for: an agent working in the background, while you are not thinking to press reload. A successful mutation now broadcasts library:update carrying the space, and the browser invalidates the Kanban, the columns and the Library tree. Three deliberate details: - It fires only AFTER the write succeeds. Announcing the attempt would put the UI a step ahead of the data — it would refetch, see the old state, and quietly disagree with what the agent was just told. - Reads announce nothing. Nothing changed, so there is nothing to refetch, and a listing agent should not make every open browser do work. - A failed broadcast does not fail the write. The write already happened; reporting failure would tell the agent its change was rejected when it was not, and a reload still shows the truth. A missed notification is the smaller harm, so it is logged and swallowed. The announcer is injected by the route rather than reached for inside mcp_server, same as the comment poster — the websocket manager is a thing only the HTTP layer has, and keeping it out means the tool layer still tests without a running app. Broadcast to all clients: a Library space is not scoped to a TeamWork project, and a client with no query for it ignores the event. 45 MCP tests, 282 TeamWork tests green, tsc + build clean. --- frontend/src/hooks/useWebSocket.ts | 15 +++++ frontend/src/types/index.ts | 2 + src/teamwork/mcp_server.py | 41 +++++++++--- src/teamwork/routers/mcp.py | 25 +++++++- src/teamwork/websocket/connection_manager.py | 4 ++ tests/test_mcp_server.py | 66 ++++++++++++++++++++ 6 files changed, 143 insertions(+), 10 deletions(-) diff --git a/frontend/src/hooks/useWebSocket.ts b/frontend/src/hooks/useWebSocket.ts index d32e624..9e82c0f 100644 --- a/frontend/src/hooks/useWebSocket.ts +++ b/frontend/src/hooks/useWebSocket.ts @@ -209,6 +209,21 @@ export function useWebSocket() { } break; } + + case 'library:update': { + // A Library space changed from outside this browser — an MCP client, + // or anything else that is not the UI. These queries do not poll, so + // without this the change is invisible until a reload, which is the + // one thing a background agent guarantees you will not think to do. + const space = event.data.space as string | undefined; + if (space) { + queryClient.invalidateQueries({ queryKey: ['library-tasks', space] }); + queryClient.invalidateQueries({ queryKey: ['library-task-columns', space] }); + } + // The tree carries notebooks and notes and is not keyed by space. + queryClient.invalidateQueries({ queryKey: ['library'] }); + break; + } } }); diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index 5141943..e51efbe 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -177,6 +177,8 @@ export type WebSocketEventType = | 'agent:typing' | 'task:update' | 'task:new' + // A Library space changed from outside this browser (e.g. an MCP client). + | 'library:update' | 'project:update' | 'channel:new' | 'error' diff --git a/src/teamwork/mcp_server.py b/src/teamwork/mcp_server.py index 4b569ea..97c45da 100644 --- a/src/teamwork/mcp_server.py +++ b/src/teamwork/mcp_server.py @@ -68,6 +68,13 @@ def __init__(self, message: str, code: int = -32000): "post_comment": CAP_MESSAGE_POST, } +# Tools that change something. A caller watching the UI should see the result +# without reloading, so these announce themselves; reads say nothing. +MUTATING_TOOLS = frozenset({ + "create_task", "update_task", "comment_on_task", + "create_notebook", "create_note", "update_note", +}) + # Tools that take no ``space`` argument, and so cannot be checked against a # space-scoped key the ordinary way. Each needs its own answer in `authorize`; # adding a tool here without one is how a scope check gets silently skipped. @@ -447,13 +454,17 @@ async def _dispatch_library(tool: str, arguments: dict[str, Any], async def call_tool(client: AgentClient, tool: str, arguments: dict[str, Any], - *, post_comment: Any = None) -> Any: + *, post_comment: Any = None, on_change: Any = None) -> Any: """Authorise and run one tool call. - ``post_comment`` is injected by the route because posting to a channel - needs the request-scoped database session and signature header that only - the HTTP layer has. Everything else goes to the Library over HTTP and needs - neither, which is why this module stays testable without a running app. + ``post_comment`` and ``on_change`` are injected by the route because both + need things only the HTTP layer has — a request-scoped database session, and + the websocket manager. Everything else goes to the Library over HTTP and + needs neither, which is why this module stays testable without a running app. + + ``on_change`` fires only after a mutation actually succeeded. Announcing the + attempt would put the UI a step ahead of the data: it would refetch, see the + old state, and quietly disagree with what the agent was told. """ authorize(client, tool, arguments) @@ -465,11 +476,24 @@ async def call_tool(client: AgentClient, tool: str, arguments: dict[str, Any], return await post_comment(project_id=project_id, channel_id=channel_id, content=content) - return await _dispatch_library(tool, arguments, client) + result = await _dispatch_library(tool, arguments, client) + + if on_change is not None and tool in MUTATING_TOOLS: + try: + await on_change(space=arguments.get("space"), tool=tool) + except Exception: # noqa: BLE001 + # The write already happened. Failing the call now would tell the + # agent its change was rejected when it was not — a reload still + # shows the truth, so a missed notification is the smaller harm. + logger.warning("could not announce an MCP change to the UI", + exc_info=True) + + return result async def handle_request(client: AgentClient, payload: dict[str, Any], - *, post_comment: Any = None) -> dict[str, Any] | None: + *, post_comment: Any = None, + on_change: Any = None) -> dict[str, Any] | None: """Handle one JSON-RPC request. Returns ``None`` for a notification.""" request_id = payload.get("id") method = payload.get("method") or "" @@ -490,7 +514,8 @@ async def handle_request(client: AgentClient, payload: dict[str, Any], tool = params.get("name") or "" arguments = params.get("arguments") or {} try: - result = await call_tool(client, tool, arguments, post_comment=post_comment) + result = await call_tool(client, tool, arguments, + post_comment=post_comment, on_change=on_change) except McpError as exc: # A refused or failed tool call is reported as a tool result with # isError, not a protocol error: the agent should read it, correct diff --git a/src/teamwork/routers/mcp.py b/src/teamwork/routers/mcp.py index 8393b12..3e4de65 100644 --- a/src/teamwork/routers/mcp.py +++ b/src/teamwork/routers/mcp.py @@ -21,6 +21,7 @@ from teamwork.agent_auth import AgentClient from teamwork.models import get_db from teamwork.routers.external import _verify_api_key +from teamwork.websocket import EventType, WebSocketEvent, manager router = APIRouter(tags=["mcp"]) logger = logging.getLogger(__name__) @@ -48,6 +49,24 @@ async def post(*, project_id: str, channel_id: str, content: str) -> Any: return post +def _change_announcer(): + """Tell every open browser that a Library space changed. + + The Kanban and note queries do not poll, so a write that did not come from + the UI was invisible until someone reloaded — which is exactly what an agent + working in the background produces. Broadcast to all: a Library space is not + scoped to a project on the TeamWork side, and a client that does not care + ignores an event it has no query for. + """ + async def announce(*, space: str | None, tool: str) -> None: + await manager.broadcast_all(WebSocketEvent( + type=EventType.LIBRARY_UPDATE, + data={"space": space, "tool": tool, "source": "mcp"}, + )) + + return announce + + @router.post("/mcp") async def mcp_endpoint( request: Request, @@ -58,12 +77,14 @@ async def mcp_endpoint( """One JSON-RPC request or a batch of them.""" if isinstance(payload, list): responses = [await mcp_server.handle_request( - client, item, post_comment=_comment_poster(request, db, client)) + client, item, post_comment=_comment_poster(request, db, client), + on_change=_change_announcer()) for item in payload] return [r for r in responses if r is not None] result = await mcp_server.handle_request( - client, payload, post_comment=_comment_poster(request, db, client)) + client, payload, post_comment=_comment_poster(request, db, client), + on_change=_change_announcer()) if result is None: return {} # notification: acknowledged, no body return result diff --git a/src/teamwork/websocket/connection_manager.py b/src/teamwork/websocket/connection_manager.py index 15bba88..9fae717 100644 --- a/src/teamwork/websocket/connection_manager.py +++ b/src/teamwork/websocket/connection_manager.py @@ -20,6 +20,10 @@ class EventType(str, Enum): AGENT_TYPING = "agent:typing" # Typing indicator TASK_UPDATE = "task:update" TASK_NEW = "task:new" + # A Library space changed from outside this browser — an MCP client, or any + # writer that is not the UI itself. The Kanban and note queries have no + # polling, so without this the only way to see the change is a reload. + LIBRARY_UPDATE = "library:update" PROJECT_UPDATE = "project:update" CHANNEL_NEW = "channel:new" ERROR = "error" diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index c75b56b..71d77dc 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -414,3 +414,69 @@ def test_every_spaceless_tool_is_answered_in_authorize(): assert SPACELESS_TOOLS == {"list_spaces", "post_comment"}, ( "a new spaceless tool needs an explicit scope decision in authorize()") + + +# ── Telling the UI something changed ───────────────────────────────────────── + +@pytest.mark.asyncio +async def test_a_write_announces_itself(prax): + """A card filed by an agent should appear without a reload. + + The Kanban query does not poll, so a write that did not come from the UI was + invisible until someone refreshed — which is the one thing you will not + think to do while a background agent is working. + """ + seen = [] + + async def on_change(**kw): + seen.append(kw) + + await _call(client(allow=frozenset({CAP_TASK_WRITE})), "create_task", + {"space": "proj-a", "title": "Ship it"}, on_change=on_change) + assert seen == [{"space": "proj-a", "tool": "create_task"}] + + +@pytest.mark.asyncio +async def test_a_read_announces_nothing(prax): + # Nothing changed, so there is nothing for anyone to refetch. + seen = [] + + async def on_change(**kw): + seen.append(kw) + + await _call(client(), "list_tasks", {"space": "proj-a"}, on_change=on_change) + assert seen == [] + + +@pytest.mark.asyncio +async def test_a_refused_call_announces_nothing(prax): + """The UI must not be told about a change that did not happen.""" + seen = [] + + async def on_change(**kw): + seen.append(kw) + + with pytest.raises(McpError): + await _call(client(spaces=frozenset({"proj-a"}), + allow=frozenset({CAP_TASK_WRITE})), + "create_task", {"space": "other", "title": "x"}, + on_change=on_change) + assert seen == [] + + +@pytest.mark.asyncio +async def test_a_failed_announcement_does_not_fail_the_write(prax): + """The write already happened. + + Failing the call now would tell the agent its change was rejected when it + was not. A reload still shows the truth, so a missed notification is the + smaller harm. + """ + async def on_change(**kw): + raise RuntimeError("websocket is down") + + result = await _call(client(allow=frozenset({CAP_TASK_WRITE})), "create_task", + {"space": "proj-a", "title": "Ship it"}, + on_change=on_change) + assert result == {"ok": True} + assert prax.calls, "the write still went through"