From 4ac1e593709268fb0334996886e5092d9c66583a Mon Sep 17 00:00:00 2001 From: Rohit C Prasad Date: Fri, 21 Aug 2026 10:43:16 -0700 Subject: [PATCH] MCP Test: flag authorizing before the background connect; notice copy says Connectors The GUI's fast poll keyed off a status the task hadn't set yet, so a failing Test looked dead until the 5s tick. --- coworker/server/app.py | 3 ++- coworker/server/manager.py | 10 ++++++++++ surfaces/gui/src/itemsFromMessages.test.ts | 4 ++-- tests/test_server.py | 23 ++++++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/coworker/server/app.py b/coworker/server/app.py index 8896a8e4..0f6c553b 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -1177,6 +1177,7 @@ def create_app(manager: SessionManager) -> FastAPI: # browser and waits on the loopback callback — that can take minutes, so it # runs as a background task; the GUI polls /v1/mcp for the status flip # (authorizing → connected | needs_auth + last_error). + manager.begin_mcp_connect(name) # authorizing shows on the very next poll asyncio.create_task(manager.connect_mcp(name)) return {"ok": True, "started": True} @@ -2367,7 +2368,7 @@ def create_app(manager: SessionManager) -> FastAPI: engine._append_notice( "mcp_error", f"MCP server “{name}” failed to start{detail}"[:300] - + " — see Settings ▸ MCP", + + " — see Settings ▸ Connectors", ) # Auto-compaction failure prompt (OPE-27): only an ATTENDED session may be asked # Retry/Trim — unattended runs auto-trim (the policy in engine._compact_now). diff --git a/coworker/server/manager.py b/coworker/server/manager.py index bbc71975..cb0e83f5 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -1303,6 +1303,15 @@ class SessionManager: ) return out + def begin_mcp_connect(self, name: str) -> None: + """Flag `authorizing` BEFORE the background connect task starts. The GUI's + fast poll keys off this status; the first refresh used to outpace the task, + so a failing Test showed nothing until the lazy 5s tick (owner-hit + 2026-08-21 — the button looked dead). Known names only, so an unknown + server can't wedge the flag (connect_mcp only clears it on a match).""" + if name in read_global(): + self._mcp_authorizing.add(name) + async def connect_mcp(self, name: str) -> dict[str, Any]: """Connect one server NOW — for OAuth servers this may open the browser and wait for the loopback callback, so callers run it as a background task and watch @@ -1346,6 +1355,7 @@ class SessionManager: return {"ok": False, "error": self._mcp_errors[name]} finally: self._mcp_authorizing.discard(name) + self._mcp_authorizing.discard(name) # begin_mcp_connect flagged a name we never matched return {"ok": False, "error": f"unknown MCP server: {name}"} async def mcp_connect_connector(self, name: str) -> dict[str, Any]: diff --git a/surfaces/gui/src/itemsFromMessages.test.ts b/surfaces/gui/src/itemsFromMessages.test.ts index 412f2de7..0c2b4680 100644 --- a/surfaces/gui/src/itemsFromMessages.test.ts +++ b/surfaces/gui/src/itemsFromMessages.test.ts @@ -113,12 +113,12 @@ describe("itemsFromMessages mcp failure", () => { it("replays the persisted mcp_error marker as a warn notice WITHOUT retry", () => { const items = itemsFromMessages([ { role: "user", content: "hi" }, - { role: "notice", kind: "mcp_error", text: "MCP server “sales-db” failed to start — see Settings ▸ MCP" }, + { role: "notice", kind: "mcp_error", text: "MCP server “sales-db” failed to start — see Settings ▸ Connectors" }, ] as any); expect(items[1]).toEqual({ kind: "notice", tone: "warn", - text: "MCP server “sales-db” failed to start — see Settings ▸ MCP", + text: "MCP server “sales-db” failed to start — see Settings ▸ Connectors", }); }); }); diff --git a/tests/test_server.py b/tests/test_server.py index 97563785..aeb86fcf 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1080,3 +1080,26 @@ def test_set_provider_persists_extra_fields(tmp_path): manager.set_provider("ollama", {"base_url": ""}) providers = {p["name"]: p for p in manager.get_providers()} assert "base_url" not in providers["ollama"]["values"] + + +def test_mcp_connect_route_flags_authorizing_immediately(tmp_path, monkeypatch): + """Owner-hit 2026-08-21: the Test button looked dead — the connect ran as a + background task, and the GUI's first refresh landed before the task set + `authorizing`, so the fast poll never armed. The route must flag it + synchronously (and only for known servers, so nothing wedges).""" + import asyncio + + from coworker.server import SessionManager + + mgr = SessionManager(data_dir=tmp_path / "data") + monkeypatch.setattr( + "coworker.server.manager.read_global", lambda: {"sales-db": {"command": "x"}} + ) + mgr.begin_mcp_connect("sales-db") + assert "sales-db" in mgr._mcp_authorizing + mgr.begin_mcp_connect("nope") + assert "nope" not in mgr._mcp_authorizing + # An unmatched name clears the flag instead of wedging "Testing…" forever. + monkeypatch.setattr("coworker.server.manager.load_mcp_servers", lambda *a, **k: []) + res = asyncio.run(mgr.connect_mcp("sales-db")) + assert not res["ok"] and "sales-db" not in mgr._mcp_authorizing