mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-12 07:10:09 +00:00
MCP Test on a live server actually tests: round-trip + tool refresh, visible receipt
verify() replaces cached-yes ensure for explicit tests; dead connections tear down and reconnect. Connected rows show 'tested ⟨when⟩' so the click has a visible result.
This commit is contained in:
@@ -85,6 +85,29 @@ class MCPManager:
|
|||||||
async def tools(self, server: MCPServerDef) -> list[Any]:
|
async def tools(self, server: MCPServerDef) -> list[Any]:
|
||||||
return (await self.ensure(server)).tools
|
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]:
|
def last_stderr(self, name: str) -> Optional[str]:
|
||||||
"""Stderr tail from the most recent failed startup of `name`, if any."""
|
"""Stderr tail from the most recent failed startup of `name`, if any."""
|
||||||
return self._stderr_tails.get(name)
|
return self._stderr_tails.get(name)
|
||||||
|
|||||||
@@ -1330,7 +1330,9 @@ class SessionManager:
|
|||||||
self._mcp_auth_hints.discard(name)
|
self._mcp_auth_hints.discard(name)
|
||||||
try:
|
try:
|
||||||
# The ONE place a browser sign-in may start: an explicit connect.
|
# 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
|
# The Connectors row says "Ready · tested ⟨when⟩" — the claim must
|
||||||
# survive an app restart, so it lives in prefs, not memory.
|
# survive an app restart, so it lives in prefs, not memory.
|
||||||
self._prefs.setdefault("mcp_last_test", {})[name] = int(time.time())
|
self._prefs.setdefault("mcp_last_test", {})[name] = int(time.time())
|
||||||
|
|||||||
@@ -69,7 +69,9 @@ export function mcpStatusLine(s: McpServer): string {
|
|||||||
/* leave the host off a malformed url */
|
/* 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);
|
const rel = relTime(s.last_test_at);
|
||||||
if (rel) bits.push(`tested ${rel}`);
|
if (rel) bits.push(`tested ${rel}`);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -417,3 +417,58 @@ def test_last_test_at_persists_and_clears_on_delete(tmp_path, monkeypatch):
|
|||||||
manager2.delete_mcp("fs")
|
manager2.delete_mcp("fs")
|
||||||
manager3 = SessionManager(data_dir=tmp_path / "data")
|
manager3 = SessionManager(data_dir=tmp_path / "data")
|
||||||
assert manager3._prefs.get("mcp_last_test", {}).get("fs") is None
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user