fix(agent): stop re-probing endpoints that blackhole TCP connects

agent/model_metadata.py

@@ -145,6 +145,96 @@ def _strip_provider_prefix(model: str) -> str:
 _ENDPOINT_PROBE_TTL_SECONDS = 3600.0
 _endpoint_probe_path_cache: Dict[str, tuple] = {}
 
+# A configured endpoint that is routable-but-dead — e.g. a corp LAN address
+# while off-VPN — blackholes TCP: the SYN draws no SYN-ACK, no RST and no ICMP
+# error, so a probe waits out its full timeout instead of failing fast. Startup
+# runs a whole waterfall of such probes across several functions here, and the
+# stalls stack into a minute-long hang before the banner renders.
+#
+# Once ANY probe has actually observed a connect timeout for an endpoint, the
+# others have nothing to gain by repeating it. Recording that observation and
+# short-circuiting on it performs no network I/O of its own — it adds no probe
+# for callers or tests to mock, and it can only ever fire after a real timeout
+# has already been paid, so it cannot suppress a probe that would have worked.
+_ENDPOINT_BLACKHOLE_TTL_SECONDS = 30.0
+# Values are monotonic timestamps of the last observed connect timeout.
+_endpoint_blackhole_cache: Dict[str, float] = {}
+
+
+def _endpoint_host_key(base_url: str) -> Optional[str]:
+    """Return a ``host:port`` key for ``base_url``, or None if it has no host.
+
+    Keyed on host:port rather than the full URL so every probe path for one
+    server — ``/v1``-suffixed or not, LM Studio root or API root — shares a
+    single entry.
+    """
+    normalized = _normalize_base_url(base_url)
+    if not normalized:
+        return None
+    url = normalized if "://" in normalized else f"http://{normalized}"
+    try:
+        parsed = urlparse(url)
+        host = parsed.hostname
+        port = parsed.port or (443 if parsed.scheme == "https" else 80)
+    except Exception:
+        return None
+    return f"{host}:{port}" if host else None
+
+
+def _note_endpoint_blackholed(base_url: str) -> None:
+    """Record that a probe to ``base_url`` timed out during TCP connect."""
+    key = _endpoint_host_key(base_url)
+    if key is None:
+        return
+    _endpoint_blackhole_cache[key] = time.monotonic()
+    logger.debug(
+        "Endpoint %s timed out connecting — skipping further probes for %.0fs",
+        key, _ENDPOINT_BLACKHOLE_TTL_SECONDS,
+    )
+
+
+def _endpoint_blackholed(base_url: str) -> bool:
+    """True if a recent probe to ``base_url`` timed out during TCP connect.
+
+    Pure cache lookup; never touches the network. The entry expires after
+    _ENDPOINT_BLACKHOLE_TTL_SECONDS — long enough to collapse one startup's
+    burst of probes, short enough that bringing the VPN up mid-session is
+    picked up without a restart.
+    """
+    if _ENDPOINT_BLACKHOLE_TTL_SECONDS <= 0:
+        return False
+    key = _endpoint_host_key(base_url)
+    if key is None:
+        return False
+    seen = _endpoint_blackhole_cache.get(key)
+    if seen is None:
+        return False
+    if (time.monotonic() - seen) >= _ENDPOINT_BLACKHOLE_TTL_SECONDS:
+        del _endpoint_blackhole_cache[key]
+        return False
+    return True
+
+
+def _is_connect_timeout(exc: BaseException) -> bool:
+    """True for connect-phase timeouts raised by httpx or requests.
+
+    Read timeouts are deliberately excluded: those mean the server accepted
+    the connection, which is the opposite of the blackhole this guards.
+    """
+    try:
+        import httpx
+        if isinstance(exc, httpx.ConnectTimeout):
+            return True
+    except Exception:
+        pass
+    try:
+        from requests.exceptions import ConnectTimeout
+        if isinstance(exc, ConnectTimeout):
+            return True
+    except Exception:
+        pass
+    return False
+
 # ── Disk L2 for local-endpoint probe results ────────────────────────────────
 # The in-process caches above die with the process, so every CLI cold start
 # with a local model re-paid the probe waterfall in AIAgent.__init__:
@@ -876,6 +966,13 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
     if cached is not None and (time.monotonic() - cached[1]) < _ENDPOINT_PROBE_TTL_SECONDS:
         return cached[0]
 
+    # The host already blackholed a connect: skip the waterfall below, each leg
+    # of which would otherwise burn its full 2s timeout. Deliberately NOT
+    # written to _endpoint_probe_path_cache — that entry lives for an hour,
+    # which would pin the endpoint to "undetected" long after it comes back.
+    if _endpoint_blackholed(server_url):
+        return None
+
     # Disk L2: a fresh cross-process verdict skips the HTTP waterfall
     # entirely (back-to-back CLI invocations, cron ticks).
     disk_hit = _local_probe_disk_get("server_type", server_url)
@@ -885,6 +982,16 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
 
     headers = _auth_headers(api_key)
 
+    def _probe_failed(exc: Exception) -> None:
+        """Swallow a probe error — or abort the waterfall if we were blackholed.
+
+        Re-raising propagates out of the ``with`` block to the outer handler,
+        so the remaining legs are skipped instead of each stalling in turn.
+        """
+        if _is_connect_timeout(exc):
+            _note_endpoint_blackholed(server_url)
+            raise exc
+
     result: Optional[str] = None
     try:
         with httpx.Client(timeout=2.0, headers=headers) as client:
@@ -893,8 +1000,8 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
                 r = client.get(f"{lmstudio_url}/api/v1/models")
                 if r.status_code == 200:
                     result = "lm-studio"
-            except Exception:
-                pass
+            except Exception as exc:
+                _probe_failed(exc)
             if result is None:
                 # Ollama exposes /api/tags and responds with {"models": [...]}
                 # LM Studio returns {"error": "Unexpected endpoint"} with status 200
@@ -908,8 +1015,8 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
                                 result = "ollama"
                         except Exception:
                             pass
-                except Exception:
-                    pass
+                except Exception as exc:
+                    _probe_failed(exc)
             if result is None:
                 # llama.cpp exposes /v1/props (older builds used /props without the /v1 prefix)
                 try:
@@ -918,8 +1025,8 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
                         r = client.get(f"{server_url}/props")  # fallback for older builds
                     if r.status_code == 200 and "default_generation_settings" in r.text:
                         result = "llamacpp"
-                except Exception:
-                    pass
+                except Exception as exc:
+                    _probe_failed(exc)
             if result is None:
                 # vLLM: /version
                 try:
@@ -928,8 +1035,8 @@ def detect_local_server_type(base_url: str, api_key: str = "") -> Optional[str]:
                         data = r.json()
                         if "version" in data:
                             result = "vllm"
-                except Exception:
-                    pass
+                except Exception as exc:
+                    _probe_failed(exc)
     except Exception:
         pass
 
@@ -1123,6 +1230,12 @@ def fetch_endpoint_model_metadata(
         if cached is not None and (time.time() - cached_at) < _ENDPOINT_MODEL_CACHE_TTL:
             return cached
 
+    # Blackholed endpoint: every candidate below would spend its full 5s
+    # connect budget. Returned empty rather than cached, so the endpoint is
+    # retried as soon as the blackhole entry expires.
+    if _endpoint_blackholed(normalized):
+        return {}
+
     candidates = [normalized]
     if normalized.endswith("/v1"):
         alternate = normalized[:-3].rstrip("/")
@@ -1185,8 +1298,15 @@ def fetch_endpoint_model_metadata(
                 return cache
         except Exception as exc:
             last_error = exc
+            if _is_connect_timeout(exc):
+                _note_endpoint_blackholed(normalized)
 
     for candidate in candidates:
+        # A connect timeout on one candidate condemns the host, not the path:
+        # the remaining candidates differ only by URL suffix, so trying them
+        # would repeat the same stall.
+        if _endpoint_blackholed(normalized):
+            break
         # normalized/candidates stay unrewritten (cache key stability); only
         # the outbound request target is IPv4-resolved to skip the multi-second
         # dual-stack IPv6 connect timeout (see _localhost_to_ipv4).
@@ -1257,6 +1377,8 @@ def fetch_endpoint_model_metadata(
             return cache
         except Exception as exc:
             last_error = exc
+            if _is_connect_timeout(exc):
+                _note_endpoint_blackholed(normalized)
         finally:
             if response is not None:
                 response.close()
@@ -1820,6 +1942,9 @@ def _query_ollama_api_show_uncached(model: str, base_url: str, api_key: str = ""
     if server_url.endswith("/v1"):
         server_url = server_url[:-3]
 
+    if _endpoint_blackholed(server_url):
+        return None
+
     headers = _auth_headers(api_key)
 
     try:
@@ -1851,8 +1976,9 @@ def _query_ollama_api_show_uncached(model: str, base_url: str, api_key: str = ""
                                     return ctx
                             except ValueError:
                                 pass
-    except Exception:
-        pass
+    except Exception as exc:
+        if _is_connect_timeout(exc):
+            _note_endpoint_blackholed(server_url)
     return None
 
 
@@ -1940,6 +2066,9 @@ def _query_local_context_length_uncached(model: str, base_url: str, api_key: str
         server_url = server_url[:-3]
     lmstudio_url = _localhost_to_ipv4(_lmstudio_server_root(base_url))
 
+    if _endpoint_blackholed(server_url):
+        return None
+
     headers = _auth_headers(api_key)
 
     try:
@@ -2015,8 +2144,9 @@ def _query_local_context_length_uncached(model: str, base_url: str, api_key: str
                         ctx = m.get("max_model_len") or m.get("context_length") or m.get("max_tokens")
                         if ctx and isinstance(ctx, (int, float)):
                             return int(ctx)
-    except Exception:
-        pass
+    except Exception as exc:
+        if _is_connect_timeout(exc):
+            _note_endpoint_blackholed(server_url)
 
     return None