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.
This commit is contained in:
Rohit C Prasad
2026-08-21 10:43:16 -07:00
parent 76e7276f7a
commit 4ac1e59370
4 changed files with 37 additions and 3 deletions
+2 -1
View File
@@ -1177,6 +1177,7 @@ def create_app(manager: SessionManager) -> FastAPI:
# browser and waits on the loopback callback — that can take minutes, so it # 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 # runs as a background task; the GUI polls /v1/mcp for the status flip
# (authorizing → connected | needs_auth + last_error). # (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)) asyncio.create_task(manager.connect_mcp(name))
return {"ok": True, "started": True} return {"ok": True, "started": True}
@@ -2367,7 +2368,7 @@ def create_app(manager: SessionManager) -> FastAPI:
engine._append_notice( engine._append_notice(
"mcp_error", "mcp_error",
f"MCP server “{name}” failed to start{detail}"[:300] 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 # 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). # Retry/Trim — unattended runs auto-trim (the policy in engine._compact_now).
+10
View File
@@ -1303,6 +1303,15 @@ class SessionManager:
) )
return out 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]: async def connect_mcp(self, name: str) -> dict[str, Any]:
"""Connect one server NOW — for OAuth servers this may open the browser and wait """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 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]} return {"ok": False, "error": self._mcp_errors[name]}
finally: finally:
self._mcp_authorizing.discard(name) 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}"} return {"ok": False, "error": f"unknown MCP server: {name}"}
async def mcp_connect_connector(self, name: str) -> dict[str, Any]: async def mcp_connect_connector(self, name: str) -> dict[str, Any]:
+2 -2
View File
@@ -113,12 +113,12 @@ describe("itemsFromMessages mcp failure", () => {
it("replays the persisted mcp_error marker as a warn notice WITHOUT retry", () => { it("replays the persisted mcp_error marker as a warn notice WITHOUT retry", () => {
const items = itemsFromMessages([ const items = itemsFromMessages([
{ role: "user", content: "hi" }, { 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); ] as any);
expect(items[1]).toEqual({ expect(items[1]).toEqual({
kind: "notice", kind: "notice",
tone: "warn", tone: "warn",
text: "MCP server “sales-db” failed to start — see Settings ▸ MCP", text: "MCP server “sales-db” failed to start — see Settings ▸ Connectors",
}); });
}); });
}); });
+23
View File
@@ -1080,3 +1080,26 @@ def test_set_provider_persists_extra_fields(tmp_path):
manager.set_provider("ollama", {"base_url": ""}) manager.set_provider("ollama", {"base_url": ""})
providers = {p["name"]: p for p in manager.get_providers()} providers = {p["name"]: p for p in manager.get_providers()}
assert "base_url" not in providers["ollama"]["values"] 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