diff --git a/coworker/mcp/client.py b/coworker/mcp/client.py index ad343e36..b4db65c7 100644 --- a/coworker/mcp/client.py +++ b/coworker/mcp/client.py @@ -85,6 +85,29 @@ class MCPManager: async def tools(self, server: MCPServerDef) -> list[Any]: return (await self.ensure(server)).tools + async def verify(self, server: MCPServerDef, *, interactive: bool = False) -> _Conn: + """A REAL health check for explicit Test actions. `ensure` returns a cached + connection untouched, which made Test-on-Live a silent no-op that could not + detect a dead server (owner-hit 2026-08-21). Here a cached connection is + round-tripped (tools/list, refreshing the tool set); a dead one is torn + down and reconnected fresh.""" + conn = self._conns.get(server.name) + if conn is not None: + try: + listed = await asyncio.wait_for(conn.session.list_tools(), timeout=20) + conn.tools = list(listed.tools) + return conn + except Exception: + conn.shutdown.set() + task = self._tasks.pop(server.name, None) + if task is not None: + try: + await asyncio.wait_for(asyncio.shield(task), timeout=5) + except Exception: + task.cancel() + self._conns.pop(server.name, None) # _serve pops too; belt and braces + return await self.ensure(server, interactive=interactive) + def last_stderr(self, name: str) -> Optional[str]: """Stderr tail from the most recent failed startup of `name`, if any.""" return self._stderr_tails.get(name) diff --git a/coworker/server/manager.py b/coworker/server/manager.py index cb0e83f5..a8d50ab8 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -1330,7 +1330,9 @@ class SessionManager: self._mcp_auth_hints.discard(name) try: # The ONE place a browser sign-in may start: an explicit connect. - conn = await self.mcp.ensure(server, interactive=True) + # verify (not ensure): an already-live server gets a real round-trip + # and a refreshed tool list instead of a cached yes. + conn = await self.mcp.verify(server, interactive=True) # The Connectors row says "Ready · tested ⟨when⟩" — the claim must # survive an app restart, so it lives in prefs, not memory. self._prefs.setdefault("mcp_last_test", {})[name] = int(time.time()) diff --git a/surfaces/gui/src/components/connectors/CustomMcp.tsx b/surfaces/gui/src/components/connectors/CustomMcp.tsx index 96a00522..3d55ea95 100644 --- a/surfaces/gui/src/components/connectors/CustomMcp.tsx +++ b/surfaces/gui/src/components/connectors/CustomMcp.tsx @@ -69,7 +69,9 @@ export function mcpStatusLine(s: McpServer): string { /* leave the host off a malformed url */ } } - if (s.status !== "connected" && s.last_test_at) { + // Live servers show it too — the visible receipt that clicking Test did + // something (it re-round-trips the connection and refreshes the tool count). + if (s.last_test_at) { const rel = relTime(s.last_test_at); if (rel) bits.push(`tested ${rel}`); } diff --git a/tests/test_mcp.py b/tests/test_mcp.py index f4ac9ae4..1713ae90 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -417,3 +417,58 @@ def test_last_test_at_persists_and_clears_on_delete(tmp_path, monkeypatch): manager2.delete_mcp("fs") manager3 = SessionManager(data_dir=tmp_path / "data") assert manager3._prefs.get("mcp_last_test", {}).get("fs") is None + + +# -- verify(): Test must actually test (owner-hit 2026-08-21) ------------------- + + +@pytest.mark.asyncio +async def test_verify_round_trips_a_live_connection_and_refreshes_tools(): + """Test-on-Live used to return the cached connection untouched — a silent + no-op that couldn't detect a dead server. verify() must round-trip and + refresh the tool list.""" + from types import SimpleNamespace + + from coworker.mcp.client import MCPManager, _Conn + + mgr = MCPManager() + + class _Session: + async def list_tools(self): + return SimpleNamespace(tools=[SimpleNamespace(name="fresh_tool")]) + + conn = _Conn(_Session(), tools=[SimpleNamespace(name="stale_tool")]) + mgr._conns["srv"] = conn + server = SimpleNamespace(name="srv", transport="http", url="http://x", auth=None) + + out = await mgr.verify(server) + assert out is conn + assert [t.name for t in out.tools] == ["fresh_tool"] + + +@pytest.mark.asyncio +async def test_verify_tears_down_a_dead_connection_and_reconnects(): + from types import SimpleNamespace + + from coworker.mcp.client import MCPManager, _Conn + + mgr = MCPManager() + + class _DeadSession: + async def list_tools(self): + raise RuntimeError("connection reset") + + dead = _Conn(_DeadSession(), tools=[]) + mgr._conns["srv"] = dead + server = SimpleNamespace(name="srv", transport="http", url="http://x", auth=None) + + fresh = object() + + async def fake_ensure(s, *, interactive=False): + return fresh + + mgr.ensure = fake_ensure # type: ignore[method-assign] + out = await mgr.verify(server, interactive=True) + assert out is fresh + assert dead.shutdown.is_set() + assert "srv" not in mgr._conns