fix: add auto-close to error HTML popup, strengthen spec_path registry test

- Error HTML page now auto-closes the popup after 4s (same as success
  page), stopping the 2s polling loop and clearing the loading spinner
  so users aren't left in a confused state after a provider error.
- Replace trivial `assert manager is not None` with a meaningful assertion:
  patch _create_mcp_client and verify it is NOT called for spec_path servers,
  directly testing the short-circuit in _get_tools_from_server.
This commit is contained in:
Ishaan Jaffer 2026-03-07 11:03:19 -08:00
parent 856215b6cb
commit ba97bc28af
2 changed files with 20 additions and 2 deletions

View file

@ -125,6 +125,10 @@ def _build_error_html(title: str, message: str) -> str:
h2 {{ color: #dc2626; font-size: 20px; margin-bottom: 12px; }}
p {{ color: #475569; font-size: 14px; line-height: 1.6; }}
</style>
<script>
// Auto-close error popup so the polling loop stops and the spinner clears
if (window.opener) {{ setTimeout(function() {{ window.close(); }}, 4000); }}
</script>
</head>
<body>
<div class="card">

View file

@ -585,8 +585,22 @@ def test_spec_path_server_uses_tool_registry():
assert server.spec_path == "https://example.com/openapi.json"
assert server.is_byok is True
# The spec_path short-circuit in _get_tools_from_server is conditional on this field
assert manager is not None
# Verify the manager's short-circuit path: _get_tools_from_server checks
# spec_path before attempting MCP client creation. We confirm this by
# patching _create_mcp_client and asserting it is NOT called when spec_path is set.
from unittest.mock import AsyncMock, patch
with patch.object(manager, "_create_mcp_client", new_callable=AsyncMock) as mock_create:
# _get_tools_from_server is async but returns early for spec_path servers
import asyncio
async def _run():
return await manager._get_tools_from_server(server)
asyncio.get_event_loop().run_until_complete(_run())
mock_create.assert_not_called()
# ---------------------------------------------------------------------------